Consolidated stepping engine code

Avoids the need to move Motor and Pin vtables into RAM, thus saving about 20K of precious RAM space. It also simplifies the structure of the stepping code dramatically, so it is all in one place instead of being distributed throughout a complex class hierarchy.
bdring · Oct 4, 2024 · 921f32b · 921f32b
1 parent c65c1d9
commit 921f32b
Show file tree

Hide file tree

Showing 31 changed files with 315 additions and 504 deletions.
diff --git a/FluidNC/ld/esp32/README.md b/FluidNC/ld/esp32/README.md
@@ -57,13 +57,6 @@ vtables from several code modules to be placed in RAM.  We use the
 INCLUDE technique to isolate the changes into one new file, for easier
 maintenance.
 
-For the wifi and noradio builds, the vtables are placed in IRAM because
-there is enough free space there, freeing up some DRAM to use as heap.
-For the bt build, bluetooth functions use so much IRAM that there is
-not enough room for the vtables, so they are placed into DRAM.  The
-bt version does not need as much heap as the wifi version (wifi needs
-quite a few large buffers for wifi packets).
-
 ### Making PlatformIO Use the Modified File
 
 To make PlatformIO use the modified sections.ld instead

diff --git a/FluidNC/ld/esp32/bt/sections.ld b/FluidNC/ld/esp32/bt/sections.ld
@@ -558,8 +558,6 @@ SECTIONS
     *libspi_flash.a:spi_flash_chip_winbond.*(.rodata .rodata.*)
     *libspi_flash.a:spi_flash_rom_patch.*(.rodata .rodata.*)
 
-    /* For the bluetooth build we must use DRAM for the ISR vtables */
-    /* because there is not enough space left in IRAM */
     INCLUDE ../vtable_in_dram.ld
 
     _data_end = ABSOLUTE(.);

diff --git a/FluidNC/ld/esp32/noradio/sections.ld b/FluidNC/ld/esp32/noradio/sections.ld
@@ -558,6 +558,8 @@ SECTIONS
     *libspi_flash.a:spi_flash_chip_winbond.*(.rodata .rodata.*)
     *libspi_flash.a:spi_flash_rom_patch.*(.rodata .rodata.*)
 
+    INCLUDE ../vtable_in_dram.ld
+
     _data_end = ABSOLUTE(.);
     . = ALIGN(4);
   } > dram0_0_seg
@@ -775,11 +777,6 @@ SECTIONS
     *(.iram2.coredump .iram2.coredump.*)
     _coredump_iram_end = ABSOLUTE(.);
 
-    /* For the wifi build we can use IRAM for the ISR vtables */
-    /* because there is enough space left there, conserving */
-    /* DRAM which is needed for extra heap space */
-    INCLUDE ../vtable_in_dram.ld
-
     _iram_data_end = ABSOLUTE(.);
 
   } > iram0_0_seg

diff --git a/FluidNC/ld/esp32/vtable_in_dram.ld b/FluidNC/ld/esp32/vtable_in_dram.ld
@@ -1,28 +1,8 @@
 /* List of files and sections to place in RAM instead of FLASH */
 /* See README.md in this directory for a complete explanation */
 
-    *Laser.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-
     /* All files whose name ends with Spindle.cpp */
-    **Spindle.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-
-    /* Motors */
-    *Motor.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *Dynamixel2.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *MotorDriver.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *NullMotor.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *RcServo.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *Servo.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *Solenoid.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *StandardStepper.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *StepStick.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *TMC*Driver.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *TrinamicBase.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *TrinamicSpiDriver.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *TrinamicUartDriver.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *UnipolarMotor.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
 
-    /* Pin Details */
-    **Detail.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *PwmPin.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
-    *Pin.cpp.o(.rodata .rodata.* .xt.prop .xt.prop.*)
+    /* An earlier version included .xt.prop and .xt.prop.* but */
+    /* it turns out that those bits can stay in FLASH */
+    **Spindle.cpp.o(.rodata .rodata.*)
diff --git a/FluidNC/ld/esp32/wifi/sections.ld b/FluidNC/ld/esp32/wifi/sections.ld
@@ -558,6 +558,8 @@ SECTIONS
     *libspi_flash.a:spi_flash_chip_winbond.*(.rodata .rodata.*)
     *libspi_flash.a:spi_flash_rom_patch.*(.rodata .rodata.*)
 
+    INCLUDE ../vtable_in_dram.ld
+
     _data_end = ABSOLUTE(.);
     . = ALIGN(4);
   } > dram0_0_seg
@@ -775,11 +777,6 @@ SECTIONS
     *(.iram2.coredump .iram2.coredump.*)
     _coredump_iram_end = ABSOLUTE(.);
 
-    /* For the wifi build we can use IRAM for the ISR vtables */
-    /* because there is enough space left there, conserving */
-    /* DRAM which is needed for extra heap space */
-    INCLUDE ../vtable_in_dram.ld
-
     _iram_data_end = ABSOLUTE(.);
 
   } > iram0_0_seg

diff --git a/FluidNC/src/Kinematics/Cartesian.cpp b/FluidNC/src/Kinematics/Cartesian.cpp
@@ -294,12 +294,11 @@ namespace Kinematics {
         auto n_axis = axes->_numberAxis;
         for (int axis = 0; axis < n_axis; axis++) {
             if (bitnum_is_true(axisMask, axis)) {
-                auto paxis = axes->_axis[axis];
                 if (bitnum_is_true(motors, Machine::Axes::motor_bit(axis, 0))) {
-                    paxis->_motors[0]->unlimit();
+                    Stepping::unlimit(axis, 0);
                 }
                 if (bitnum_is_true(motors, Machine::Axes::motor_bit(axis, 1))) {
-                    paxis->_motors[1]->unlimit();
+                    Stepping::unlimit(axis, 1);
                 }
             }
         }

diff --git a/FluidNC/src/Kinematics/CoreXY.cpp b/FluidNC/src/Kinematics/CoreXY.cpp
@@ -75,7 +75,7 @@ namespace Kinematics {
         auto n_axis = axes->_numberAxis;
         for (size_t axis = X_AXIS; axis < n_axis; axis++) {
             if (bitnum_is_true(axisMask, axis)) {
-                axes->_axis[axis]->_motors[0]->unlimit();
+                Stepping::unlimit(axis, 0);
             }
         }
     }

diff --git a/FluidNC/src/Machine/Axes.cpp b/FluidNC/src/Machine/Axes.cpp
@@ -93,9 +93,9 @@ namespace Machine {
                 auto a = _axis[axis];
                 if (a != nullptr) {
                     for (size_t motor = 0; motor < Axis::MAX_MOTORS_PER_AXIS; motor++) {
+                        Stepping::unblock(axis, motor);
                         auto m = _axis[axis]->_motors[motor];
                         if (m) {
-                            m->unblock();
                             if (m->_driver->set_homing_mode(isHoming)) {
                                 set_bitnum(motorsCanHome, motor_bit(axis, motor));
                             }
@@ -108,62 +108,6 @@ namespace Machine {
         return motorsCanHome;
     }
 
-    void IRAM_ATTR Axes::step(uint8_t step_mask, uint8_t dir_mask) {
-        auto n_axis = _numberAxis;
-        //log_info("motors_set_direction_pins:0x%02X", onMask);
-
-        // Set the direction pins, but optimize for the common
-        // situation where the direction bits haven't changed.
-        static uint8_t previous_dir = 255;  // should never be this value
-        if (dir_mask != previous_dir) {
-            previous_dir = dir_mask;
-
-            for (int axis = X_AXIS; axis < n_axis; axis++) {
-                bool thisDir = bitnum_is_true(dir_mask, axis);
-
-                for (size_t motor = 0; motor < Axis::MAX_MOTORS_PER_AXIS; motor++) {
-                    auto m = _axis[axis]->_motors[motor];
-                    if (m) {
-                        m->_driver->set_direction(thisDir);
-                    }
-                }
-            }
-            config->_stepping->waitDirection();
-        }
-
-        // Turn on step pulses for motors that are supposed to step now
-        for (size_t axis = X_AXIS; axis < n_axis; axis++) {
-            if (bitnum_is_true(step_mask, axis)) {
-                bool dir = bitnum_is_true(dir_mask, axis);
-
-                auto a = _axis[axis];
-                for (size_t motor = 0; motor < Axis::MAX_MOTORS_PER_AXIS; motor++) {
-                    auto m = a->_motors[motor];
-                    if (m) {
-                        m->step(dir);
-                    }
-                }
-            }
-        }
-        config->_stepping->startPulseTimer();
-    }
-
-    // Turn all stepper pins off
-    void IRAM_ATTR Axes::unstep() {
-        config->_stepping->waitPulse();
-        auto n_axis = _numberAxis;
-        for (size_t axis = X_AXIS; axis < n_axis; axis++) {
-            for (size_t motor = 0; motor < Axis::MAX_MOTORS_PER_AXIS; motor++) {
-                auto m = _axis[axis]->_motors[motor];
-                if (m) {
-                    m->_driver->unstep();
-                }
-            }
-        }
-
-        config->_stepping->finishPulse();
-    }
-
     void Axes::config_motors() {
         for (int axis = 0; axis < _numberAxis; ++axis) {
             _axis[axis]->config_motors();

diff --git a/FluidNC/src/Machine/Homing.cpp b/FluidNC/src/Machine/Homing.cpp
@@ -199,11 +199,11 @@ namespace Machine {
                     travel    = axisConfig->extraPulloff();
                     if (travel < 0) {
                         // Motor0's pulloff is greater than motor1's, so we block motor1
-                        axisConfig->_motors[1]->block();
+                        Stepping::block(axis, 1);
                         travel = -travel;
                     } else if (travel > 0) {
                         // Motor1's pulloff is greater than motor0's, so we block motor0
-                        axisConfig->_motors[0]->block();
+                        Stepping::block(axis, 0);
                     }
                     // All motors will be unblocked later by set_homing_mode()
                     break;
@@ -357,7 +357,7 @@ namespace Machine {
         gc_sync_position();
         plan_sync_position();
 
-        config->_stepping->endLowLatency();
+        Stepping::endLowLatency();
 
         if (!sys.abort) {
             set_state(unhomed_axes() ? State::Alarm : State::Idle);
@@ -521,7 +521,7 @@ namespace Machine {
             set_state(State::Alarm);
             return;
         }
-        config->_stepping->beginLowLatency();
+        Stepping::beginLowLatency();
 
         set_state(State::Homing);
         nextCycle();

diff --git a/FluidNC/src/Machine/LimitPin.cpp b/FluidNC/src/Machine/LimitPin.cpp
@@ -6,9 +6,9 @@
 #include "src/Protocol.h"  // protocol_send_event_from_ISR()
 
 namespace Machine {
-    LimitPin::LimitPin(Pin& pin, int axis, int motor, int direction, bool& pHardLimits, bool& pLimited) :
-        EventPin(&limitEvent, "Limit"), _axis(axis), _motorNum(motor), _value(false), _pHardLimits(pHardLimits), _pLimited(pLimited),
-        _pin(&pin) {
+    LimitPin::LimitPin(Pin& pin, int axis, int motor, int direction, bool& pHardLimits) :
+        EventPin(&limitEvent, "Limit"), _axis(axis), _motorNum(motor), _value(false), _pHardLimits(pHardLimits), _pin(&pin) {
+        _pLimited = Stepping::limit_var(axis, motor);
         const char* sDir;
         // Select one or two bitmask variables to receive the switch data
         switch (direction) {
@@ -56,7 +56,7 @@ namespace Machine {
     void LimitPin::update(bool value) {
         if (value) {
             if (Homing::approach() || (!state_is(State::Homing) && _pHardLimits)) {
-                _pLimited = value;
+                *_pLimited = value;
 
                 if (_pExtraLimited != nullptr) {
                     *_pExtraLimited = value;
@@ -70,7 +70,7 @@ namespace Machine {
                 set_bits(*_negLimits, _bitmask);
             }
         } else {
-            _pLimited = value;
+            *_pLimited = value;
 
             if (_pExtraLimited != nullptr) {
                 *_pExtraLimited = value;
@@ -92,6 +92,6 @@ namespace Machine {
     }
 
     void LimitPin::setExtraMotorLimit(int axis, int motorNum) {
-        _pExtraLimited = &config->_axes->_axis[axis]->_motors[motorNum]->_limited;
+        _pExtraLimited = Stepping::limit_var(axis, motorNum);
     }
 }
diff --git a/FluidNC/src/Machine/LimitPin.h b/FluidNC/src/Machine/LimitPin.h
@@ -19,7 +19,7 @@ namespace Machine {
         // touch, increasing the accuracy of homing
         // _pExtraLimited lets the limit control two motors, as with
         // CoreXY
-        volatile bool& _pLimited;
+        volatile bool* _pLimited;
         volatile bool* _pExtraLimited = nullptr;
 
         volatile uint32_t* _posLimits = nullptr;
@@ -28,7 +28,7 @@ namespace Machine {
         Pin* _pin;
 
     public:
-        LimitPin(Pin& pin, int axis, int motorNum, int direction, bool& phardLimits, bool& pLimited);
+        LimitPin(Pin& pin, int axis, int motorNum, int direction, bool& phardLimits);
 
         void update(bool value) override;
 

diff --git a/FluidNC/src/Machine/Motor.cpp b/FluidNC/src/Machine/Motor.cpp
@@ -31,15 +31,13 @@ namespace Machine {
         }
         _driver->init();
 
-        _negLimitPin = new LimitPin(_negPin, _axis, _motorNum, -1, _hardLimits, _limited);
-        _posLimitPin = new LimitPin(_posPin, _axis, _motorNum, 1, _hardLimits, _limited);
-        _allLimitPin = new LimitPin(_allPin, _axis, _motorNum, 0, _hardLimits, _limited);
+        _negLimitPin = new LimitPin(_negPin, _axis, _motorNum, -1, _hardLimits);
+        _posLimitPin = new LimitPin(_posPin, _axis, _motorNum, 1, _hardLimits);
+        _allLimitPin = new LimitPin(_allPin, _axis, _motorNum, 0, _hardLimits);
 
         _negLimitPin->init();
         _posLimitPin->init();
         _allLimitPin->init();
-
-        unblock();
     }
 
     void Motor::config_motor() {
@@ -71,21 +69,6 @@ namespace Machine {
         return _driver->isReal();
     }
 
-    void IRAM_ATTR Motor::step(bool reverse) {
-        // Skip steps based on limit pins
-        // _blocked is for asymmetric pulloff
-        // _limited is for limit pins
-        if (_blocked || _limited) {
-            return;
-        }
-        _driver->step();
-        _steps += reverse ? -1 : 1;
-    }
-
-    void IRAM_ATTR Motor::unstep() {
-        _driver->unstep();
-    }
-
     Motor::~Motor() {
         delete _driver;
     }

diff --git a/FluidNC/src/Machine/Motor.h b/FluidNC/src/Machine/Motor.h
@@ -35,10 +35,6 @@ namespace Machine {
         Pin  _allPin;
         bool _hardLimits = false;
 
-        int32_t _steps   = 0;
-        bool    _limited = false;  // _limited is set by the LimitPin ISR
-        bool    _blocked = false;  // _blocked is used during asymmetric homing pulloff
-
         // Configuration system helpers:
         void group(Configuration::HandlerBase& handler) override;
         void afterParse() override;
@@ -48,11 +44,6 @@ namespace Machine {
         void limitOtherAxis(int axis);
         void init();
         void config_motor();
-        void step(bool reverse);
-        void unstep();
-        void block() { _blocked = true; }
-        void unblock() { _blocked = false; }
-        void unlimit() { _limited = false; }
         ~Motor();
     };
 }
diff --git a/FluidNC/src/Motors/MotorDriver.cpp b/FluidNC/src/Motors/MotorDriver.cpp
@@ -38,14 +38,11 @@ namespace MotorDrivers {
 
     size_t MotorDriver::axis_index() const {
         Assert(config != nullptr && config->_axes != nullptr, "Expected machine to be configured before this is called.");
-        return size_t(config->_axes->findAxisIndex(this));
+        return size_t(Axes::findAxisIndex(this));
     }
     size_t MotorDriver::dual_axis_index() const {
         Assert(config != nullptr && config->_axes != nullptr, "Expected machine to be configured before this is called.");
-        return size_t(config->_axes->findAxisMotor(this));
+        return size_t(Axes::findAxisMotor(this));
     }
     void IRAM_ATTR MotorDriver::set_disable(bool disable) {}
-    void IRAM_ATTR MotorDriver::set_direction(bool) {}
-    void IRAM_ATTR MotorDriver::step() {}
-    void IRAM_ATTR MotorDriver::unstep() {}
 }
diff --git a/FluidNC/src/Motors/MotorDriver.h b/FluidNC/src/Motors/MotorDriver.h
@@ -64,22 +64,6 @@ namespace MotorDrivers {
         // make a motor transition between idle and non-idle states.
         virtual void set_disable(bool disable);
 
-        // set_direction() sets the motor movement direction.  It is
-        // invoked for every motion segment.
-        virtual void set_direction(bool);
-
-        // step() initiates a step operation on a motor.  It is called
-        // from motors_step() for ever motor than needs to step now.
-        // For ordinary step/direction motors, it sets the step pin
-        // to the active state.
-        virtual void step();
-
-        // unstep() turns off the step pin, if applicable, for a motor.
-        // It is called from motors_unstep() for all motors, since
-        // motors_unstep() is used in many contexts where the previous
-        // states of the step pins are unknown.
-        virtual void unstep();
-
         // this is used to configure and test motors. This would be used for Trinamic
         virtual void config_motor() {}