Generate exactly 1 userInput event when button pressed (and held)

This PR is a different solution to what @gudnimg found in PR#233 / PFW-1404.

The benefit of this approach is the fact, that the button press event is generated when the button is pressed and not after it has been released.

The downside is obvious:
CPUFLASH: +28B
RAM: +1B

src/modules/user_input.cpp

@@ -7,13 +7,28 @@ namespace user_input {
 
 UserInput userInput;
 
+static_assert(config::buttonCount <= 8, "To use >8 buttons the lastButtonStates variable needs to occupy more bytes");
+static_assert((uint8_t)mb::Left == (uint8_t)Event::Left, "Indices of buttons and events must match for optimization purposes");
+static_assert((uint8_t)mb::Middle == (uint8_t)Event::Middle, "Indices of buttons and events must match for optimization purposes");
+static_assert((uint8_t)mb::Right == (uint8_t)Event::Right, "Indices of buttons and events must match for optimization purposes");
+
+void UserInput::StepOneButton(uint8_t button) {
+    bool press = mb::buttons.ButtonPressed(button);
+    if (press != LastButtonState(button)) {
+        if (press) { // emit an event only if the previous button state was "not pressed"
+            eventQueue.push(static_cast<Event>(button));
+        }
+        // flipping lastState is a bit speculative, but should be safe, because we are in the "press != lastState" branch
+        FlipLastButtonState(button);
+    }
+}
+
 void UserInput::Step() {
-    if (buttons::buttons.ButtonPressed(mb::Left))
-        eventQueue.push(Event::Left);
-    if (buttons::buttons.ButtonPressed(mb::Middle))
-        eventQueue.push(Event::Middle);
-    if (buttons::buttons.ButtonPressed(mb::Right))
-        eventQueue.push(Event::Right);
+    // for(uint8_t button = 0; button < config::buttonCount; ++button)...
+    // The usual style of iterating can be rewritten into a more cryptic but shorter one (saves 4B):
+    for (uint8_t button = config::buttonCount; button; /* nothing */) {
+        StepOneButton(--button);
+    }
 }
 
 void UserInput::ProcessMessage(uint8_t ev) {

src/modules/user_input.h

@@ -56,9 +56,20 @@ public:
         return printerInCharge;
     }
 
+#ifndef UNITTEST
 private:
+#endif
     CircularBuffer<Event, uint_fast8_t, 4> eventQueue;
     bool printerInCharge = false;
+    uint8_t lastButtonStates = 0; ///< one bit per button, 1 means pressed
+
+    constexpr bool LastButtonState(uint8_t button) const {
+        return (lastButtonStates & (1 << button)) != 0;
+    }
+    void FlipLastButtonState(uint8_t button) {
+        lastButtonStates ^= (1 << button);
+    }
+    void StepOneButton(uint8_t button);
 
     static Event StripFromPrinterBit(uint8_t e);
 };

tests/unit/logic/load_filament/test_load_filament.cpp

@@ -171,14 +171,14 @@ void FailedLoadToFindaResolveManual(uint8_t slot, logic::LoadFilament &lf) {
     // simulate the user fixed the issue himself
 
     // Perform press on button 2 + debounce + switch on FINDA
-    hal::gpio::WritePin(FINDA_PIN, hal::gpio::Level::high);
+    SetFINDAStateAndDebounce(true);
     PressButtonAndDebounce(lf, mb::Right, false);
 
     // the Idler also engages in this call as this is planned as the next step
     SimulateIdlerHoming(lf);
 
     // pulling filament back
-    REQUIRE(VerifyState(lf, mg::FilamentLoadState::InSelector, slot, slot, true, true, ml::blink0, ml::off, ErrorCode::RUNNING, ProgressCode::RetractingFromFinda));
+    REQUIRE(VerifyState(lf, mg::FilamentLoadState::InSelector, mi::Idler::IdleSlotIndex(), slot, true, true, ml::blink0, ml::off, ErrorCode::RUNNING, ProgressCode::RetractingFromFinda));
 
     ClearButtons(lf);
 

tests/unit/logic/unload_filament/test_unload_filament.cpp

@@ -76,14 +76,14 @@ void RegularUnloadFromSlot04(uint8_t slot, logic::UnloadFilament &uf, uint8_t en
     // Stage 2 - retracting from FINDA
     REQUIRE(WhileTopState(uf, ProgressCode::RetractingFromFinda, idlerEngageDisengageMaxSteps));
 
-    // Stage 3 - idler was engaged, disengage it
-    REQUIRE(WhileTopState(uf, ProgressCode::DisengagingIdler, idlerEngageDisengageMaxSteps));
-
     if (selectorShallHomeAtEnd) {
         REQUIRE(ms::selector.Slot() == 0xff);
         SimulateSelectorHoming(uf);
     }
 
+    // Stage 3 - idler was engaged, disengage it
+    REQUIRE(WhileTopState(uf, ProgressCode::DisengagingIdler, idlerEngageDisengageMaxSteps));
+
     // filament unloaded
     // idler should have been disengaged
     // no change in selector's position
@@ -268,6 +268,8 @@ void FindaDidntTriggerResolveTryAgain(uint8_t slot, logic::UnloadFilament &uf) {
     // red LED should blink, green LED should be off
     REQUIRE(VerifyState(uf, mg::FilamentLoadState::InSelector, mi::Idler::IdleSlotIndex(), slot, true, true, ml::off, ml::off, ErrorCode::RUNNING, ProgressCode::UnloadingToFinda));
 
+    ClearButtons(uf);
+
     // Assume, the Idler homed (homing is invalidated after pressing the recovery button)
     SimulateIdlerHoming(uf);
 }

tests/unit/modules/user_input/test_user_input.cpp

@@ -22,6 +22,12 @@ void PressButtonAndDebounce(uint8_t btnIndex) {
     }
 }
 
+void UnpressButtons() {
+    hal::adc::SetADC(config::buttonsADCIndex, config::buttonADCMaxValue);
+    mb::buttons.Step(); // should fall into "waiting" state
+    REQUIRE(mb::buttons.AnyButtonPressed() == false);
+}
+
 TEST_CASE("user_input::printer_in_charge", "[user_input]") {
     uint8_t button;
     mui::Event event;
@@ -60,6 +66,17 @@ TEST_CASE("user_input::printer_in_charge", "[user_input]") {
     REQUIRE_FALSE(mui::userInput.AnyEvent());
 }
 
+template <typename T>
+bool OtherButtons(uint8_t activeButton, T f) {
+    for (uint8_t button = 0; button < config::buttonCount; ++button) {
+        if (button != activeButton) {
+            if (!f(button))
+                return false;
+        }
+    }
+    return true;
+}
+
 TEST_CASE("user_input::button_pressed_MMU", "[user_input]") {
     uint8_t button;
     mui::Event event;
@@ -75,9 +92,33 @@ TEST_CASE("user_input::button_pressed_MMU", "[user_input]") {
     // reset UI
     new (&mui::userInput) mui::UserInput();
     REQUIRE_FALSE(mui::userInput.PrinterInCharge());
+    REQUIRE(mui::userInput.LastButtonState(button) == false);
 
     PressButtonAndDebounce(button);
     REQUIRE(mb::buttons.ButtonPressed(button));
+    REQUIRE(OtherButtons(button, [&](uint8_t b) { return mb::buttons.ButtonPressed(b) == false; }));
+
+    // check internal flags
+    REQUIRE(mui::userInput.LastButtonState(button) == true);
+    REQUIRE(OtherButtons(button, [&](uint8_t b) { return mui::userInput.LastButtonState(b) == false; }));
+
+    REQUIRE(mui::userInput.eventQueue.count() == 1);
+
+    // step userInput again (prevent multiple generated events when button still pressed)
+    for (uint8_t i = 0; i < 10; ++i) {
+        mui::userInput.Step();
+        REQUIRE(mui::userInput.LastButtonState(button) == true);
+        REQUIRE(OtherButtons(button, [&](uint8_t b) { return mui::userInput.LastButtonState(b) == false; }));
+        REQUIRE(mui::userInput.eventQueue.count() == 1);
+    }
+
+    // release the button
+    UnpressButtons();
+    mui::userInput.Step();
+    REQUIRE(mui::userInput.LastButtonState(button) == false);
+    REQUIRE(OtherButtons(button, [&](uint8_t b) { return mui::userInput.LastButtonState(b) == false; }));
+    REQUIRE(mui::userInput.eventQueue.count() == 1);
+
     // we should be able to extract the event with ConsumeEvent
     REQUIRE(mui::userInput.AnyEvent());
     REQUIRE(mui::userInput.ConsumeEvent() == event);
@@ -97,3 +138,20 @@ TEST_CASE("user_input::empty_queue", "[user_input]") {
     REQUIRE(mui::userInput.ConsumeEvent() == mui::NoEvent);
     REQUIRE(mui::userInput.ConsumeEventForPrinter() == mui::NoEvent);
 }
+
+TEST_CASE("user_input::lastStates", "[user_input]") {
+    uint8_t button;
+    uint8_t bitMask;
+    std::tie(button, bitMask) = GENERATE(
+        std::make_tuple(mb::Left, 4),
+        std::make_tuple(mb::Middle, 2),
+        std::make_tuple(mb::Right, 1));
+
+    new (&mui::userInput) mui::UserInput();
+
+    mui::userInput.FlipLastButtonState(button);
+    REQUIRE(mui::userInput.lastButtonStates == bitMask);
+
+    mui::userInput.FlipLastButtonState(button);
+    REQUIRE(mui::userInput.lastButtonStates == 0);
+}