search

webcerebrium / java-binance-api

Java Binance API Client
MIT License
74 stars 40 forks source link

Websockets don't reconnect #28

Open shufps opened 6 years ago

shufps commented 6 years ago

Websocket-Lister do not reconnect after they have been disconnected (from binance or when disconnected by 24h DSL-disconnect)

It would be nice, to automatically reconnect Websocket-Listeners.

wcrbrm commented 6 years ago

pull requests are welcome, we're open to suggestions ;)

Websockets receive general critics as current Jetty's version is not supported for Android, so any help is appreciated, thanks

shufps commented 6 years ago

Mmmhmm, I'll think about it ... today I came about a WebSocketClient-Replacement which used something more common ... Jetty is like shooting big guns on sparrows :)

shufps commented 6 years ago

This Jetty Websocket implementation is really very problematic ... It is impossible to get rid of all these threads being created when having network problems like 24h disconnect ... I googled for hours and didn't find a solution for this problem ... Not working on android is the smaller problem :)

spectacle j15948

shufps commented 6 years ago

In this state I wouldn't run it 24/7 ... I guess, I'll fork and try to switch to a lightweight implementation. There is one which also work on android and only uses basic java network sockets.

wcrbrm commented 6 years ago

@shufps absolutely agree with the statement that this websocket implementation is a problem.

From my experience - it works only wrapped with forced re-connection. And I am also thinking of removing websockets part from this library, and moving websockets interaction into another package. So main API wrapper would not depend on sockets wrappers, and few different libraries could exist at the same time.

That would allow to play with other implementations and choose what would be the best

shufps commented 6 years ago

Perhaps we can make a deal ... I fork and change the implementation to something lightweight and you can modulize it afterwards?

wcrbrm commented 6 years ago

@shufps yes sure. We are open to any kind of conribution

shufps commented 6 years ago

I did my changes into a fork ... I'm not very familiar with git and I guess a pull-request wouldn't be a wise idea sind I changed a couple of things.

You can look into the repo here: https://github.com/shufps/java-binance-api

(Please ignore the updates on build.gradle ... I never used it before and was a bit struggeling 😂 )

I used this websocket library: https://github.com/TooTallNate/Java-WebSocket

It is supposed to work with Android 4.

I'll test it - it seems to be working - but if I know more about stability, I'll post here :)

shufps commented 6 years ago

So far new websockets working great and they survived disconnect events without going crazy :)

It makes a huuuuge difference :D

spectacle tj2634

wcrbrm commented 6 years ago

@shufps thanks for contribution, I've added a reference to your repository in the beginning of ReadMe.

shufps commented 6 years ago

I think you misunderstood ... I don't want my repository to be listed in your Readme because the fork only exists for you that you can have a look and decide whether you want to integrate the changes in your repo or not :) I've no interest in providing an alternative version because I won't ever develop them further - just wanted to contribute another websocket implementation which will - hopefully - find it's way into your repository :) I only didn't submit a pull request because the new version wouldn't be totally compatible to your version.

For instance... Session is now WebSocketClient and the BinanceWebSocketAdapter-Classes are now interfaces which have to implement unused functions too. That's not a big overhead because there are a total of 4 functions, but it wouldn't be compatible out of the box.

I hope you understood :) I'm not one of these "I fork your project and then do everything better afterwards" ... Just wanted to contribute in hope you can get rid of Jetty :)