Skip to content

Implemented OnGetCandlesWebSocketAsync for BinanceGroup#690

Closed
egehlen wants to merge 1 commit intoDigitalRuby:masterfrom
egehlen:master
Closed

Implemented OnGetCandlesWebSocketAsync for BinanceGroup#690
egehlen wants to merge 1 commit intoDigitalRuby:masterfrom
egehlen:master

Conversation

@egehlen
Copy link
Copy Markdown

@egehlen egehlen commented Nov 26, 2021

Implemented websocket Kline/Candlestick Streams following Binance docs:
https://github.com/binance/binance-spot-api-docs/blob/master/web-socket-streams.md#klinecandlestick-streams

The data is received each 2 seconds. I know that MiniTicker method (right below this method in the docs) is faster (it updates each 1s) but there's no aggregation by period. That's why I believe this method is more suitable for this case.

Implemented websocket Kline/Candlestick Streams following Binance docs: https://github.com/binance/binance-spot-api-docs/blob/master/web-socket-streams.md#klinecandlestick-streams.
The data is received each 2 seconds. I know that MiniTicker method (right below this method in the docs) is faster (it updates each 1s) but there's no aggregation by period. That's why I believe this method is more suitable for this case.
@BZ-CO
Copy link
Copy Markdown
Contributor

BZ-CO commented Nov 29, 2021

Why you are subscribing only for the first symbol in the array?
What if marketSymbols array is empty?

@egehlen
Copy link
Copy Markdown
Author

egehlen commented Nov 29, 2021

@BZ-CO this method in binance websocket allows only one symbol per connection (see docs), if user needs to subscribe to more symbols, new connections should be made. Regarding your second question, in my commit, line 1143 is already checking for marketSymbols null or length zero.

@BZ-CO
Copy link
Copy Markdown
Contributor

BZ-CO commented Nov 30, 2021

I've opened a new PR which addresses both of my concerns.
Now you can subscribe for more than one pair on a single connection and to provide empty marketSymbols as well.

Please close this one.

@egehlen egehlen closed this Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants