From 299a31b765695329871b8ac530c071cd2bba54cb Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Mon, 14 Nov 2022 07:05:20 +0100 Subject: [PATCH] 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 | 27 +++++++-- src/modules/user_input.h | 11 ++++ .../load_filament/test_load_filament.cpp | 4 +- .../unload_filament/test_unload_filament.cpp | 8 ++- .../modules/user_input/test_user_input.cpp | 58 +++++++++++++++++++ 5 files changed, 97 insertions(+), 11 deletions(-) diff --git a/src/modules/user_input.cpp b/src/modules/user_input.cpp index a2ed731..6d5999d 100644 --- a/src/modules/user_input.cpp +++ b/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(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) { diff --git a/src/modules/user_input.h b/src/modules/user_input.h index f128623..de0b932 100644 --- a/src/modules/user_input.h +++ b/src/modules/user_input.h @@ -56,9 +56,20 @@ public: return printerInCharge; } +#ifndef UNITTEST private: +#endif CircularBuffer 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); }; diff --git a/tests/unit/logic/load_filament/test_load_filament.cpp b/tests/unit/logic/load_filament/test_load_filament.cpp index 1c7bc43..1800885 100644 --- a/tests/unit/logic/load_filament/test_load_filament.cpp +++ b/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); diff --git a/tests/unit/logic/unload_filament/test_unload_filament.cpp b/tests/unit/logic/unload_filament/test_unload_filament.cpp index a07773b..3351e8d 100644 --- a/tests/unit/logic/unload_filament/test_unload_filament.cpp +++ b/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); } diff --git a/tests/unit/modules/user_input/test_user_input.cpp b/tests/unit/modules/user_input/test_user_input.cpp index 6bcc3a1..800fdfc 100644 --- a/tests/unit/modules/user_input/test_user_input.cpp +++ b/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 +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); +}