app: add historical market data chart #1208
Conversation
|
Oh, we could put this in 0.3 if the issues are resolvable and it's smooth for other testers. But the other PRs tagged for 0.3 take precedence, so if we can't get this until 0.4 that's fine because we are gonna make that happen super quick after 0.3 anyway. |
buck54321
force-pushed
the
candlesticks
branch
from
fc7d23f to
7312d0c
Compare
Adds a candlestick chart to the markets view. server/db: Candle types moved from /server/db to /dex/candles. core: Synchronize candlesticks and serve via BookFeed. BookFeed is changed from a concrete type to an interface, and changes are made to improve the design of the underlying core type. bookie now handles a map of *candles.Cache. app: DepthChart now inherits a new parent class, Chart, as does the new CandleChart. webserver: New wsLoadCandles method handles the 'loadcandles' websocket request by finding (or creating new) the BookFeed and calling it's Candles method. This causes candlestick data to be sent on the BookFeeds update channel, and subsequently updates for that bin width.
buck54321
force-pushed
the
candlesticks
branch
from
7312d0c to
679e1e5
Compare
There was a problem hiding this comment.
Working well.
There are a few things that could be better in the UI imo, but can always be added later.
For a new market, there is just one big candle.
And as it progresses.
Would be better to put some constant number of divisions and then these would just fill up from the left.
Also, when going to a rate that is higher than what the graph currently shows, it takes some time to show the top. Would be nice if it was more reactive there. And maybe a zoom?
Also, for rates under 1, like 0.1 to 0.00001 I don't see any change. I would think it would drop further. For example, here I bought at 10 btc/dcr, then I dropped it down to .000001 bt/dcr. Shouldn't it go down further, or is my brain too flat?
| // newBookie is a constructor for a bookie. The caller should provide a callback | ||
| // function to be called when there are no subscribers and the close timer has | ||
| // expired. | ||
| func newBookie(base, quote uint32, logger dex.Logger, close func()) *bookie { | ||
| func newBookie(dc *dexConnection, base, quote uint32, binSizes []string, logger dex.Logger) *bookie { |
There was a problem hiding this comment.
It looks like the comment about a callback function should be removed.
| for _, durStr := range binSizes { | ||
| dur, err := time.ParseDuration(durStr) | ||
| if err != nil { | ||
| logger.Errorf("failed to ParseDuration(%q)", durStr) |
There was a problem hiding this comment.
Could also include the error.
| // Next returns the channel for receiving updates. | ||
| func (f *bookFeed) Next() <-chan *BookUpdate { | ||
| return f.c | ||
| } |
There was a problem hiding this comment.
Is it expected that the bookies feedMtx be held when calling this?
There was a problem hiding this comment.
Just for the sends and close, I believe. The field itself is not modified/reassigned after bookFeed construction from what I can tell.
There was a problem hiding this comment.
On this topic, the safest pattern with channels imo is to have the same goroutine that sends on the channel also be responsible for closing it. Doing that means having a dedicated send loop for all sends and when the loop breaks it closes the channel to signal to receivers.
Not that I have a clear picture of how the bookie stuff would ideally look, but these pieces for core have always been pretty difficult for me to reason about.
That said, this PR works for me and is no less clear than what we have before I think.
| return msgjson.NewError(msgjson.RPCInternal, errMsg) | ||
| } | ||
| cl.feedMtx.RLock() | ||
| feed := cl.feed |
There was a problem hiding this comment.
After taking another look, I'm wondering if there's any way a "loadcandles" could happen before "loadmarket", which makes the feed. Should wsLoadCandles either error if not already subscribed (with a non-nil feed) or even do a loadMarket with the embedded candlesLoad.marketLoad? Just error maybe?
|
Merged, but have a look at this comment @buck54321 and anything from Joe's review that you'd like to sneak in somewhere else. |
Adds a candlestick chart to the markets view.
server/db: Candle types moved from /server/db to /dex/candles.
core: Synchronize candlesticks and serve via
BookFeed.BookFeedis changed from a concrete type to an interface, and changes are made to improve the design of the underlying core type. bookie now handles a map of*candles.Cache.app:
DepthChartnow inherits a new parent class,Chart, as does the newCandleChart.webserver: New
wsLoadCandlesmethod handles the'loadcandles'websocket request by finding (or creating new) theBookFeedand calling it'sCandlesmethod. This causes candlestick data to be sent on theBookFeedsupdate channel, and subsequently updates for that bin width.