-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix(add_sqlite3_for_persistent_storage): adding sqlite3 for persistent storage between browsers. #44
base: master
Are you sure you want to change the base?
Conversation
@caleyg Thanks for writing this! I had a branch where I was thinking to try this too. Maybe I'm missing something, but in this approach, isn't it just persisting the playlist state so one browser client could resume another one's playlist? I see you left the main pattern launch code in App.js untouched. I guess in my imagination of how #25 would be implemented, the |
HI @bosgood, thanks for the questions and thoughts. I never considered that, and you are correct, in that, in its current implementation, each browser will cause React to launch the state with data that is detected in the DB store and loaded in the browser React state. Just so I understand correctly, are you suggesting that we change or remove entirely (depending on actual implementation) the We can have the frontend push items to the playlist into the DB as it is doing but then have an I'll noodle on implementation some more. Thank you again for your input! EDIT: I think I have an approach in the works based on your recommendation. I'll keep tinkering with it over the following day or two and circle back :) but first, sleep 💤 |
Okay, I'm super close. I have the server running in a loop, launching the playlist on the server side. The UI responds to the running patterns and pulls from the DB. I want to figure out a way to get the server to react immediately instead of waiting on the following "duration" finish, and the loop continues to send the command to the controllers for the new pattern. Currently, today if you start a new pattern, react sends the command to the node-server, and the server sends the new pattern to the PBs in almost real time. With this implementation, I run in a loop with a promisefied sleep that adjusts its time based on the duration saved in the DB. for each pattern, the default being 15000ms. In this PR now running in its loop, I launch a new playlist from the UI that fires off to the PBs a few seconds later... I'm working on tightening that up, and I'll push up my changes.. while going back and revisiting this work, I uncovered a few other oversights in my first version, so thanks again for your input. Primarily around making a new playlist, doing a DELETE, then an INSERT into the DB in a single transaction, vs. just clearing the table and not inserting a new value before. |
Sounds like you're getting closer! Happy to take another look once you push your updates. For the "almost realtime" problem, perhaps there is a way for the code to, when it receives a playlist update, instead of letting the current interval finish and then updating, first cancel the current interval, fire off the change, then resume the normal 15000ms interval. |
Thanks for the pointer! I now have real-time feedback after clicking the new pattern (creating a new playlist) and having a command sent out to the PBs, which is exactly what we want... Essentially implementing exactly what you recommended with the Now the set intervals in the react code aren't in time with the actual times in the node-server... I'd like to know if I should make a WebSocket server for the playlist to emit messages to the front end when a given pattern is actually playing and update the browser state accordingly. 🤔 I'll tinker with it and see if I can get it working :) EDIT: I was able to send my first message from the backend to the frontend last night, so I'll iterate from there and see if I can get react to render appropriately |
A websocket sounds perfect for that. That's great attention to detail, it'll be really cool if you can get the frontends to actually timing-sync with the backend's patterns. |
c1c49db
to
ef3c81c
Compare
OK!!! I think this is in a good spot for validation by someone else besides me :D ohh what a journey it has been! I learned some things along the way. WebSockets is pretty rad le sigh, classic works on my machine... I'm getting when launching it on my server
troubleshooting now lmao I'm embarrassed it was this
I took inspiration from a pixelblaze on my network to resolve... ok ok once this is pushed in, I'll work on bringing these changes into my other PRs because those are awesome too <3 |
826b968
to
476ee48
Compare
https://github.com/caleyg/Firestorm/tree/caleys_branch I have an integration branch pulling in all these changes from my other PRs. And I am moving the other things in my other PRs into Websockets for a faster experience. I have to rethink how to share the WebSocket instance in all my modules without getting a circular dependency... also so I can run this in my network until we can merge this work upstream. |
@caleyg Just wanted to say how much I appreciate the enhancements you're making to Firestorm, including this one. It's really exciting to see. |
476ee48
to
20aafee
Compare
Okay, I've pushed up my integrations into this branch... The only real thing left I would like to do is move the playlist add and remove on pattern click into WebSockets, but that can be a PR for another day. If I'm feeling fancy, I'll look at that in the coming days, but it's been a wild ride, so I'll let it rest for now. There are some UI alignment issues on small browsers like telephones for my PR with the buttons I've added that I would like to address, but functionally server-wise, this is in fantastic shape! Now to see if I can build this branch and run it on my server 🤞 It would be much appreciated if anyone else could build this and give me feedback. I'm seeing a minor bug on many page loads/refreshes (and or quickly enabling and disabling all patterns) that send a message that eventually invokes 🥇 I think I found the culprit. I was saving instances of
I moved |
b69010e
to
209d069
Compare
okay, okay, in my heart of hearts, I think this is in a state where I like it a lot, and its something I can be proud of... I'll squash the commits after blessings... signing off for now be well all |
@caleyg, is it good to go? I've been holding off review and merging until it settled a little. Let me know! |
@simap Yes, I could move the add and remove the pattern calls to playlist fetch calls to WebSockets, but the fetches are working fine for now. If it proves that they would be better put into the WebSockets I'll do it or also make a new PR for it :)... before the merge tho I'll squash the commits so they look nicer in the git log on main. Thanks and be well! |
e1e6f8e
to
7a37ea3
Compare
…t storage between browsers. -- fixes dump download archive to actually download archive and alert user to not navigate from page until archive finishes. -- adds a view current playlist dialog to assist user of firestorm to quickly see whats in the current playlist being sent to PixelBlazes -- adds brightness slider to Firestorm UI to control brightness on all PixelBlazes in network. -- add SQLite to backend, and send messages to backend via websockets to assist in user expereince verus using fetch to send requests to backend node-server. Node-server on backend will process playlist now. -- add Enable/Disable All buttons to quickly enable all patterns in Firestorm list so a user doesn't have to click through them one by one if they dont want to. -- minor styling issues to better use the bootstrap templating. -- upgraded webstorm to 8.13.0 and modified controller to look at binary messages due to spec changes
7a37ea3
to
4bbc1da
Compare
I squashed this PR down to one commit, so if anyone has downloaded this branch locally, delete your local branch and pull down this one in its place! |
Hi all, I've been running the latest commit on this branch for the better part of the last week on my raspberry pi, and it works with no observable issues. I haven't had it locking up the PBs as it was before, and the data persists between restarts, and the current status reflects the same when viewed on different devices. Additionally, the response is near real-time when changing patterns in the Firestorm UI from the PB control panel and with actual LED response. I found it helpful to have the raspberry pi auto restart itself via a cronjob in the middle of the night during off hours and let it start up pm2, which launches the Firestorm server picking up the current playlist which is saved in the database. I would be interested if others have attempted or would be willing to run this branch in their local environments and the results they have. Thanks, and be well |
Hi, all! Is there anything else needed that may be preventing this from being merged? I'll be happy to address any issues that might arise |
Circling back on this with a gentle bump in hopes this can be merged in :) |
This looks great @caleyg! I have a request to make the brightness control optional. When I first launch this, it sets every PB to zero brightness, and makes it very hard to control them individually while using firestorm. Maybe a toggle next to the default-disabled brightness slider. In the disabled state it should not sent brightness commands when connecting. |
I can do this! --- I'm in the middle of moving right now, and all my PBs are packed in a pod right now as soon as I'm able I'll add that feature! Great idea! Thanks for the feedback :) |
fix(add_sqlite3_for_persistent_storage): adding sqlite3 for persistent storage between browsers.
This PR adds sqlite3 and api routes for CRUD operations to firestorm so that we can have persistent storage between browser sessions and devices.
There is some cleanup I would like to do, but this is a good first iteration. I do welcome feedback.
Validated that
durationChanges
,newPlaylist
,addPattern
, andremovePattern
from a playlist all function as expected.This PR resolves #25
Additionally, the PR in #43 does help illuminate what is happening under the hood for our users.
Thanks 🙇 🙏
This PR also includes issues:
#49
#46
#45