From 4e323c76ffe2b67035622eb9c6bb5587fcff3828 Mon Sep 17 00:00:00 2001 From: Maurice Makaay Date: Wed, 7 Apr 2021 19:06:20 +0200 Subject: [PATCH] A round of code cleanup. --- color_night_light.h | 17 ++++++++++++++--- color_off.h | 7 +++++++ color_rgb_light.h | 7 ++++++- color_translator.h | 42 +++++++++++++++--------------------------- color_white_light.h | 6 ++++++ light.h | 43 +++++++++++++++++++++++++------------------ 6 files changed, 73 insertions(+), 49 deletions(-) diff --git a/color_night_light.h b/color_night_light.h index f2f62dd..78a6ad2 100644 --- a/color_night_light.h +++ b/color_night_light.h @@ -1,8 +1,5 @@ #pragma once -#include -#include - #include "common.h" namespace esphome { @@ -12,6 +9,20 @@ namespace bs2 { class ColorNightLight : public GPIOOutputs { public: bool set_light_color_values(light::LightColorValues v) { + // This class can handle the GPIO outputs for the night light mode. + // + // At the lowest brightness setting, switch to night light mode. + // In the Yeelight integration in Home Assistant, this feature is + // exposed trough a separate switch. I have found that the switch + // is both confusing and made me run into issues when automating + // the lights. + // I don't simply check for a brightness at or below 0.01 (1%), + // because the lowest brightness setting from Home Assistant + // turns up as 0.011765 in here (which is 3/255). + if (v.get_brightness() >= 0.012f) { + return false; + } + values = v; // This night light mode is activated when white light is selected. diff --git a/color_off.h b/color_off.h index ef2b4ab..443d026 100644 --- a/color_off.h +++ b/color_off.h @@ -12,7 +12,14 @@ namespace bs2 { class ColorOff : public GPIOOutputs { public: bool set_light_color_values(light::LightColorValues v) { + // This class can handle the light settings when the light is turned + // off or the brightness is set to zero. + if (v.get_state() != 0.0f && v.get_brightness() != 0.0f) { + return false; + } + values = v; + red = 1.0f; green = 1.0f; blue = 1.0f; diff --git a/color_rgb_light.h b/color_rgb_light.h index f008cb4..801ba5f 100644 --- a/color_rgb_light.h +++ b/color_rgb_light.h @@ -5,7 +5,6 @@ #pragma once #include -#include #include #include "common.h" @@ -248,6 +247,12 @@ static const RGBCircle rgb_circle_ {{ class ColorRGBLight : public GPIOOutputs { public: bool set_light_color_values(light::LightColorValues v) { + // This class can handle the GPIO outputs for RGB light, based + // on RGB color values + brightness. + if (v.get_white() > 0.0f) { + return false; + } + values = v; // Determine the ring level for the color. This is a value between diff --git a/color_translator.h b/color_translator.h index 0e29db8..0e90fb9 100644 --- a/color_translator.h +++ b/color_translator.h @@ -29,33 +29,21 @@ public: GPIOOutputs *delegate; - // Well, not much light here! Use the off "color". - if (v.get_state() == 0.0f || v.get_brightness() == 0.0f) { - delegate = off_light_; - } - // At the lowest brightness setting, switch to night light mode. - // In the Yeelight integration in Home Assistant, this feature is - // exposed trough a separate switch. I have found that the switch - // is both confusing and made me run into issues when automating - // the lights. - // I don't simply check for a brightness at or below 0.01 (1%), - // because the lowest brightness setting from Home Assistant - // turns up as 0.011765 in here (which is 3/255). - else if (v.get_brightness() < 0.012f) { - delegate = night_light_; - } - // When white light is requested, then use the color temperature - // white light mode: temperature + brightness. - else if (v.get_white() > 0.0f) { - delegate = white_light_; - } - // Otherwise, use RGB color mode: red, green, blue + brightness. - else { - delegate = rgb_light_; - } - - delegate->set_light_color_values(v); - delegate->copy_to(this); + // The actual implementation of the various light modes is in + // separated targeted classes. These classes are called here + // in a chain of command-like pattern, to let the first one + // that can handle the light settings do the honours. + if (off_light_->set_light_color_values(v)) + off_light_->copy_to(this); + else if (night_light_->set_light_color_values(v)) + night_light_->copy_to(this); + else if (white_light_->set_light_color_values(v)) + white_light_->copy_to(this); + else if (rgb_light_->set_light_color_values(v)) + rgb_light_->copy_to(this); + else + throw std::logic_error( + "None of the GPIOOutputs classes handles the requested light state"); return true; } diff --git a/color_white_light.h b/color_white_light.h index cb902b6..c1fd7d0 100644 --- a/color_white_light.h +++ b/color_white_light.h @@ -62,6 +62,12 @@ static const RGBWLevelsTable rgbw_levels_100_ {{ class ColorWhiteLight : public GPIOOutputs { public: bool set_light_color_values(light::LightColorValues v) { + // This class can handle the light settings when white light is + // requested, based on color temperature + brightness. + if (v.get_white() == 0.0f) { + return false; + } + values = v; auto temperature = clamp_temperature_(v.get_color_temperature()); diff --git a/light.h b/light.h index 5091fbc..c6e3a0f 100644 --- a/light.h +++ b/light.h @@ -172,36 +172,41 @@ namespace bs2 { { auto values = state->current_values; - // Turn off the light when its state is 'off'. - if (values.get_state() == 0) - { - ESP_LOGD(TAG, "Turn off the light"); - red_->set_level(1.0f); - green_->set_level(1.0f); - blue_->set_level(1.0f); - white_->set_level(0.0f); - master2_->turn_off(); - master1_->turn_off(); - return; - } - + // The color must either be set instantly, or the color is + // transitioning to an end color. The transition handler + // will do its own inspection to see if a transition is + // currently active or not. Based on the outcome, use either + // the instant or transition handler. GPIOOutputs *delegate; if (transition_handler_->set_light_color_values(values)) { - transition_handler_->log("TRANSITION"); delegate = transition_handler_; } else { instant_handler_->set_light_color_values(values); - instant_handler_->log("INSTANT"); delegate = instant_handler_; } - delegate->set_light_color_values(values); - master2_->turn_on(); - master1_->turn_on(); + // Note: one might think that it is more logical to turn on + // the LED circuitry master switch after setting the individual + // channels, but this is the order that was used by the original + // firmware. I tried to stay as close as possible to the original + // behavior, so that's why these GPIOs are turned on at this point. + if (values.get_state() != 0) + { + master2_->turn_on(); + master1_->turn_on(); + } + + // Apply the current GPIO output levels from the selected handler. red_->set_level(delegate->red); green_->set_level(delegate->green); blue_->set_level(delegate->blue); white_->set_level(delegate->white); + + if (values.get_state() == 0) + { + master2_->turn_off(); + master1_->turn_off(); + } } protected: @@ -223,6 +228,8 @@ namespace bs2 { } }; + /// This custom LightState class is used to provide access to the + /// protected LightTranformer information in the LightState class. class YeelightBS2LightState : public light::LightState, public LightStateTransformerInspector { public: