Introduce bowden length RW register (no runtime autotune)

contains updated unit tests

src/config/config.h

@@ -18,7 +18,10 @@
 /// Wrangler for assorted compile-time configuration and constants.
 namespace config {
 
-static constexpr const uint8_t toolCount = 5U; ///< Max number of extruders/tools/slots
+/// Max number of extruders/tools/slots
+/// Beware - if you change this, the EEPROM structure will become invalid and no migration procedures have been implemented.
+/// So if you really have to change this, erase your EEPROM content then.
+static constexpr const uint8_t toolCount = 5U;
 static_assert(toolCount < 15, "Up to 14 valid slots (+1 parking) is supported in EEPROM storage");
 
 // Printer's filament sensor setup
@@ -92,9 +95,9 @@ static constexpr U_mm couplerToBowden = 3.5_mm; /// FINDA Coupler screw to bowde
 // just another piece of PLA (probably having more resistance in the tubes)
 // and we are at least 40mm off! It looks like this really depends on the exact position
 // We'll probably need to check for StallGuard while pushing the filament to avoid ginding the filament
-static constexpr U_mm defaultBowdenLength = 427.0_mm; ///< ~427.0_mm - Default Bowden length. TODO Should be stored in EEPROM. 392 a 784
+static constexpr U_mm defaultBowdenLength = 427.0_mm; /// ~427.0_mm - Default Bowden length.
-static constexpr U_mm minimumBowdenLength = 341.0_mm; ///< ~341.0_mm - Minimum bowden length. TODO Should be stored in EEPROM.
+static constexpr U_mm minimumBowdenLength = 341.0_mm; /// ~341.0_mm - Minimum bowden length.
-static constexpr U_mm maximumBowdenLength = 792.0_mm; ///< ~792.0_mm - Maximum bowden length. TODO Should be stored in EEPROM.
+static constexpr U_mm maximumBowdenLength = 792.0_mm; /// ~792.0_mm - Maximum bowden length.
 static constexpr U_mm feedToFinda = cuttingEdgeToFindaMidpoint + filamentMinLoadedToMMU;
 static constexpr U_mm maximumFeedToFinda = feedToFinda + 20.0_mm; ///< allow for some safety margin to load to FINDA
 static constexpr U_mm pulleyHelperMove = 10.0_mm; ///< Helper move for Load/Unload error states - when the MMU should slowly move the filament a bit

src/hal/avr/eeprom.cpp

@@ -20,5 +20,13 @@ void EEPROM::UpdateByte(EEPROM::addr_t addr, uint8_t value) {
     eeprom_update_byte((uint8_t *)addr, value);
 }
 
+uint16_t EEPROM::ReadWord(EEPROM::addr_t addr) {
+    return eeprom_read_word((const uint16_t *)addr);
+}
+
+void EEPROM::UpdateWord(EEPROM::addr_t addr, uint16_t value) {
+    eeprom_update_word((uint16_t *)addr, value);
+}
+
 } // namespace eeprom
 } // namespace hal

src/hal/eeprom.h

@@ -1,6 +1,7 @@
 /// @file eeprom.h
 #pragma once
 #include <stdint.h>
+#include <stddef.h>
 
 namespace hal {
 
@@ -9,7 +10,11 @@ namespace eeprom {
 
 class EEPROM {
 public:
+#ifdef UNITTEST
+    using addr_t = size_t;
+#else
     using addr_t = uint16_t;
+#endif
 
     static void WriteByte(addr_t addr, uint8_t value);
     static void UpdateByte(addr_t addr, uint8_t value);

src/logic/feed_to_bondtech.cpp

@@ -9,6 +9,7 @@
 #include "../modules/permanent_storage.h"
 #include "../modules/pulley.h"
 #include "../debug.h"
+#include "../config/axis.h"
 
 namespace logic {
 
@@ -23,7 +24,7 @@ void FeedToBondtech::Reset(uint8_t maxRetries) {
 void logic::FeedToBondtech::GoToPushToNozzle() {
     mg::globals.SetFilamentLoaded(mg::globals.ActiveSlot(), mg::FilamentLoadState::InFSensor);
     // plan a slow move to help push filament into the nozzle
-    //@@TODO the speed in mm/s must correspond to printer's feeding speed!
+    // the speed in mm/s must correspond to printer's feeding speed!
     mpu::pulley.PlanMove(mg::globals.FSensorToNozzle_mm(), mg::globals.PulleySlowFeedrate_mm_s());
     state = PushingFilamentIntoNozzle;
 }
@@ -37,11 +38,13 @@ bool FeedToBondtech::Step() {
             state = PushingFilamentFast;
             mpu::pulley.InitAxis();
             // plan a fast move while in the safe minimal length
-            mpu::pulley.PlanMove(config::minimumBowdenLength,
+            // fast feed in millimeters - if the EEPROM value is incorrect, we'll get the default length
+            unit::U_mm fastFeedDistance = { (long double)mps::BowdenLength::Get() };
+            mpu::pulley.PlanMove(fastFeedDistance,
                 mg::globals.PulleyLoadFeedrate_mm_s(),
                 mg::globals.PulleySlowFeedrate_mm_s());
             // plan additional slow move while waiting for fsensor to trigger
-            mpu::pulley.PlanMove(config::maximumBowdenLength - config::minimumBowdenLength,
+            mpu::pulley.PlanMove(config::maximumBowdenLength - fastFeedDistance,
                 mg::globals.PulleySlowFeedrate_mm_s(),
                 mg::globals.PulleySlowFeedrate_mm_s());
         }

src/logic/feed_to_bondtech.h

@@ -1,6 +1,7 @@
 /// @file feed_to_bondtech.h
 #pragma once
 #include <stdint.h>
+#include "../modules/axisunit.h"
 
 namespace logic {
 
@@ -10,6 +11,10 @@ namespace logic {
 /// Then it feeds a bit more very gently to push the filament into the nozzle
 /// Disengages the Idler after finishing the feed.
 /// Disables the Pulley axis after disengaging the idler.
+///
+/// If filament has been successfully fed into the fsensor,
+/// records/updates PTFE length.
+/// To prevent constant EEPROM updates only significant changes are recorded.
 struct FeedToBondtech {
     /// internal states of the state machine
     enum {

src/modules/permanent_storage.cpp

@@ -3,6 +3,7 @@
 #include "../hal/eeprom.h"
 #include "globals.h"
 #include "../config/config.h"
+#include "axisunit.h"
 
 #include <stddef.h>
 
@@ -19,13 +20,14 @@ namespace permanent_storage {
 /// Last byte in EEPROM is reserved for layoutVersion. If some field is repurposed, layoutVersion
 /// needs to be changed to force an EEPROM erase.
 struct eeprom_t {
-    uint8_t eepromLengthCorrection; ///< Legacy bowden length correction
+    uint8_t eepromLengthCorrection; ///< pre-MMU2 Legacy bowden length correction - not used
-    uint16_t eepromBowdenLen[config::toolCount]; ///< Bowden length for each filament
+    uint16_t eepromBowdenLen[config::toolCount]; ///< MMU2 Bowden length for each filament - not used
     uint8_t eepromFilamentStatus[3]; ///< Majority vote status of eepromFilament wear leveling
     uint8_t eepromFilament[800]; ///< Top nibble status, bottom nibble last filament loaded
     uint8_t eepromDriveErrorCountH;
     uint8_t eepromDriveErrorCountL[2];
     uint8_t sg_thrs[3];
+    uint16_t bowdenLengthMM; ///< MMU3 default bowden length in millimeters
 } __attribute__((packed));
 
 static_assert(sizeof(eeprom_t) - 2 <= hal::eeprom::EEPROM::End(), "eeprom_t doesn't fit into EEPROM available.");
@@ -44,14 +46,14 @@ static const uint8_t layoutVersion = 0xff;
 // ideally, this would have been a nice constexpr (since it is a compile time constant), but the C++ standard prohibits reinterpret_casts in constexpr
 static eeprom_t *const eepromBase = reinterpret_cast<eeprom_t *>(0); ///< First EEPROM address
 constexpr const uint16_t eepromEmpty = 0xffffU; ///< EEPROM content when erased
-constexpr const uint16_t eepromLengthCorrectionBase = 7900U; ///< legacy bowden length correction base (~391mm)
+constexpr const uint16_t eepromLengthCorrectionBase = config::defaultBowdenLength.v;
-constexpr const uint16_t eepromBowdenLenDefault = 8900U; ///< Default bowden length (~427 mm)
+constexpr const uint16_t eepromBowdenLenDefault = config::defaultBowdenLength.v; ///< Default bowden length (~427 mm)
-constexpr const uint16_t eepromBowdenLenMinimum = 6900U; ///< Minimum bowden length (~341 mm)
+constexpr const uint16_t eepromBowdenLenMinimum = config::minimumBowdenLength.v; ///< Minimum bowden length (~341 mm)
-constexpr const uint16_t eepromBowdenLenMaximum = 16000U; ///< Maximum bowden length (~792 mm)
+constexpr const uint16_t eepromBowdenLenMaximum = config::maximumBowdenLength.v; ///< Maximum bowden length (~792 mm)
 
 namespace ee = hal::eeprom;
 
-#define EEOFFSET(x) reinterpret_cast<size_t>(&(x))
+#define EEOFFSET(x) reinterpret_cast<ee::EEPROM::addr_t>(&(x))
 
 void Init() {
     if (ee::EEPROM::ReadByte(ee::EEPROM::End()) != layoutVersion) {
@@ -66,88 +68,22 @@ void EraseAll() {
     ee::EEPROM::UpdateByte(ee::EEPROM::End(), layoutVersion);
 }
 
-/// @brief Is filament number valid?
-/// @retval true valid
-/// @retval false invalid
-static bool validFilament(uint8_t filament) {
-    return filament < ARR_SIZE(eeprom_t::eepromBowdenLen);
-}
-
 /// @brief Is bowden length in valid range?
 /// @param BowdenLength bowden length
 /// @retval true valid
 /// @retval false invalid
-static bool validBowdenLen(const uint16_t BowdenLength) {
+static constexpr bool validBowdenLen(const uint16_t BowdenLength) {
-    if ((BowdenLength >= eepromBowdenLenMinimum)
+    return ((BowdenLength >= eepromBowdenLenMinimum)
-        && BowdenLength <= eepromBowdenLenMaximum) {
+        && BowdenLength <= eepromBowdenLenMaximum);
-        return true;
-    }
-    return false;
 }
 
-/// @brief Get bowden length for active filament
+uint16_t BowdenLength::Get() {
-///
+    uint16_t bl = ee::EEPROM::ReadWord(EEOFFSET(eepromBase->bowdenLengthMM));
-/// Returns stored value, doesn't return actual value when it is edited by increase() / decrease() unless it is stored.
+    return validBowdenLen(bl) ? bl : eepromBowdenLenDefault;
-/// @return stored bowden length
-uint16_t BowdenLength::get() {
-    uint8_t filament = mg::globals.ActiveSlot();
-    if (validFilament(filament)) {
-        // @@TODO these reinterpret_cast expressions look horrible but I'm keeping them almost intact to respect the original code from MM_control_01
-        uint16_t bowdenLength = ee::EEPROM::ReadByte(reinterpret_cast<size_t>(&(eepromBase->eepromBowdenLen[filament])));
-
-        if (eepromEmpty == bowdenLength) {
-            const uint8_t LengthCorrectionLegacy = ee::EEPROM::ReadByte(reinterpret_cast<size_t>(&(eepromBase->eepromLengthCorrection)));
-            if (LengthCorrectionLegacy <= 200) {
-                bowdenLength = eepromLengthCorrectionBase + LengthCorrectionLegacy * 10;
-            }
-        }
-        if (validBowdenLen(bowdenLength))
-            return bowdenLength;
 }
 
-    return eepromBowdenLenDefault;
+void BowdenLength::Set(uint16_t mm) {
-}
+    ee::EEPROM::UpdateWord(EEOFFSET(eepromBase->bowdenLengthMM), mm);
-
-/// @brief Construct BowdenLength object which allows bowden length manipulation
-///
-/// To be created on stack, new value is permanently stored when object goes out of scope.
-/// Active filament and associated bowden length is stored in member variables.
-BowdenLength::BowdenLength()
-    : filament(mg::globals.ActiveSlot()) // @@TODO - verify correct initialization order
-    , length(BowdenLength::get()) // @@TODO
-{
-}
-
-/// @brief Increase bowden length
-///
-/// New value is not stored immediately. See ~BowdenLength() for storing permanently.
-/// @retval true passed
-/// @retval false failed, it is not possible to increase, new bowden length would be out of range
-bool BowdenLength::increase() {
-    if (validBowdenLen(length + stepSize)) {
-        length += stepSize;
-        return true;
-    }
-    return false;
-}
-
-/// @brief Decrease bowden length
-///
-/// New value is not stored immediately. See ~BowdenLength() for storing permanently.
-/// @retval true passed
-/// @retval false failed, it is not possible to decrease, new bowden length would be out of range
-bool BowdenLength::decrease() {
-    if (validBowdenLen(length - stepSize)) {
-        length -= stepSize;
-        return true;
-    }
-    return false;
-}
-
-/// @brief Store bowden length permanently.
-BowdenLength::~BowdenLength() {
-    if (validFilament(filament))
-        ee::EEPROM::UpdateWord(EEOFFSET(eepromBase->eepromBowdenLen[filament]), length);
 }
 
 /// @brief Get filament storage status

src/modules/permanent_storage.h

@@ -17,22 +17,21 @@ void Init();
 /// Erase the whole EEPROM
 void EraseAll();
 
-/// @brief Read manipulate and store bowden length
+/// @brief Read and store bowden length
 ///
-/// Value is stored independently for each filament.
+/// Legacy functions for setting bowden length for each slot have been removed (we are out of space).
-/// Active filament is deduced from active_extruder global variable.
+/// It doesn't look to be practical to set bowden lengths for per slot anymore,
+/// because the filament loading algoritm is much more intelligent than in FW 1.0.6
+/// and slight differences in pulley gear diameters will be hard to spot at runtime.
+/// Moreover, the FW now contains autotuning of the bowden length,
+/// so the user should not need to set the register in any way.
 class BowdenLength {
 public:
-    static uint16_t get();
+    /// @returns default bowden length in millimeters
-    static const uint8_t stepSize = 10u; ///< increase()/decrease() bowden length step size
+    static uint16_t Get();
-    BowdenLength();
-    bool increase();
-    bool decrease();
-    ~BowdenLength();
 
-private:
+    /// Sets
-    uint8_t filament; ///< Selected filament
+    static void Set(uint16_t mm);
-    uint16_t length; ///< Selected filament bowden length
 };
 
 /// @brief Read and store last filament loaded to nozzle

src/registers.cpp

@@ -165,6 +165,7 @@
 | 0x1fh 31 | uint16  |Set/Get Selector iRun current| 0-31         | 1fh 31      | 31->530mA: see TMC2130 current conversion| Read / Write | M707 A0x1f | M708 A0x1f Xn
 | 0x20h 32 | uint16   | Set/Get Idler iRun current | 0-31         | 1fh 31      | 31->530mA: see TMC2130 current conversion| Read / Write | M707 A0x20 | M708 A0x20 Xn
 | 0x21h 33 | uint16   | Reserved for internal use  | 225          |             | N/A                                      | N/A          | N/A        | N/A
+| 0x22h 34 | uint16   | Bowden length              | 155h 341     | 318h 792    | unit mm                                  | Read / Write | M707 A0x22 | M708 A0x22 Xn
 */
 
 struct __attribute__((packed)) RegisterFlags {
@@ -287,7 +288,7 @@ static const RegisterRec registers[] PROGMEM = {
     RegisterRec(false, &project_build_number),
     // 0x04
     RegisterRec( // MMU errors
-        []() -> uint16_t { return mg::globals.DriveErrors(); },
+        []() -> uint16_t { return mg::globals.DriveErrors(); }, // compiles to: <{lambda()#1}::_FUN()>: jmp <modules::permanent_storage::DriveError::get()>
         // [](uint16_t) {}, // @@TODO think about setting/clearing the error counter from the outside
         2),
     // 0x05
@@ -430,6 +431,12 @@ static const RegisterRec registers[] PROGMEM = {
     RegisterRec(
         []() -> uint16_t { return 225; /*mv::vcc.CurrentBandgapVoltage();*/ },
         2),
+
+    // 0x22 Detected bowden length R
+    RegisterRec(
+        []() -> uint16_t { return mps::BowdenLength::Get(); },
+        [](uint16_t d) { mps::BowdenLength::Set(d); },
+        2),
 };
 
 static constexpr uint8_t registersSize = sizeof(registers) / sizeof(RegisterRec);

tests/unit/logic/feed_to_bondtech/test_feed_to_bondtech.cpp

@@ -27,6 +27,11 @@ TEST_CASE("feed_to_bondtech::feed_phase_unlimited", "[feed_to_bondtech]") {
     ForceReinitAllAutomata();
     REQUIRE(EnsureActiveSlotIndex(slot, mg::FilamentLoadState::AtPulley));
 
+    // reset bowden lenghts in EEPROM
+    SetMinimalBowdenLength();
+    // check bowden lengths
+    REQUIRE(mps::BowdenLength::Get() == config::minimumBowdenLength.v);
+
     FeedToBondtech fb;
     main_loop();
 
@@ -57,20 +62,23 @@ TEST_CASE("feed_to_bondtech::feed_phase_unlimited", "[feed_to_bondtech]") {
     REQUIRE(ml::leds.Mode(mg::globals.ActiveSlot(), ml::green) == ml::blink0);
 
     // fast load - no fsensor trigger
+    // performs fast load for config::minimumBowdenLength distance
     REQUIRE(WhileCondition(
         fb,
         [&](uint32_t) { return fb.State() == FeedToBondtech::PushingFilamentFast; },
         mm::unitToSteps<mm::P_pos_t>(config::minimumBowdenLength) + 2));
 
     // slow load - expecting fsensor trigger
+    // This gets interesting with bowden length autotuning - we should trigger at the right step
+    constexpr uint32_t additionalBowdenLengthTrigger = mm::unitToSteps<mm::P_pos_t>(config::defaultBowdenLength - config::minimumBowdenLength);
     REQUIRE(WhileCondition(
         fb,
         [&](uint32_t step) {
-        if( step == 100 ){
+        if( step == additionalBowdenLengthTrigger ){
             mfs::fsensor.ProcessMessage(true);
         }
         return fb.State() == FeedToBondtech::PushingFilamentToFSensor; },
-        1500));
+        additionalBowdenLengthTrigger + 5));
 
     REQUIRE(mfs::fsensor.Pressed());
 
@@ -107,5 +115,11 @@ TEST_CASE("feed_to_bondtech::feed_phase_unlimited", "[feed_to_bondtech]") {
     REQUIRE(fb.State() == FeedToBondtech::OK);
     REQUIRE(ml::leds.LedOn(mg::globals.ActiveSlot(), ml::green));
 
+    // detected bowden length is expected still to be 341, not runtime detection available
+    uint16_t bowdenLength = mps::BowdenLength::Get();
+    CHECK(bowdenLength == 341);
+    // must be within the specified tolerance of 10mm from the default bowden length
+    REQUIRE(abs(bowdenLength - config::minimumBowdenLength.v) < 10);
+
     REQUIRE(fb.Step() == true); // the automaton finished its work, any consecutive calls to Step must return true
 }

tests/unit/logic/stubs/main_loop_stub.cpp

@@ -165,6 +165,11 @@ void ClearButtons(logic::CommandBase &cb) {
     }
 }
 
+void SetMinimalBowdenLength() {
+    // reset bowdenLenght in EEPROM
+    mps::BowdenLength::Set(config::minimumBowdenLength.v);
+}
+
 void SetFSensorStateAndDebounce(bool press) {
     mfs::fsensor.ProcessMessage(press);
     for (uint8_t fs = 0; fs < config::fsensorDebounceMs + 1; ++fs) {

tests/unit/logic/stubs/main_loop_stub.h

@@ -32,6 +32,7 @@ bool SimulateRetractFromFINDA(uint32_t step, uint32_t findaOff);
 void PressButtonAndDebounce(logic::CommandBase &cb, uint8_t btnIndex, bool fromPrinter);
 void ClearButtons(logic::CommandBase &cb);
 
+void SetMinimalBowdenLength();
 void SetFSensorStateAndDebounce(bool press);
 
 // these are recommended max steps for simulated movement of the idler and selector

tests/unit/logic/tool_change/test_tool_change.cpp

@@ -79,6 +79,7 @@ bool SimulateUnloadFilament(uint32_t step, const logic::CommandBase *tc, uint32_
 
 void ToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
     SetFINDAStateAndDebounce(true);
@@ -104,6 +105,7 @@ void ToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
 
 void NoToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
     // the filament is LOADED
@@ -123,6 +125,7 @@ void NoToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
 void JustLoadFilament(logic::ToolChange &tc, uint8_t slot) {
     for (uint8_t startSelectorSlot = 0; startSelectorSlot < config::toolCount; ++startSelectorSlot) {
         ForceReinitAllAutomata();
+        SetMinimalBowdenLength();
         // make sure all the modules are ready
         // MMU-196: Move selector to a "random" slot
         REQUIRE(EnsureActiveSlotIndex(startSelectorSlot, mg::FilamentLoadState::AtPulley));
@@ -188,6 +191,7 @@ TEST_CASE("tool_change::same_slot_just_unloaded_filament", "[tool_change]") {
 
 void ToolChangeFailLoadToFinda(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
     SetFINDAStateAndDebounce(true);
@@ -315,6 +319,7 @@ TEST_CASE("tool_change::load_fail_FINDA_resolve_btnM", "[tool_change]") {
 void ToolChangeFailFSensor(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     using namespace std::placeholders;
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
     SetFINDAStateAndDebounce(true);
@@ -397,6 +402,7 @@ TEST_CASE("tool_change::load_fail_FSensor_resolve_btnM", "[tool_change]") {
 
 void ToolChangeWithFlickeringFINDA(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot, bool keepFindaPressed) {
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
     SetFINDAStateAndDebounce(true);
@@ -495,6 +501,7 @@ TEST_CASE("tool_change::test_flickering_FINDA_keepPressed", "[tool_change]") {
 
 void ToolChangeFSENSOR_TOO_EARLY(logic::ToolChange &tc, uint8_t slot) {
     ForceReinitAllAutomata();
+    SetMinimalBowdenLength();
     REQUIRE(EnsureActiveSlotIndex(slot, mg::FilamentLoadState::AtPulley));
 
     // verify filament NOT loaded