lubeda / EspHoMaTriXv2

A simple DIY status display with a 8x32 RGB LED matrix, implemented with esphome.io and Home Assistant.
MIT License
276 stars 25 forks source link

[WIP] Question about rainbow_color #192

Closed andrewjswan closed 7 months ago

andrewjswan commented 8 months ago

Question

I see it changes in two places, in one place every tick: https://github.com/lubeda/EspHoMaTriXv2/blob/2024.1.0-prerelease/components/ehmtxv2/EHMTX.cpp#L1121-L1128 and in the second place once every EHMTXv2_RAINBOW_INTERVALL period: https://github.com/lubeda/EspHoMaTriXv2/blob/2024.1.0-prerelease/components/ehmtxv2/EHMTX_queue.cpp#L395-L406

In my opinion, this is somehow not entirely true, maybe it’s worth somehow bringing this to a unified change method? So that it changes only in one place?

True, I don’t quite understand which option is better to leave, but I mean that it is more logical to move the color change from update_screen to tick and change it there as soon as the color change time exceeds EHMTXv2_RAINBOW_INTERVALL.

andrewjswan commented 7 months ago

Maybe better move this from draw update_screen https://github.com/lubeda/EspHoMaTriXv2/blob/a18101cdee355cb678632351e4f55f36763e1568/components/ehmtxv2/EHMTX_queue.cpp#L395-L406 To tick with replace this https://github.com/lubeda/EspHoMaTriXv2/blob/a18101cdee355cb678632351e4f55f36763e1568/components/ehmtxv2/EHMTX.cpp#L1191-L1195

andrewjswan commented 7 months ago

And maybe change

   float red, green, blue; 
   esphome::hsv_to_rgb(this->config_->hue_, 0.8, 0.8, red, green, blue); 
   this->config_->rainbow_color = Color(uint8_t(255 * red), uint8_t(255 * green), uint8_t(255 * blue));

To

   this->config_->rainbow_color = esphome::light::ESPHSVColor(this->config_->hue_, 255, 240).to_rgb();
andrewjswan commented 7 months ago

In my opinion it looks much better.