search

thebigpotatoe / Super-Simple-RGB-WiFi-Lamp

A project based on the ESP8266 and WS2812b
MIT License
66 stars 28 forks source link

Mode separation #25

Closed LarsMichelsen closed 4 years ago

LarsMichelsen commented 4 years ago

I just got involved in a more extensive refactoring. My goal was to centralize the implementation of each mode and encapsulate all mode-specific code in the mode classes, which now already contain the rendering and processing of the config.

I've tried to make the change in manageable commits, so hopefully it's easy to understand.

thebigpotatoe commented 4 years ago

Awesome stuff. This may take me a while to read through. I also want to give it a test as well and hopefully make some additions.

thebigpotatoe commented 4 years ago

I cannot commend you enough for your work on this. I thought I was going to spend a day getting myself up to speed but since you wrote this so well I was able to do it in one night. The code is fantastic, and I think it should really start pushing towards a library since its using classes now.

I did find and fix one little bug where during a mode change the lights would change to that mode, dim down, then dim up rather than dim down, change mode, then dim up. Luckily this was already written and just required a change of variable in one spot. I also hopefully reduce the potential of a crash by moving this logic to an else block as you will see.

Once again, amazing work, very happily merging this in. Looking forward to whats next (probably #23).

tristndev commented 4 years ago

Could you write a short documentation on how you would add new modes and what one needs to consider in this regard (e.g., how do I nicely integrate them into the web UI)? That would be great. :)

thebigpotatoe commented 4 years ago

With this merge and a the new branch I created I want to change a few things to make it easier to add modes. If you look at the "new modes" branch you can start to see the changes I'm making to get to this.

LarsMichelsen commented 4 years ago

Thanks! Looks interesting. Next I would like to split up the website and move the mode specific things to the mode classes to reach the final goal that the mode specific code is completely separated.

Are there any overlaps with your plans? Would like to avoid conflicts during merging.

thebigpotatoe commented 4 years ago

That is where I would like to go. I wrote the code originally as a sketch not knowing how much attention it would actually get, and shortcut a few things to get it in time for the competition.

Now there is no rush, what would be great is moving each mode into a derived class as you have done and push the html for each mode into the class too.

I have had a few thoughts on this, and I think the way I see it happening is to store a F string in each class of the specific HTML then send that via sockets to a statically built web page. I don't want to use a dynamically built page as its harder to maintain, or at least this is what I have found in previous projects.

So having a generic page that can be edited and debugged separate of the ESP is the way I have built it currently and the way I would like to keep it. Other than that its all open to ideas.