From 9ba116e06ee6cf5143edebe46cc3fc88b25462b9 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 11 Aug 2021 08:11:54 +0200 Subject: [PATCH] Handle slot indices out of range correctly at top level Besides Unload Filament, which only operates on active slot, all other top level state machines check the validity of the command's parameter. If the parameter is out of range for available slots, they return ErrorCode::INVALID_TOOL now. --- src/logic/command_base.cpp | 8 ++++++++ src/logic/command_base.h | 4 ++++ src/logic/cut_filament.cpp | 4 ++++ src/logic/eject_filament.cpp | 4 ++++ src/logic/error_codes.h | 2 ++ src/logic/load_filament.cpp | 4 ++++ src/logic/tool_change.cpp | 4 ++++ tests/unit/logic/cut_filament/test_cut_filament.cpp | 6 ++++++ .../logic/eject_filament/test_eject_filament.cpp | 8 ++++++++ tests/unit/logic/helpers/helpers.ipp | 13 +++++++++++++ .../unit/logic/load_filament/test_load_filament.cpp | 6 ++++++ tests/unit/logic/tool_change/test_tool_change.cpp | 8 ++++++++ 12 files changed, 71 insertions(+) diff --git a/src/logic/command_base.cpp b/src/logic/command_base.cpp index a27195e..83604b2 100644 --- a/src/logic/command_base.cpp +++ b/src/logic/command_base.cpp @@ -76,4 +76,12 @@ bool CommandBase::Step() { return StepInner(); } +bool CommandBase::CheckToolIndex(uint8_t index) { + if (index >= config::toolCount) { + error = ErrorCode::INVALID_TOOL; + return false; + } + return true; +} + } // namespace logic diff --git a/src/logic/command_base.h b/src/logic/command_base.h index d44eb06..143705f 100644 --- a/src/logic/command_base.h +++ b/src/logic/command_base.h @@ -62,6 +62,10 @@ public: virtual ErrorCode Error() const { return error; } protected: + /// @returns true if the slot/tool index is within specified range (0 - config::toolCount) + /// If not, it returns false and sets the error to ErrorCode::INVALID_TOOL + bool CheckToolIndex(uint8_t index); + ProgressCode state; ///< current progress state of the state machine ErrorCode error; ///< current error code }; diff --git a/src/logic/cut_filament.cpp b/src/logic/cut_filament.cpp index 580de9a..d7dd9f6 100644 --- a/src/logic/cut_filament.cpp +++ b/src/logic/cut_filament.cpp @@ -13,6 +13,10 @@ namespace logic { CutFilament cutFilament; void CutFilament::Reset(uint8_t param) { + if (!CheckToolIndex(param)) { + return; + } + error = ErrorCode::OK; cutSlot = param; diff --git a/src/logic/eject_filament.cpp b/src/logic/eject_filament.cpp index 9d8c718..e865022 100644 --- a/src/logic/eject_filament.cpp +++ b/src/logic/eject_filament.cpp @@ -13,6 +13,10 @@ namespace logic { EjectFilament ejectFilament; void EjectFilament::Reset(uint8_t param) { + if (!CheckToolIndex(param)) { + return; + } + error = ErrorCode::OK; slot = param; diff --git a/src/logic/error_codes.h b/src/logic/error_codes.h index 1cf7207..81406c3 100644 --- a/src/logic/error_codes.h +++ b/src/logic/error_codes.h @@ -26,6 +26,8 @@ enum class ErrorCode : uint_fast16_t { FILAMENT_ALREADY_LOADED = 0x8005, ///< cannot perform operation LoadFilament or move the selector as the filament is already loaded + INVALID_TOOL = 0x8006, ///< tool/slot index out of range (typically issuing T5 into an MMU with just 5 slots - valid range 0-4) + MMU_NOT_RESPONDING = 0x802e, ///< internal error of the printer - communication with the MMU is not working INTERNAL = 0x802f, ///< internal runtime error (software) diff --git a/src/logic/load_filament.cpp b/src/logic/load_filament.cpp index 7a5be26..152207b 100644 --- a/src/logic/load_filament.cpp +++ b/src/logic/load_filament.cpp @@ -13,6 +13,10 @@ namespace logic { LoadFilament loadFilament; void LoadFilament::Reset(uint8_t param) { + if (!CheckToolIndex(param)) { + return; + } + state = ProgressCode::EngagingIdler; error = ErrorCode::OK; mg::globals.SetActiveSlot(param); diff --git a/src/logic/tool_change.cpp b/src/logic/tool_change.cpp index a0955a8..6bfaae6 100644 --- a/src/logic/tool_change.cpp +++ b/src/logic/tool_change.cpp @@ -13,6 +13,10 @@ namespace logic { ToolChange toolChange; void ToolChange::Reset(uint8_t param) { + if (!CheckToolIndex(param)) { + return; + } + if (param == mg::globals.ActiveSlot()) return; diff --git a/tests/unit/logic/cut_filament/test_cut_filament.cpp b/tests/unit/logic/cut_filament/test_cut_filament.cpp index 1a04b88..d2275d4 100644 --- a/tests/unit/logic/cut_filament/test_cut_filament.cpp +++ b/tests/unit/logic/cut_filament/test_cut_filament.cpp @@ -90,3 +90,9 @@ TEST_CASE("cut_filament::cut0", "[cut_filament]") { CutSlot(cutSlot); } } + +TEST_CASE("cut_filament::invalid_slot", "[cut_filament]") { + for (uint8_t cutSlot = 0; cutSlot < config::toolCount; ++cutSlot) { + InvalidSlot(config::toolCount, cutSlot); + } +} diff --git a/tests/unit/logic/eject_filament/test_eject_filament.cpp b/tests/unit/logic/eject_filament/test_eject_filament.cpp index feb5cb6..d9671a5 100644 --- a/tests/unit/logic/eject_filament/test_eject_filament.cpp +++ b/tests/unit/logic/eject_filament/test_eject_filament.cpp @@ -19,6 +19,8 @@ using Catch::Matchers::Equals; +#include "../helpers/helpers.ipp" + // temporarily disabled TEST_CASE("eject_filament::eject0", "[eject_filament][.]") { using namespace logic; @@ -67,6 +69,12 @@ TEST_CASE("eject_filament::eject0", "[eject_filament][.]") { // the next states are still @@TODO } +TEST_CASE("eject_filament::invalid_slot", "[eject_filament]") { + for (uint8_t cutSlot = 0; cutSlot < config::toolCount; ++cutSlot) { + InvalidSlot(config::toolCount, cutSlot); + } +} + // comments: // The tricky part of the whole state machine are the edge cases - filament not loaded, stall guards etc. // ... all the external influence we can get on the real HW diff --git a/tests/unit/logic/helpers/helpers.ipp b/tests/unit/logic/helpers/helpers.ipp index 9ab5d4e..982a65c 100644 --- a/tests/unit/logic/helpers/helpers.ipp +++ b/tests/unit/logic/helpers/helpers.ipp @@ -51,3 +51,16 @@ bool VerifyState2(SM &uf, bool filamentLoaded, uint8_t idlerSlotIndex, uint8_t s CHECKED_ELSE(uf.TopLevelState() == topLevelProgress) { return false; } return true; } + +template +void InvalidSlot(uint8_t invSlot, uint8_t activeSlot){ + ForceReinitAllAutomata(); + + SM logicSM; + REQUIRE(VerifyState(logicSM, false, mi::Idler::IdleSlotIndex(), 0, false, ml::off, ml::off, ErrorCode::OK, ProgressCode::OK)); + + EnsureActiveSlotIndex(activeSlot); + + logicSM.Reset(invSlot); + REQUIRE(VerifyState(logicSM, false, mi::Idler::IdleSlotIndex(), activeSlot, false, ml::off, ml::off, ErrorCode::INVALID_TOOL, ProgressCode::OK)); +} diff --git a/tests/unit/logic/load_filament/test_load_filament.cpp b/tests/unit/logic/load_filament/test_load_filament.cpp index 59b164e..10de810 100644 --- a/tests/unit/logic/load_filament/test_load_filament.cpp +++ b/tests/unit/logic/load_filament/test_load_filament.cpp @@ -159,3 +159,9 @@ TEST_CASE("load_filament::failed_load_to_finda_0-4_resolve_help_second_fail", "[ FailedLoadToFindaResolveHelpFindaDidntTrigger(slot, lf); } } + +TEST_CASE("load_filament::invalid_slot", "[load_filament]") { + for (uint8_t cutSlot = 0; cutSlot < config::toolCount; ++cutSlot) { + InvalidSlot(config::toolCount, cutSlot); + } +} diff --git a/tests/unit/logic/tool_change/test_tool_change.cpp b/tests/unit/logic/tool_change/test_tool_change.cpp index e596bba..3f252c9 100644 --- a/tests/unit/logic/tool_change/test_tool_change.cpp +++ b/tests/unit/logic/tool_change/test_tool_change.cpp @@ -19,6 +19,8 @@ using Catch::Matchers::Equals; +#include "../helpers/helpers.ipp" + void ToolChange(uint8_t fromSlot, uint8_t toSlot) { ForceReinitAllAutomata(); @@ -79,3 +81,9 @@ TEST_CASE("tool_change::test0", "[tool_change]") { } } } + +TEST_CASE("tool_change::invalid_slot", "[tool_change]") { + for (uint8_t cutSlot = 0; cutSlot < config::toolCount; ++cutSlot) { + InvalidSlot(config::toolCount, cutSlot); + } +}