Skip to content
This repository was archived by the owner on Oct 30, 2023. It is now read-only.

Conversation

@anydoby
Copy link

@anydoby anydoby commented Apr 8, 2020

First of all, thank you for your brilliant effort at making this API. I was missing some features though. Maybe not entirely in line with your style, but I tried hard to code in a similar fashion.

For my humble trading needs I had to add support for creating OCO orders and also watching trade events. This MR includes this, hope this can be useful for the others.

Copy link
Member

@joaopsilva joaopsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@joaopsilva
Copy link
Member

Can you please just add an example?

Copy link
Member

@joaopsilva joaopsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update the examples and the README.md? Thanks!

@anydoby
Copy link
Author

anydoby commented Apr 9, 2020 via email

…s for oco buy and sell orders, added an example for OCO trades in the readme
@anydoby
Copy link
Author

anydoby commented Apr 22, 2020

Hi João, I've added an example to the README and also monitoring for OCO trade updates. Now it is usable and I think mostly complete.

@TheLeBot
Copy link

@joaopsilva

Hi joaopsilva,

can you please resolve conflicts and merge so we can use that OCO stuff? Seems to be a very nice feature for trading at binance. I have seen another Pull request for similar implementation, but this from Sergey looks good enough to merge.

Thanks,
Andrej

@ghost
Copy link

ghost commented Oct 20, 2020

Hello,

How can I get the new changes with OCO support? Does waiting conflicts/changes prevent to access it?
Regards, Thank you,

@anydoby
Copy link
Author

anydoby commented Oct 20, 2020

Hello,

How can I get the new changes with OCO support? Does waiting conflicts/changes prevent to access it?
Regards, Thank you,

You can just take my fork :)

@joaopsilva
Copy link
Member

Hi @alierturk122 @anydoby , yes, please just resolve the conflicts and I will merge it. Thanks.

@hawkeye-bot
Copy link

hawkeye-bot commented Apr 1, 2021

@joaopsilva @anydoby is anybody still going to merge this? I saw #341 as well for OCO orders, I think it would be good to get at least one of the two merged? Also, I noticed that a switch to netty has been made as well

@anydoby
Copy link
Author

anydoby commented Apr 1, 2021

@joaopsilva @anydoby is anybody still going to merge this? I saw #341 as well for OCO orders, I think it would be good to get at least one of the two merged? Also, I noticed that a switch to netty has been made as well

You can use my fork as inspiration, I also added isolated margin client. It also uses netty as http client, which in my experience uses less memory and connections are more stable.

@hawkeye-bot
Copy link

@joaopsilva @anydoby is anybody still going to merge this? I saw #341 as well for OCO orders, I think it would be good to get at least one of the two merged? Also, I noticed that a switch to netty has been made as well

You can use my fork as inspiration, I also added isolated margin client. It also uses netty as http client, which in my experience uses less memory and connections are more stable.

@anydoby thanks for your reply! I actually tried out your fork, but for some websocket-data (like onAllMarketTickersEvent), the default max-frame-size used is causing problems. Other than that, your fork looks really good :) it would be good to feed back the improvements into the root repository :) as far as I can see, @joaopsilva is willing to include this merge request so it would be good to make that final push! If there's anything I can do to help please let me know, I'm happy to put in some effort to get this into the root repository!

@anydoby
Copy link
Author

anydoby commented Apr 3, 2021

Good one @hoeckxer , I increased buffer to 512k, looks like this works fine. I think my fork deviated quite a lot from the original already to be just merged. But of course I'm happy to get it merged somehow (my bots work anyway, be it merged or not ;) )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants