logic improvements

src/core/led.cpp

@@ -43,17 +43,13 @@ void LED::init() {
         pinMode(led_gpio_, OUTPUT);
     }
 
-    reset_led(false); // start with LED off
+    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) {
-    if (!led_gpio_) {
-        return false;
-    }
-
     // 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();
@@ -61,24 +57,14 @@ bool LED::loop(uint8_t healthcheck, bool button_busy) {
     }
 
     // show the LED status based on the healthcheck and button busy status
-    monitor(healthcheck, button_busy);
+    sequence_led(healthcheck, button_busy);
 
     return false;
 }
 
-// turn the LED off (false) or to it's default state (true)
+// turn the LED back it's default state depending on if it's hidden or not
-void LED::reset_led(bool default_state) {
+void LED::reset_led() {
-    if (!led_gpio_) {
+    set_led(hide_led_ ? Color::OFF : Color::GREEN); // Green
-        return;
-    }
-
-    if (default_state) {
-        // set the LED to it's default state (for RGB its green)
-        set_led(hide_led_ ? Color::OFF : Color::GREEN); // Green
-    } else {
-        // force it off
-        set_led(Color::OFF);
-    }
 }
 
 // LED flash every few ms and then perform a factory reset
@@ -93,7 +79,7 @@ void LED::led_fast_flash() {
 
     // after duration, turn off the LED
     if (current_time - led_flash_start_time_ >= led_flash_duration_) {
-        reset_led(false);
+        set_led(Color::OFF);
         led_fast_flash_timer_ = false;
 #ifndef EMSESP_DEBUG
         System::command_format(nullptr, 0); // Execute format operation, unless in debug mode
@@ -133,6 +119,10 @@ void LED::set_led(Color color) {
         blue  = RGB_LED_BRIGHTNESS;
     }
 
+    if (!led_gpio_) {
+        return;
+    }
+
     if (led_type_) {
         rgbLedWrite(led_gpio_, red, green, blue);
     } else {
@@ -179,12 +169,15 @@ void LED::set_led_routine(std::string color, std::string pattern) {
         color_steps_[1] = Color::GREEN;
         color_steps_[2] = Color::BLUE;
     }
+
+    // when this is called we want the sequence_led to restart immediately and skip the long pause
+    led_long_timer_ = uuid::get_uptime() + HEALTHCHECK_LED_FLASH_FAST_DURATION + 200UL;
 }
 
 // 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::monitor(uint8_t healthcheck, bool button_busy) {
+void LED::sequence_led(uint8_t healthcheck, bool button_busy) {
     // see if we're doing as user-requested LED blink
     bool is_user_led_blink = false;
     if (color_steps_[0] != Color::OFF || color_steps_[1] != Color::OFF || color_steps_[2] != Color::OFF) {
@@ -197,7 +190,7 @@ void LED::monitor(uint8_t healthcheck, bool button_busy) {
         set_led(button_busy ? Color::OFF : Color::YELLOW); // Yellow
     }
 
-    // we only need to run the LED monitor if there are errors, or if a button has been pressed or a user-requested LED blink is active
+    // we only need to run the LED sequence_led if there are errors, or if a button has been pressed or a user-requested LED blink is active
     if ((!healthcheck || button_busy) && !is_user_led_blink) {
         return; // nothing to show
     }
@@ -218,10 +211,10 @@ void LED::monitor(uint8_t healthcheck, bool button_busy) {
         led_short_timer_ = current_time;
 
         if (++led_flash_step_ == 8) {
-            // finished first iteration, reset the whole sequence
+            // finished first iteration, reset the whole sequence, turn off LED
             led_long_timer_ = uuid::get_uptime();
             led_flash_step_ = 0;
-            reset_led(true); // LED back to what is was before
+            set_led(Color::OFF);
         } else if (led_flash_step_ % 2) {
             // handle the three step events (on odd numbers 3,5,7 etc). see if we need to set a LED color
 

src/core/led.h

@@ -33,7 +33,7 @@ class LED {
     void init();
     bool loop(uint8_t healthcheck, bool button_busy);
 
-    void reset_led(bool default_state = true);   // turn the LED to default state or use false for off
+    void reset_led();                            // turn the LED back it's default state depending on if it's hidden or not
     void start_led_fast_flash(uint8_t duration); // duration in seconds
 
     void set_led(Color color);
@@ -42,7 +42,7 @@ class LED {
   private:
     static uuid::log::Logger logger_;
 
-    void monitor(uint8_t led_routine, bool button_busy);
+    void sequence_led(uint8_t led_routine, bool button_busy);
     void led_fast_flash();
 
     static constexpr uint32_t HEALTHCHECK_LED_LONG_DURATION       = 1000; // 1 second between flash sequences
@@ -66,7 +66,7 @@ class LED {
     bool     led_flash_state_  = false;
     uint32_t last_toggle_time_ = 0;
 
-    // monitor() state
+    // sequence_led() state
     bool     last_button_busy_ = false;
     uint32_t led_long_timer_   = 1; // 1 will kick it off immediately
     uint32_t led_short_timer_  = 0;

src/core/system.cpp

@@ -734,13 +734,17 @@ void System::start() {
     modbus_init();       // start modbus
 }
 
-// button single click
+// button single click - does nothing in normal operation
+// in debug mode, it will trigger a special healthcheck to test the LED monitoring and sequence_led
 void System::button_OnClick(PButton & b) {
     LOG_NOTICE("Button pressed - single click");
 
+#ifdef EMSESP_DEBUG
 #ifndef EMSESP_STANDALONE
-    // show filesystem
+    listDir("/", 3); // show filesystem
-    listDir("/", 3);
+#endif
+    // used to test LED monitoring and sequence_led. See system_check() for more details.
+    EMSESP::system_.healthcheck(99); // 99 = special trigger
 #endif
 }
 
@@ -973,27 +977,24 @@ void System::system_check() {
         LOG_NOTICE("Ping test, #%d", ping_count++);
 #endif
 
-        // check if we have a valid network connection
+        if (healthcheck_ != 99) { // skip if we're testing
-        if (!EMSESP::network_.network_connected()) {
+            // check if we have a valid network connection
-            healthcheck_ |= HEALTHCHECK_NO_NETWORK;
+            healthcheck_ = (healthcheck_ & ~HEALTHCHECK_NO_NETWORK) | (EMSESP::network_.network_connected() ? 0 : HEALTHCHECK_NO_NETWORK);
+
+            // check if we have a bus connection
+            healthcheck_ = (healthcheck_ & ~HEALTHCHECK_NO_BUS) | (EMSbus::bus_connected() ? 0 : HEALTHCHECK_NO_BUS);
         } else {
-            healthcheck_ &= ~HEALTHCHECK_NO_NETWORK;
+            LOG_DEBUG("Healthcheck: testing mode");
+            healthcheck_ = 0; // make it all look healthy - this is temporary for one cycle
         }
 
-        // check if we have a bus connection
+        // see if the healthcheck state has changed, if so send out the new heartbeat
-        if (!EMSbus::bus_connected()) {
-            healthcheck_ |= HEALTHCHECK_NO_BUS;
-        } else {
-            healthcheck_ &= ~HEALTHCHECK_NO_BUS;
-        }
-
-        // see if the healthcheck state has changed
         static uint8_t last_healthcheck_ = 0;
         if (healthcheck_ != last_healthcheck_) {
             last_healthcheck_ = healthcheck_;
-            EMSESP::system_.send_heartbeat(); // send MQTT heartbeat immediately when connected
+            EMSESP::system_.send_heartbeat();
             if (healthcheck_ == 0) {
-                EMSESP::led_.reset_led(true); // LED back to what is was before
+                EMSESP::led_.reset_led(); // all healthy again. Set LED back to what is was before
             }
         }
     }