LEDs: unify common app-logic behavior into functions

In the entire code base, we basically use 4 LED scenarios:
- all off
- active slot green on
- active slot green blinking
- active slot red blinking

Compacting this behaviour into 4 functions saves in total ~140B - which is huge.
It's not an entirely clean solution, LEDs should not know anything about globals::ActiveSlot, but the savings are more important.
Ideally, such an optimization could have been done by the compiler.

src/logic/command_base.cpp

@@ -212,7 +212,7 @@ void CommandBase::ErrDisengagingIdler() {
 void CommandBase::GoToErrDisengagingIdler(ErrorCode deferredEC) {
     state = ProgressCode::ERRDisengagingIdler;
     deferredErrorCode = deferredEC;
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::blink0);
+    ml::leds.ActiveSlotError();
     mi::idler.Disengage();
 }
 

src/logic/cut_filament.cpp

@@ -86,7 +86,7 @@ bool CutFilament::StepInner() {
                 // move selector aside - prepare the blade into active position
                 state = ProgressCode::PreparingBlade;
                 mg::globals.SetFilamentLoaded(cutSlot, mg::FilamentLoadState::AtPulley);
-                ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+                ml::leds.ActiveSlotProcessing();
                 MoveSelector(cutSlot + 1);
             }
         }
@@ -134,7 +134,7 @@ bool CutFilament::StepInner() {
     case ProgressCode::ReturningSelector:
         if (ms::selector.State() == ms::selector.Ready) {
             FinishedOK();
-            ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::on, ml::off);
+            ml::leds.ActiveSlotDonePrimed();
         }
         break;
     case ProgressCode::OK:

src/logic/feed_to_bondtech.cpp

@@ -17,7 +17,7 @@ void FeedToBondtech::Reset(uint8_t maxRetries) {
     dbg_logic_P(PSTR("\nFeed to Bondtech\n\n"));
     state = EngagingIdler;
     this->maxRetries = maxRetries;
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+    ml::leds.ActiveSlotProcessing();
     mi::idler.Engage(mg::globals.ActiveSlot());
 }
 
@@ -95,7 +95,7 @@ bool FeedToBondtech::Step() {
             dbg_logic_P(PSTR("Feed to Bondtech --> Idler disengaged"));
             dbg_logic_fP(PSTR("Pulley end steps %u"), mpu::pulley.CurrentPosition_mm());
             state = OK;
-            ml::leds.SetMode(mg::globals.ActiveSlot(), ml::green, ml::on);
+            ml::leds.ActiveSlotDonePrimed();
         }
         return false;
     case OK:

src/logic/feed_to_finda.cpp

@@ -17,7 +17,7 @@ bool FeedToFinda::Reset(bool feedPhaseLimited, bool haltAtEnd) {
     state = EngagingIdler;
     this->feedPhaseLimited = feedPhaseLimited;
     this->haltAtEnd = haltAtEnd;
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+    ml::leds.ActiveSlotProcessing();
     if (ms::selector.MoveToSlot(mg::globals.ActiveSlot()) != ms::Selector::OperationResult::Accepted) {
         // We can't get any FINDA readings if the selector is at the wrong spot - move it accordingly if necessary
         // And prevent issuing any commands to the idler in such an error state
@@ -68,7 +68,7 @@ bool FeedToFinda::Step() {
             return true; // return immediately to allow for a seamless planning of another move (like feeding to bondtech)
         } else if (mm::motion.QueueEmpty()) { // all moves have been finished and FINDA didn't switch on
             state = Failed;
-            ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::blink0);
+            ml::leds.ActiveSlotError();
         }
     }
         return false;

src/logic/load_filament.cpp

@@ -50,8 +50,6 @@ void logic::LoadFilament::Reset2(bool feedPhaseLimited) {
     if (!feed.Reset(feedPhaseLimited, true)) {
         // selector refused to move
         GoToErrDisengagingIdler(ErrorCode::FINDA_FLICKERS);
-    } else {
-        ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
     }
 }
 

src/logic/retract_from_finda.cpp

@@ -13,7 +13,7 @@ namespace logic {
 void RetractFromFinda::Reset() {
     dbg_logic_P(PSTR("\nRetract from FINDA\n\n"));
     state = EngagingIdler;
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+    ml::leds.ActiveSlotProcessing();
     mi::idler.Engage(mg::globals.ActiveSlot());
 }
 
@@ -33,10 +33,10 @@ bool RetractFromFinda::Step() {
                 state = OK;
                 mg::globals.SetFilamentLoaded(mg::globals.ActiveSlot(), mg::FilamentLoadState::AtPulley);
                 dbg_logic_fP(PSTR("Pulley end steps %u"), mpu::pulley.CurrentPosition_mm());
-                ml::leds.SetMode(mg::globals.ActiveSlot(), ml::green, ml::off);
+                ml::leds.ActiveSlotDoneEmpty();
             } else { // FINDA didn't switch off
                 state = Failed;
-                ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::blink0);
+                ml::leds.ActiveSlotError();
             }
         }
         return false;

src/logic/tool_change.cpp

@@ -54,20 +54,19 @@ bool ToolChange::Reset(uint8_t param) {
     return true;
 }
 
-void logic::ToolChange::GoToFeedingToBondtech() {
+void ToolChange::GoToFeedingToBondtech() {
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
     james.Reset(3);
     state = ProgressCode::FeedingToBondtech;
     error = ErrorCode::RUNNING;
 }
 
-void logic::ToolChange::ToolChangeFinishedCorrectly() {
+void ToolChange::ToolChangeFinishedCorrectly() {
-    ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::on, ml::off);
+    ml::leds.ActiveSlotDonePrimed();
     mui::userInput.SetPrinterInCharge(false);
     FinishedOK();
 }
 
-void logic::ToolChange::GoToFeedingToFinda() {
+void ToolChange::GoToFeedingToFinda() {
     state = ProgressCode::FeedingToFinda;
     error = ErrorCode::RUNNING;
     mg::globals.SetFilamentLoaded(plannedSlot, mg::FilamentLoadState::AtPulley);

src/logic/unload_to_finda.cpp

@@ -22,7 +22,7 @@ void UnloadToFinda::Reset(uint8_t maxTries) {
         state = EngagingIdler;
         mi::idler.PartiallyDisengage(mg::globals.ActiveSlot()); // basically prepare before the active slot - saves ~1s
         started_ms = mt::timebase.Millis();
-        ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+        ml::leds.ActiveSlotProcessing();
     }
 }
 
@@ -77,12 +77,12 @@ bool UnloadToFinda::Step() {
             // This scenario should not be tried again - repeating it may cause more damage to filament + potentially more collateral damage
             state = FailedFSensor;
             mm::motion.AbortPlannedMoves(); // stop rotating the pulley
-            ml::leds.SetMode(mg::globals.ActiveSlot(), ml::green, ml::off);
+            ml::leds.ActiveSlotDoneEmpty();
         } else if (!mf::finda.Pressed()) {
             // detected end of filament
             state = OK;
             mm::motion.AbortPlannedMoves(); // stop rotating the pulley
-            ml::leds.SetMode(mg::globals.ActiveSlot(), ml::green, ml::off);
+            ml::leds.ActiveSlotDoneEmpty();
         } else if (/*tmc2130_read_gstat() &&*/ mm::motion.QueueEmpty()) {
             // we reached the end of move queue, but the FINDA didn't switch off
             // two possible causes - grinded filament or malfunctioning FINDA

src/main.cpp

@@ -125,7 +125,7 @@ static void setup2() {
 
         // activate the correct LED if filament is present
         if (mg::globals.FilamentLoaded() > mg::FilamentLoadState::AtPulley) {
-            ml::leds.SetMode(mg::globals.ActiveSlot(), ml::green, ml::on);
+            ml::leds.ActiveSlotDonePrimed();
         }
     }
 

src/modules/leds.cpp

@@ -2,6 +2,7 @@
 #include "leds.h"
 #include "../hal/shr16.h"
 #include "timebase.h"
+#include "globals.h"
 
 namespace modules {
 namespace leds {
@@ -68,5 +69,21 @@ void LEDs::SetAllOff() {
     }
 }
 
+void LEDs::ActiveSlotProcessing() {
+    SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
+}
+
+void LEDs::ActiveSlotError() {
+    SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::blink0);
+}
+
+void LEDs::ActiveSlotDoneEmpty() {
+    SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::off);
+}
+
+void LEDs::ActiveSlotDonePrimed() {
+    SetPairButOffOthers(mg::globals.ActiveSlot(), ml::on, ml::off);
+}
+
 } // namespace leds
 } // namespace modules

src/modules/leds.h

@@ -122,6 +122,14 @@ public:
     /// Turn off all LEDs
     void SetAllOff();
 
+    /// Convenience functions - provide uniform implementation of LED behaviour through all the logic commands.
+    /// Intentionally not inlined to save quite some space (140B)
+    /// It's not a clean solution, LEDs should not know about mg::globals.ActiveSlot(), but the savings are important
+    void ActiveSlotProcessing();
+    void ActiveSlotError();
+    void ActiveSlotDoneEmpty();
+    void ActiveSlotDonePrimed();
+
 private:
     constexpr static const uint8_t ledPairs = config::toolCount;
     /// pairs of LEDs:

tests/unit/modules/leds/CMakeLists.txt

@@ -1,7 +1,13 @@
 # define the test executable
 add_executable(
-  leds_tests ${CMAKE_SOURCE_DIR}/src/modules/leds.cpp ${MODULES_STUBS_DIR}/stub_shr16.cpp
+  leds_tests
-             ${MODULES_STUBS_DIR}/stub_timebase.cpp test_leds.cpp
+  ${CMAKE_SOURCE_DIR}/src/modules/leds.cpp
+  ${CMAKE_SOURCE_DIR}/src/modules/globals.cpp
+  ${CMAKE_SOURCE_DIR}/src/modules/permanent_storage.cpp
+  ${MODULES_STUBS_DIR}/stub_shr16.cpp
+  ${MODULES_STUBS_DIR}/stub_timebase.cpp
+  ${MODULES_STUBS_DIR}/stub_eeprom.cpp
+  test_leds.cpp
   )
 
 # define required search paths