From df2c1ba7fe2d9e383b772b25faab46578a14b82b Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 20 Jul 2021 06:57:00 +0200 Subject: [PATCH] Add prototype of unified handling of HW errors in the logic layer --- src/config/config.h | 4 +++ src/hal/tmc2130.h | 4 ++- src/logic/command_base.cpp | 20 +++++++++++++ src/logic/command_base.h | 11 +++++-- src/logic/cut_filament.cpp | 4 +-- src/logic/cut_filament.h | 2 +- src/logic/eject_filament.cpp | 4 +-- src/logic/eject_filament.h | 2 +- src/logic/load_filament.cpp | 4 +-- src/logic/load_filament.h | 2 +- src/logic/no_command.h | 2 +- src/logic/progress_codes.h | 1 + src/logic/tool_change.cpp | 6 ++-- src/logic/tool_change.h | 2 +- src/logic/unload_filament.cpp | 2 +- src/logic/unload_filament.h | 2 +- src/modules/idler.cpp | 29 ++++++++++++++++--- src/modules/idler.h | 18 +++++++++--- src/modules/motion.cpp | 12 ++++---- src/modules/motion.h | 3 +- tests/unit/logic/cut_filament/CMakeLists.txt | 1 + .../unit/logic/eject_filament/CMakeLists.txt | 1 + tests/unit/logic/load_filament/CMakeLists.txt | 1 + tests/unit/logic/stubs/stub_motion.cpp | 3 +- tests/unit/logic/tool_change/CMakeLists.txt | 1 + .../unit/logic/unload_filament/CMakeLists.txt | 1 + .../unload_filament/test_unload_filament.cpp | 4 +-- 27 files changed, 108 insertions(+), 38 deletions(-) diff --git a/src/config/config.h b/src/config/config.h index baa2ce6..6c0f75a 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -106,6 +106,10 @@ static constexpr IdlerLimits idlerLimits = { .accel = 1000.0_deg_s2, }; + +/// Max retries of FeedToBondtech used in LoadFilament +static constexpr uint8_t feedToBondtechMaxRetries = 2; + // TMC2130 setup static constexpr int8_t tmc2130_sg_thrs = 3; // @todo 7bit two's complement for the sg_thrs diff --git a/src/hal/tmc2130.h b/src/hal/tmc2130.h index 7b667dc..f309d23 100644 --- a/src/hal/tmc2130.h +++ b/src/hal/tmc2130.h @@ -80,7 +80,9 @@ public: const MotorCurrents ¤ts, MotorMode mode); - /// (re)initialization of the chip + /// (re)initialization of the chip - please note this is necessary due to some HW flaws in the original MMU boards. + /// And yes, the TMC may not get correctly initialized. + /// @returns true if the TMC2130 was inited correctly bool Init(const MotorParams ¶ms); /// Get the current motor mode diff --git a/src/logic/command_base.cpp b/src/logic/command_base.cpp index 084573b..2ca28e9 100644 --- a/src/logic/command_base.cpp +++ b/src/logic/command_base.cpp @@ -1,5 +1,25 @@ #include "command_base.h" +#include "../modules/idler.h" + +namespace mi = modules::idler; namespace logic { +bool CommandBase::Step() { + // check the global HW errors - may be we should avoid the modules layer and check for the HAL layer errors directly + // @@TODO discuss... + bool any_error = mi::idler.State() == mi::Idler::Failed; + + // @@TODO check all other HW issues here to be able to respond with the appropriate error code into the printer + + if (any_error) { + state = ProgressCode::ERR1TMCInitFailed; + error = ErrorCode::TMC_INIT_ERROR; + return true; // the HW error prevents us from continuing with the with the state machine + // the MMU must be restarted/fixed before continuing + } else { + return StepInner(); + } +} + } // namespace logic diff --git a/src/logic/command_base.h b/src/logic/command_base.h index 2ebfc25..d44eb06 100644 --- a/src/logic/command_base.h +++ b/src/logic/command_base.h @@ -30,9 +30,16 @@ public: /// @param param numerical parameter that comes with some commands (e.g. T1 for tool change 1) virtual void Reset(uint8_t param) = 0; - /// steps the state machine + /// Steps the state machine. This is the preferred way of stepping the machine + /// as it handles the global HW error states uniformly (so that the derived classes do not have to deal + /// with these error states on their own). + /// Each derived class then only implements its own logic via the virtual #StepInner method. /// @returns true if the automaton finished its work - virtual bool Step() = 0; + bool Step(); + + /// Each derived class shall implement its own state machine logic in this method + /// It is being called from #Step after the HW error states have been checked + virtual bool StepInner() = 0; /// @returns progress of operation - each automaton consists of several internal states /// which should be reported to the user via the printer's LCD diff --git a/src/logic/cut_filament.cpp b/src/logic/cut_filament.cpp index 151cad4..0a282db 100644 --- a/src/logic/cut_filament.cpp +++ b/src/logic/cut_filament.cpp @@ -38,10 +38,10 @@ void CutFilament::SelectFilamentSlot() { ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off); } -bool CutFilament::Step() { +bool CutFilament::StepInner() { switch (state) { case ProgressCode::UnloadingFilament: - if (unl.Step()) { + if (unl.StepInner()) { // unloading sequence finished - basically, no errors can occurr here // as UnloadFilament should handle all the possible error states on its own // There is no way the UnloadFilament to finish in an error state diff --git a/src/logic/cut_filament.h b/src/logic/cut_filament.h index d60ec96..c3dc7b6 100644 --- a/src/logic/cut_filament.h +++ b/src/logic/cut_filament.h @@ -17,7 +17,7 @@ public: void Reset(uint8_t param) override; /// @returns true if the state machine finished its job, false otherwise - bool Step() override; + bool StepInner() override; ProgressCode State() const override; diff --git a/src/logic/eject_filament.cpp b/src/logic/eject_filament.cpp index 50f2e18..b6c037b 100644 --- a/src/logic/eject_filament.cpp +++ b/src/logic/eject_filament.cpp @@ -36,10 +36,10 @@ void EjectFilament::MoveSelectorAside() { ms::selector.MoveToSlot(selectorParkedPos); } -bool EjectFilament::Step() { +bool EjectFilament::StepInner() { switch (state) { case ProgressCode::UnloadingFilament: - if (unl.Step()) { + if (unl.StepInner()) { // unloading sequence finished - basically, no errors can occurr here // as UnloadFilament should handle all the possible error states on its own // There is no way the UnloadFilament to finish in an error state diff --git a/src/logic/eject_filament.h b/src/logic/eject_filament.h index 32ef524..e1716e1 100644 --- a/src/logic/eject_filament.h +++ b/src/logic/eject_filament.h @@ -29,7 +29,7 @@ public: void Reset(uint8_t param) override; /// @returns true if the state machine finished its job, false otherwise - bool Step() override; + bool StepInner() override; ProgressCode State() const override; diff --git a/src/logic/load_filament.cpp b/src/logic/load_filament.cpp index 30bcddf..1467c3a 100644 --- a/src/logic/load_filament.cpp +++ b/src/logic/load_filament.cpp @@ -29,7 +29,7 @@ void LoadFilament::Reset(uint8_t param) { ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off); } -bool LoadFilament::Step() { +bool LoadFilament::StepInner() { switch (state) { case ProgressCode::EngagingIdler: if (mi::idler.Engaged()) { @@ -49,7 +49,7 @@ bool LoadFilament::Step() { ml::leds.SetMode(mg::globals.ActiveSlot(), ml::Color::red, ml::Mode::blink0); // signal loading error } else { state = ProgressCode::FeedingToBondtech; - james.Reset(2); + james.Reset(config::feedToBondtechMaxRetries); } } break; diff --git a/src/logic/load_filament.h b/src/logic/load_filament.h index 0ac217e..f469a7d 100644 --- a/src/logic/load_filament.h +++ b/src/logic/load_filament.h @@ -17,7 +17,7 @@ public: void Reset(uint8_t param) override; /// @returns true if the state machine finished its job, false otherwise - bool Step() override; + bool StepInner() override; private: FeedToFinda feed; diff --git a/src/logic/no_command.h b/src/logic/no_command.h index adee86b..6f372c4 100644 --- a/src/logic/no_command.h +++ b/src/logic/no_command.h @@ -15,7 +15,7 @@ public: void Reset(uint8_t /*param*/) override {} /// @returns true if the state machine finished its job, false otherwise - bool Step() override { return true; } + bool StepInner() override { return true; } }; /// The one and only instance of NoCommand state machine in the FW diff --git a/src/logic/progress_codes.h b/src/logic/progress_codes.h index e3d7e60..b0d1261 100644 --- a/src/logic/progress_codes.h +++ b/src/logic/progress_codes.h @@ -21,6 +21,7 @@ enum class ProgressCode : uint_fast8_t { ERR1WaitingForUser, ERRInternal, ERR1HelpingFilament, + ERR1TMCInitFailed, UnloadingFilament, LoadingFilament, diff --git a/src/logic/tool_change.cpp b/src/logic/tool_change.cpp index b06bf15..6354bf8 100644 --- a/src/logic/tool_change.cpp +++ b/src/logic/tool_change.cpp @@ -29,10 +29,10 @@ void ToolChange::Reset(uint8_t param) { } } -bool ToolChange::Step() { +bool ToolChange::StepInner() { switch (state) { case ProgressCode::UnloadingFilament: - if (unl.Step()) { + if (unl.StepInner()) { // unloading sequence finished - basically, no errors can occurr here // as UnloadFilament should handle all the possible error states on its own // There is no way the UnloadFilament to finish in an error state @@ -41,7 +41,7 @@ bool ToolChange::Step() { } break; case ProgressCode::LoadingFilament: - if (load.Step()) { + if (load.StepInner()) { // loading sequence finished - basically, no errors can occurr here // as LoadFilament should handle all the possible error states on its own // There is no way the LoadFilament to finish in an error state diff --git a/src/logic/tool_change.h b/src/logic/tool_change.h index b4c9197..e58cf34 100644 --- a/src/logic/tool_change.h +++ b/src/logic/tool_change.h @@ -17,7 +17,7 @@ public: void Reset(uint8_t param) override; /// @returns true if the state machine finished its job, false otherwise - bool Step() override; + bool StepInner() override; ProgressCode State() const override; diff --git a/src/logic/unload_filament.cpp b/src/logic/unload_filament.cpp index 8072d8f..a2472bb 100644 --- a/src/logic/unload_filament.cpp +++ b/src/logic/unload_filament.cpp @@ -28,7 +28,7 @@ void UnloadFilament::Reset(uint8_t /*param*/) { ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off); } -bool UnloadFilament::Step() { +bool UnloadFilament::StepInner() { switch (state) { // state 1 engage idler - will be done by the Unload to FINDA state machine case ProgressCode::UnloadingToFinda: // state 2 rotate pulley as long as the FINDA is on diff --git a/src/logic/unload_filament.h b/src/logic/unload_filament.h index 6600e67..0e4e7a1 100644 --- a/src/logic/unload_filament.h +++ b/src/logic/unload_filament.h @@ -16,7 +16,7 @@ public: void Reset(uint8_t param) override; /// @returns true if the state machine finished its job, false otherwise - bool Step() override; + bool StepInner() override; private: constexpr static const uint8_t maxRetries = 3; diff --git a/src/modules/idler.cpp b/src/modules/idler.cpp index ca7618d..66b057e 100644 --- a/src/modules/idler.cpp +++ b/src/modules/idler.cpp @@ -11,9 +11,9 @@ Idler idler; namespace mm = modules::motion; -bool Idler::Disengage() { +Idler::EngageDisengage Idler::Disengage() { if (state == Moving) - return false; + return EngageDisengage::Refused; plannedEngage = false; @@ -25,11 +25,22 @@ bool Idler::Disengage() { mm::motion.PlanMoveTo(SlotPosition(IdleSlotIndex()), 1000._I_deg_s); // @@TODO state = Moving; return true; +// return EngageDisengage::Accepted; +// +// if (!mm::motion.InitAxis(mm::Idler)) { +// state = Failed; +// return EngageDisengage::Failed; +// } else { +// // plan move to idle position +// mm::motion.PlanMove(mm::Idler, config::idlerSlotPositions[IdleSlotIndex()] - mm::motion.Position(mm::Idler), 1000); // @@TODO +// state = Moving; +// return EngageDisengage::Accepted; +// } } -bool Idler::Engage(uint8_t slot) { +Idler::EngageDisengage Idler::Engage(uint8_t slot) { if (state == Moving) - return false; + return EngageDisengage::Refused; plannedSlot = slot; plannedEngage = true; @@ -41,6 +52,16 @@ bool Idler::Engage(uint8_t slot) { mm::motion.PlanMoveTo(SlotPosition(slot), 1000._I_deg_s); // @@TODO state = Moving; return true; +// return EngageDisengage::Accepted; +// +// if (!mm::motion.InitAxis(mm::Idler)) { +// state = Failed; +// return EngageDisengage::Failed; +// } else { +// mm::motion.PlanMove(mm::Idler, config::idlerSlotPositions[slot] - mm::motion.Position(mm::Idler), 1000); // @@TODO +// state = Moving; +// return EngageDisengage::Accepted; +// } } bool Idler::Home() { diff --git a/src/modules/idler.h b/src/modules/idler.h index 79946cf..78fe803 100644 --- a/src/modules/idler.h +++ b/src/modules/idler.h @@ -26,14 +26,21 @@ public: , currentSlot(0) , currentlyEngaged(false) {} + /// Engage/Disengage return values + enum class EngageDisengage : uint8_t { + Accepted, ///< the operation has been successfully started + Refused, ///< another operation is currently underway, cannot start a new one + Failed ///< the operation could not been started due to HW issues + }; + /// Plan engaging of the idler to a specific filament slot /// @param slot index to be activated - /// @returns false in case an operation is already underway - bool Engage(uint8_t slot); + /// @returns #EngageDisengage + EngageDisengage Engage(uint8_t slot); /// Plan disengaging of the idler, i.e. parking the idler - /// @returns false in case an operation is already underway - bool Disengage(); + /// @returns #EngageDisengage + EngageDisengage Disengage(); /// Plan homing of the idler axis /// @returns false in case an operation is already underway @@ -59,6 +66,9 @@ public: /// @returns the index of idle position of the idler, usually 5 in case of 0-4 valid indices of filament slots inline static constexpr uint8_t IdleSlotIndex() { return config::toolCount; } + /// @returns internal state of the Idler + inline uint8_t State() const { return state; } + private: /// internal state of the automaton uint8_t state; diff --git a/src/modules/motion.cpp b/src/modules/motion.cpp index 0fe2d15..5db6d8c 100644 --- a/src/modules/motion.cpp +++ b/src/modules/motion.cpp @@ -5,13 +5,11 @@ namespace motion { Motion motion; -void Motion::InitAxis(Axis axis) { - for (uint8_t i = 0; i != NUM_AXIS; ++i) { - // disable the axis and re-init the driver: this will clear the internal - // StallGuard data as a result without special handling - Disable(axis); - axisData[axis].drv.Init(axisParams[axis].params); - } +bool Motion::InitAxis(Axis axis) { + // disable the axis and re-init the driver: this will clear the internal + // StallGuard data as a result without special handling + Disable(axis); + return axisData[axis].drv.Init(axisParams[axis].params); } void Motion::SetEnabled(Axis axis, bool enabled) { diff --git a/src/modules/motion.h b/src/modules/motion.h index 3a1ad3a..f307aa9 100644 --- a/src/modules/motion.h +++ b/src/modules/motion.h @@ -75,7 +75,8 @@ public: /// Init axis driver - @@TODO this should be probably hidden /// somewhere deeper ... something should manage the axes and their /// state especially when the TMC may get randomly reset (deinited) - void InitAxis(Axis axis); + /// @returns true if the init was successful (TMC2130 responded ok) + bool InitAxis(Axis axis); /// Set axis power status. One must manually ensure no moves are currently being /// performed by calling QueueEmpty(). diff --git a/tests/unit/logic/cut_filament/CMakeLists.txt b/tests/unit/logic/cut_filament/CMakeLists.txt index 651b238..7a68f82 100644 --- a/tests/unit/logic/cut_filament/CMakeLists.txt +++ b/tests/unit/logic/cut_filament/CMakeLists.txt @@ -1,6 +1,7 @@ # define the test executable add_executable( cut_filament_tests + ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp ${CMAKE_SOURCE_DIR}/src/logic/cut_filament.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp diff --git a/tests/unit/logic/eject_filament/CMakeLists.txt b/tests/unit/logic/eject_filament/CMakeLists.txt index e347b1d..eab363d 100644 --- a/tests/unit/logic/eject_filament/CMakeLists.txt +++ b/tests/unit/logic/eject_filament/CMakeLists.txt @@ -1,6 +1,7 @@ # define the test executable add_executable( eject_filament_tests + ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp ${CMAKE_SOURCE_DIR}/src/logic/eject_filament.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp diff --git a/tests/unit/logic/load_filament/CMakeLists.txt b/tests/unit/logic/load_filament/CMakeLists.txt index 74e8951..e8f2fb9 100644 --- a/tests/unit/logic/load_filament/CMakeLists.txt +++ b/tests/unit/logic/load_filament/CMakeLists.txt @@ -1,6 +1,7 @@ # define the test executable add_executable( load_filament_tests + ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_bondtech.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp ${CMAKE_SOURCE_DIR}/src/logic/load_filament.cpp diff --git a/tests/unit/logic/stubs/stub_motion.cpp b/tests/unit/logic/stubs/stub_motion.cpp index 86cee4e..7c89e57 100644 --- a/tests/unit/logic/stubs/stub_motion.cpp +++ b/tests/unit/logic/stubs/stub_motion.cpp @@ -14,8 +14,9 @@ AxisSim axes[3] = { { -32767, -32767, false, false, false }, // idler }; -void Motion::InitAxis(Axis axis) { +bool Motion::InitAxis(Axis axis) { axes[axis].enabled = true; + return true; } void Motion::SetEnabled(Axis axis, bool enabled) { diff --git a/tests/unit/logic/tool_change/CMakeLists.txt b/tests/unit/logic/tool_change/CMakeLists.txt index 064f0f3..1b38c07 100644 --- a/tests/unit/logic/tool_change/CMakeLists.txt +++ b/tests/unit/logic/tool_change/CMakeLists.txt @@ -1,6 +1,7 @@ # define the test executable add_executable( tool_change_tests + ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_bondtech.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp ${CMAKE_SOURCE_DIR}/src/logic/load_filament.cpp diff --git a/tests/unit/logic/unload_filament/CMakeLists.txt b/tests/unit/logic/unload_filament/CMakeLists.txt index 01bf7f7..b3d0672 100644 --- a/tests/unit/logic/unload_filament/CMakeLists.txt +++ b/tests/unit/logic/unload_filament/CMakeLists.txt @@ -1,6 +1,7 @@ # define the test executable add_executable( unload_filament_tests + ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp ${CMAKE_SOURCE_DIR}/src/logic/unload_to_finda.cpp diff --git a/tests/unit/logic/unload_filament/test_unload_filament.cpp b/tests/unit/logic/unload_filament/test_unload_filament.cpp index 24da0be..f81aaa8 100644 --- a/tests/unit/logic/unload_filament/test_unload_filament.cpp +++ b/tests/unit/logic/unload_filament/test_unload_filament.cpp @@ -186,7 +186,7 @@ void FindaDidntTriggerResolveHelp(uint8_t slot, logic::UnloadFilament &uf) { hal::adc::SetADC(config::buttonsADCIndex, config::buttonADCLimits[0][0] + 1); while (!mb::buttons.ButtonPressed(0)) { main_loop(); - uf.Step(); + uf.StepInner(); } // we still think we have filament loaded at this stage @@ -270,7 +270,7 @@ void FindaDidntTriggerResolveTryAgain(uint8_t slot, logic::UnloadFilament &uf) { hal::adc::SetADC(config::buttonsADCIndex, config::buttonADCLimits[1][0] + 1); while (!mb::buttons.ButtonPressed(1)) { main_loop(); - uf.Step(); + uf.StepInner(); } // we still think we have filament loaded at this stage