fix LED flashing

src/core/led.cpp

@@ -16,16 +16,16 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "led.h"
-#include "emsesp.h"
+ #include "led.h"
+ #include "emsesp.h"
  
-namespace emsesp {
+ namespace emsesp {
  
-uuid::log::Logger LED::logger_{F_(led), uuid::log::Facility::KERN};
+ uuid::log::Logger LED::logger_{F_(led), uuid::log::Facility::KERN};
  
-// initialise the LED, fetching the settings from the WebSettingsService
-// set the LED to on or off when in normal operating mode
-void LED::init() {
+ // initialise the LED, fetching the settings from the WebSettingsService
+ // set the LED to on or off when in normal operating mode
+ void LED::init() {
      // copy the application settings
      EMSESP::webSettingsService.read([&](WebSettings & settings) {
          led_gpio_ = settings.led_gpio;
@@ -44,12 +44,12 @@ void LED::init() {
      }
  
      reset_led(); // start with LED in default state, depending on if it's hidden or not
-}
+ }
  
-// handle LED routine
-// called from the System::loop()
-// returns true if the LED flash is active, i.e its a lock down state
-bool LED::loop(uint8_t healthcheck, bool button_busy) {
+ // handle LED routine
+ // called from the System::loop()
+ // returns true if the LED flash is active, i.e its a lock down state
+ bool LED::loop(uint8_t healthcheck, bool button_busy) {
      // if LED flashing is active it means its about to perform a factory reset, so don't do anything else and keep it flashing
      if (led_fast_flash_timer_) {
          led_fast_flash();
@@ -59,11 +59,16 @@ bool LED::loop(uint8_t healthcheck, bool button_busy) {
      // the user-requested LED blink always has preference
      if (!is_user_led_blink_) {
          // check for button press.
-        // if button is pressed, show LED (yellow on RGB LED, on/off on standard LED)
-        // it will turn off on the next loop cycle
+         // while the button is held show the LED (yellow on RGB LED, on/off on standard LED),
+         // and on release go back to the default state and re-check the health on the next cycle
          if (last_button_busy_ != button_busy) {
              last_button_busy_ = button_busy;
-            set_led(button_busy ? Color::OFF : Color::YELLOW); // Yellow
+             if (button_busy) {
+                 set_led(Color::YELLOW); // Yellow
+             } else {
+                 reset_led();
+                 previous_healthcheck_ = System::HEALTHCHECK_RESET;
+             }
              return false;
          }
  
@@ -104,25 +109,36 @@ bool LED::loop(uint8_t healthcheck, bool button_busy) {
                  color_steps_[2] = Color::BLUE; // blue, no network and no bus
              }
          }
+ 
+         // there's nothing to flash, so leave the LED in the state reset_led() gave it.
+         // Running the sequence here would keep writing Color::OFF and a healthy system would never show green.
+         if (healthcheck == 0 || button_busy) {
+             return false;
+         }
      }
  
      // show the LED status based on the healthcheck and button busy status
      sequence_led();
  
      return false;
-}
+ }
  
-// turn the LED back it's default state depending on if it's hidden or not
-void LED::reset_led() {
+ // turn the LED back it's default state depending on if it's hidden or not
+ void LED::reset_led() {
      is_user_led_blink_ = false;
      set_led(hide_led_ ? Color::OFF : Color::GREEN); // Green
      color_steps_[0] = Color::OFF;
      color_steps_[1] = Color::OFF;
      color_steps_[2] = Color::OFF;
-}
  
-// LED flash every few ms and then perform a factory reset
-void LED::led_fast_flash() {
+     // rewind the sequence so it starts from the top if the health degrades again
+     led_long_timer_  = 1; // 1 will kick it off immediately
+     led_short_timer_ = 0;
+     led_flash_step_  = 0;
+ }
+ 
+ // LED flash every few ms and then perform a factory reset
+ void LED::led_fast_flash() {
      uint32_t current_time = uuid::get_uptime();
  
      if (current_time - last_toggle_time_ >= LED_FLASH_INTERVAL_MS) {
@@ -135,15 +151,15 @@ void LED::led_fast_flash() {
      if (current_time - led_flash_start_time_ >= led_flash_duration_) {
          set_led(Color::OFF);
          led_fast_flash_timer_ = false;
-#ifndef EMSESP_DEBUG
+ #ifndef EMSESP_DEBUG
          System::command_format(nullptr, 0); // Execute format operation, unless in debug mode
-#endif
+ #endif
+     }
  }
-}
  
-// set LED on/off or RGB color
-// ignores whether the LED is hidden or not (if hide_led_ is set)
-void LED::set_led(Color color) {
+ // set LED on/off or RGB color
+ // ignores whether the LED is hidden or not (if hide_led_ is set)
+ void LED::set_led(Color color) {
      if (!led_gpio_) {
          return;
      }
@@ -174,12 +190,12 @@ void LED::set_led(Color color) {
      } else {
          digitalWrite(led_gpio_, (red == 0 && green == 0 && blue == 0) || color == Color::OFF ? !LED_ON : LED_ON);
      }
-}
+ }
  
-// set LED custom routine
-// For example: /api/system/led?data=red:blink1
-// For older non-RGB models, the colour would default to just being on.
-bool LED::set_custom_led_routine(std::string color, std::string pattern) {
+ // set LED custom routine
+ // For example: /api/system/led?data=red:blink1
+ // For older non-RGB models, the colour would default to just being on.
+ bool LED::set_custom_led_routine(std::string color, std::string pattern) {
      static constexpr struct {
          const char * name;
          Color        value;
@@ -244,12 +260,12 @@ bool LED::set_custom_led_routine(std::string color, std::string pattern) {
      led_long_timer_ = uuid::get_uptime() + HEALTHCHECK_LED_FLASH_FAST_DURATION + 200UL;
  
      return true;
-}
+ }
  
-// uses LED to show system health and user-requested LED blinks
-// it works in a batch of 3 configured flashes, then a long pause
-// the timing is different for user-requested LED blink and for system healthcheck
-void LED::sequence_led() {
+ // uses LED to show system health and user-requested LED blinks
+ // it works in a batch of 3 configured flashes, then a long pause
+ // the timing is different for user-requested LED blink and for system healthcheck
+ void LED::sequence_led() {
      // first long pause before we start flashing
      auto current_time = uuid::get_uptime();
      if (led_long_timer_
@@ -298,10 +314,10 @@ void LED::sequence_led() {
              set_led(Color::OFF); // turn off on even number count, to make it flash
          }
      }
-}
+ }
  
-// Start the LED flash timer - duration in seconds
-void LED::start_led_fast_flash(uint8_t duration) {
+ // Start the LED flash timer - duration in seconds
+ void LED::start_led_fast_flash(uint8_t duration) {
      // Don't start if already running
      if (led_fast_flash_timer_) {
          return;
@@ -311,6 +327,6 @@ void LED::start_led_fast_flash(uint8_t duration) {
      led_flash_start_time_ = uuid::get_uptime();        // current time
      led_flash_duration_   = (uint32_t)duration * 1000; // duration in milliseconds
      led_fast_flash_timer_ = true;                      // it's active
-}
+ }
  
-} // namespace emsesp
+ } // namespace emsesp