Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binance: add watchTickers #16159

Merged
merged 9 commits into from Jan 4, 2023
Merged

Conversation

sc0Vu
Copy link
Contributor

@sc0Vu sc0Vu commented Dec 21, 2022

In this PR, I made these changes:

  • Add watchTickers in binance.
  • Add miniTicker in binance.

Test command:

 node examples/js/cli binance watchTickers '["ETH/USDT"]' '{"name":"!ticker"}'
 node examples/js/cli binance watchTickers '["ETH/USDT"]' '{"name":"!miniTicker"}'
 node examples/js/cli binance watchTickers 'undefined' '{"name":"!ticker"}'
 node examples/js/cli binance watchTickers 'undefined' '{"name":"!miniTicker"}'
 node examples/js/cli binance watchTicker ETH/USDT '{"name":"miniTicker"}'
 node examples/js/cli binance watchTicker ETH/USDT '{"name":"ticker"}'

js/pro/binance.js Outdated Show resolved Hide resolved
js/pro/binance.js Outdated Show resolved Hide resolved
const subscribe = {
'id': requestId,
};
const tickers = await this.watch (url, messageHash, this.extend (request, params), messageHash, subscribe);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sc0Vu I think we need to apply the same logic as we have in Cex, because, for futures Binance does not send all the updates within the same message, so we end up returning an empty array when the message does not contain the subscribed symbol
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems the topic and data format are the same for different types, probably need to wait the ticker.

These command worked for me:

node examples/js/cli binanceusdm watchTickers '["MATIC/BUSD"]'
node examples/js/cli binanceusdm watchTickers '["APT/BUSD"]'

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sc0Vu what do you mean by "probably need to wait the ticker." ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carlosmiei I see, you mean user might get empty array in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sc0Vu yeah but as I said cex solves this issue in an easy way, can you check it out?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sc0Vu is this ready to be reviewed again?

@carlosmiei
Copy link
Collaborator

LGTM ✅

 p binanceusdm watchTickers '["BTC/USDT", "LTC/USDT"]' 
Python v3.10.8
CCXT v2.5.26
binanceusdm.watchTickers(['BTC/USDT', 'LTC/USDT'])
{'BTC/USDT': {'ask': None,
              'askVolume': None,
              'average': None,
              'baseVolume': 265879.535,
              'bid': None,
              'bidVolume': None,
              'change': 186.2,
              'close': 16836.4,
              'datetime': '2023-01-04T15:51:15.807Z',
              'high': 16913.6,
              'info': {'C': 1672847475807,
                       'E': 1672847475811,
                       'F': 3169027704,
                       'L': 3170295935,
                       'O': 1672761060000,
                       'P': '1.118',
                       'Q': '0.002',
                       'c': '16836.40',
                       'e': '24hrTicker',
                       'h': '16913.60',
                       'l': '16600.30',
                       'n': 1268186,
                       'o': '16650.20',
                       'p': '186.20',
                       'q': '4460485857.92',
                       's': 'BTCUSDT',
                       'v': '265879.535',
                       'w': '16776.34'},
              'last': 16836.4,
              'low': 16600.3,
              'open': 16650.2,
              'percentage': 1.118,
              'previousClose': None,
              'quoteVolume': 4460485857.92,
              'symbol': 'BTC/USDT',
              'timestamp': 1672847475807,
              'vwap': 16776.34},
 'LTC/USDT': {'ask': None,
              'askVolume': None,
              'average': None,
              'baseVolume': 6301554.458,
              'bid': None,
              'bidVolume': None,
              'change': 1.77,
              'close': 76.1,
              'datetime': '2023-01-04T15:51:15.616Z',
              'high': 78.0,
              'info': {'C': 1672847475616,
                       'E': 1672847475620,
                       'F': 624153865,
                       'L': 624685760,
                       'O': 1672761060000,
                       'P': '2.381',
                       'Q': '0.288',
                       'c': '76.10',
                       'e': '24hrTicker',
                       'h': '78.00',
                       'l': '74.23',
                       'n': 531876,
                       'o': '74.33',
                       'p': '1.77',
                       'q': '479622872.21',
                       's': 'LTCUSDT',
                       'v': '6301554.458',
                       'w': '76.11'},
              'last': 76.1,
              'low': 74.23,
              'open': 74.33,
              'percentage': 2.381,
              'previousClose': None,
              'quoteVolume': 479622872.21,
              'symbol': 'LTC/USDT',
              'timestamp': 1672847475616,
              'vwap': 76.11}}

@kroitor kroitor merged commit 4e75dc7 into ccxt:master Jan 4, 2023
@ttodua
Copy link
Member

ttodua commented Jan 16, 2023

@sc0Vu do you have plans for adding watchTickers for coinbase (advanced api) too?
https://docs.cloud.coinbase.com/advanced-trade-api/docs/ws-channels#ticker-batch-channel
I think coinbase might be more priority compared to other secondary exchanges (i.e. cex-io, etc..)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants