corbanmailloux / esp-mqtt-rgb-led

MQTT RGB LEDs Using JSON for Home Assistant
MIT License
270 stars 74 forks source link

Merge scripts into one #29

Closed r41d closed 6 years ago

r41d commented 6 years ago

Although this would introduce some (ugly) #ifdef and #ifndef I think it makes sense because 90% of the code is shared and it is very tedious to add features in all three files.

corbanmailloux commented 6 years ago

I've actually been wanting to do this. It was initially just one script, but then I made a new one for one specific light and then we ended up where we are now. If you'd like to take a first pass at combining them, I welcome it. I've been a little too busy on some other projects to do much with this one.

r41d commented 6 years ago

If I were to do this, I would leave as much of the code doing RGBW logic and only use the #ifdef stuff where it's relevant for communication with the outside world (i.e. when receiving messages, publishing them, or accessing the IO pins of the ESP). Does that sound reasonable? EDIT: also, I would start with merging RGB and RGBW and then thinking about the best way to get just W (=brightness) integrated as well ^^

corbanmailloux commented 6 years ago

Yeah, I think that makes sense. I'm worried that the code will become pretty messy, but maybe I'm overestimating that.

corbanmailloux commented 6 years ago

Closed with #31. One last step is tracked in #36.