-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add Initial Support for a Stock Ticker #564
Add Initial Support for a Stock Ticker #564
Conversation
This uses the FinHub API https://finnhub.io/docs/api/introduction - Needs a free API Key - Accepts a comma separated list of stock symbols, any exchange - Shows the data for each stock one at a time for 30 seconds - Shows Current Price color coded for direction of change Red, Green, White - Alternates the daily high and low prices and the opening and closing prices Additional Work Pending - Scrolling of more than one symbol - Add additional data: amount and percentage of change, Market Cap, and outstanding shares. - Any other suggested features
@mggates39 Would you be ok with me taking a look at the code and the effect on my Mesmerizer, report my feedback, but hold on the addition of the effect to the main tree until the first two bullets under "Additional Work Pending" have been dealt with? I know we recently concluded that some changes benefit from a "divide and conquer" approach, but in the case of adding a new effect I'd like the first version we have people flash on their devices to include the stuff we were kind-of planning to have in that first version anyway. In fact, we can discuss the "additional data" bullet, but I'd like it to show multiple symbols from the beginning. People who trade in stocks tend to have a portfolio containing more than one, and adding an effect instance per symbol is rather expensive from a memory perspective. |
@rbergen I would love if you would do that. I would be interested in pictures too, if it is not too difficult. I do not mind holding off until we have addressed the additional points. The data mentioned is already being collected. As it is now, one instance of the effect will rotate through the comma separated list of stock symbols, one every 30 seconds. It just does not scroll sideways through them, and @davepl pointed me to the methods I should be able to use to implement the scrolling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mggates39 As promised a first review based on me just looking at the code. I'll do my best to get round to checking out your code and running it later this weekend.
Ensure bad symbol requests can show errant spaces.
Addressed all the issues and pulled the current main branch in as well. |
So, I've today finally found the time to start testing this. Long story short: I didn't get far because of a low-memory condition, which prevented the HTTP client from making the TLS connection to Finnhub (more about Finnhub later). It seems to effectively be the same situation as discussed in discussion #562, issue #565 and a number of related discussions/PRs. I didn't expect this to happen on Mesmerizer due to our rather heavy use of PSRAM on that board, but here we are. If I recall correctly @robertlipe indicated he would look into this after the holidays, and I think we can safely conclude we're generally stuck until we figure this out. To elaborate a little on the memory usage, looking at the debug logging on my board, we seem to lose a lot of regular RAM between the invocation of When all the network dependent services (web server - also an expensive service, socket server, color data server) have started after the WiFi connection has been established, we're down to just below 40K free RAM, which is definitely too close to 0. So again, I think we have another issue to address before we can move forward with this PR. There is one thing I did notice while getting started. In secrets.example.h, the comment after the new stock ticker API key define mentions "Finhub" and "finhub.io". The correct names are "Finnhub" and "finnhub.io"; the domain with one "n" seems to be for sale if anybody's interested. |
After updating all dependencies to their now current versions - which have indeed seen updates between my previous message and this one -, we seem to once again have a sane overall free RAM situation. With which I mean that with everything started up and settled down, we start the effect cycle with approximately 75K regular RAM free. Unfortunately, this does not address the problem with the fetching of the stock ticker information; the embedded SSL client still complains about failed memory allocations. The concrete logging I see after the board has started and WiFi has connected, is this:
As I'm sure this is the first time we're trying to pull data in over HTTPS instead of HTTP, I can't fall back on previous experience for ideas on how to address this. @mggates39 I'm assuming you tested this effect before opening this PR. Did you do anything specific to get the stock info fetches to work for you? |
@rbergen I did test it on a 4MB PSRAM board with EVERY other effect turned off. I will load it up and see what the RAM numbers look like next time I am at the board, probably tonight. Then start adding things until it fails. My main issue with that board is the program memory runs out quickly. Also, to the Finnhub typos. I will get those fixed too. The URLs are right, but the comments are not. Go figure. |
Here are the configuration changes I made to get the Stock Ticker running and pulling data. Changes to include/global.h:In the #elif MESMERIZER block disable these items to free up memory
Changes to platformio.ini:In the [dev_mesmerizer] block set these items to use the 4MB Flash partition This change is not required for the standard 8MB Mesmerizer board, just my little 4MB one. board_build.partitions = config/partitions_custom.csv Changes to src/effects.cpp:Comment out most of the include effects statements. Be sure to leave the following ones in place
Comment out the static initialization for the subscriber effect since it is not included
And in the #elif MESMERIZER Block ensure that only
is enabled and all the other ADD_EFFECT() calls are commented out. From the build:
From the run:
|
Thanks! I will actually keep this at the ready if the memory improvements @davepl's been working on - which he will open a PR to merge into main soon - don't turn out to be enough. That said, if we need to switch off key features to get the effect running then we will have a problem further down the line. It'd be okay to do so while developing the effect, but in the end it would have to work in unison with the rest of the project. Anyway, I'll revisit this when I can retest with Dave's upcoming changes in. |
You are welcome. Again, I had to cut things down because the board was a small one. I wonder if the String's data is in base RAM or PSRAM when the String itself is defined in an object that is allocated out of PSRAM. I look forward to digging into this further. |
I'd expect String to use the default allocator, which on Mesmerizer should allocate any block larger than (currently) 128 bytes in PSRAM. Which kind of translates to "it depends on the size/composition of the String". But this is only based on my common sense reasoning, not a deep dive into the actual implementation of String. |
Remember, bulk of the changes are just so it 'fits' on my 4MB board. I wonder if the data behind the Strings defined in the Stock Ticker objects are in base RAM or PSRAM? Do you know? I look forward to digging into this further. |
I think I shared my perspective on this question in my previous comment? |
@rbergen Yes you did. I accidentally posted my comment twice. My box restarted and I did not see the first comment when I came back to the conversation. |
I have a new development board from AliExpress on the way. My choice was based on the discussion here. The new board has enough memory to allow me to run the full stack with the latest memory fixes. This will allow me to better debug my memory usage on HTTPS connections and get this working on a normal configuration. Also, I have some ideas about scrolling that I will start testing once I have my matrix wired to the new board. |
@mggates39 Thanks for that update. I hope to find the time to start testing your effect with the current state of |
I have started the scrolling changes on a new development thread. If I like how it looks, I will merge it back into this one. Wish me luck. I will continue to keep this branch up to date as master evolves. |
Good luck! :) |
I am back on this project. Had work issues come up, and then Google threatened to terminate my developer account if I did not get my applications up to one of the latest SDK versions. That took forever, but I beat the deadline and can now get back to this. Updates to the Pull Request to follow. |
@mggates39 Good to have you back! As I think most people are, I'm quite familiar with work getting in the way of free time. :) I look forward to future updates to this PR. As it stands, for me the memory usage related to the use of HTTPS is still an unresolved issue. |
@mggates39 I merged #626 yesterday, which includes a finished stock ticker effect. It supports multiple ticker symbols being configured and it circumvents the HTTPS issue by not using HTTPS - instead it pulls its data from a proxy service hosted by Dave. With this effect now in, I think the right thing to do from a project perspective is to close this PR. I know you've spent your time and energy on this, and I really appreciate the investment you did in this. For one, it brought one issue to the surface that had to be addressed in the other stock ticker effect, and that we'll have to consider when developing other effects and capabilities in the future. |
I agree. As I ease back into this project, pulled away by health issues, I will look at the new implementation. I am glad that the effort was not wasted and the issues uncovered were addressed. I look forward to making additional contributions to this project. |
Thank you for responding. I'm sorry to hear about the reason for your absence, and look forward to any future contributions you can make. |
Description
This begins to addresses Issue 341
Additional Work Pending
Contributing requirements
main
as the target branch.