Firestorm
Firestorm copied to clipboard
fix(add_sqlite3_for_persistent_storage): adding sqlite3 for persistent storage between browsers.
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
, and removePattern
from a playlist all function as expected.
This PR resolves https://github.com/simap/Firestorm/issues/25
Additionally, the PR in https://github.com/simap/Firestorm/pull/43 does help illuminate what is happening under the hood for our users.
Thanks 🙇 🙏
This PR also includes issues: https://github.com/simap/Firestorm/issues/49 https://github.com/simap/Firestorm/issues/46 https://github.com/simap/Firestorm/issues/45
@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 setInterval
wouldn't happen in React at all, but rather in the node server, sort of like how discovery works.
@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
setInterval
wouldn't happen in React at all, but rather in the node server, sort of like how discovery works.
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 _launchPatternAndSetTimeout
and _launchCurrentPattern
from App.js
and let the process that launches the pattern singularly run in the node server? If so, I never considered that, and it honestly makes a lot of sense.
We can have the frontend push items to the playlist into the DB as it is doing but then have an api/playlist.js
running to fire off the patterns appropriately and as a single process vs. many browsers (devices) that happen to be on the network.
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.
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 clearInterval
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.
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
WebSocket connection to 'ws://0.0.0.0:1890/ws' failed:
WebSocket is already in CLOSING or CLOSED state.
troubleshooting now
lmao I'm embarrassed it was this
const hostname = window.location.hostname
export const ws = new WebSocket(`ws://${hostname}:1890/ws`);
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
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.
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 clearintervals
on the playlist loop, causing a few different loops to fire off patterns in a spam-like fashion... I'll need to see how to handle that edge case, I effectively locked up one of my leader PBs... I'll noodle on the most effective way to handle this.
🥇 I think I found the culprit. I was saving instances of setIntervals
in a prop of the Playlist
class and every time we instantiated that class, we got a new instance of the setIntervals
:
constructor(utils) {
this.playlistLoopTimeout = null
this.c = null
<...>
}
I moved playlistLoopTimeout
, playlistLoopTimeout
as global vars, and saving to that, and then operating on those objects in each instance of the Playlist
made it behave like I want w00t. I'll bang on it some more and see if I can find any issues
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!
@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!
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.
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 :)