diff --git a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp index 72036de7..c09c4ef9 100644 --- a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp +++ b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp @@ -24,12 +24,42 @@ #include // This is global so it can be accessed from the override -Pin* csPinTMC; +namespace Motors { + namespace Details { + // How this hack works... In short, we just derive from the TMCStepper classes, and add the pin as a field. This is + // fine of course, as long as we know what CS pin we have to switch. Unfortunately, this is a bit tougher, because + // the TMC5160Stepper class derives from TMC2130Stepper. Normally, this is solved through multiple inheritance, where + // we put the pin in the second class. Unfortunately, that requires dynamic_cast down the road, which is not available. + // So instead, we use the old _pinCS in the TMC2130Stepper class to figure out which class we actually have, and cast + // away with static_cast. + + class MyTMC2130Stepper : public TMC2130Stepper { + public: + MyTMC2130Stepper(Pin& cspin, float rsense, int spiIndex) : TMC2130Stepper(2130, rsense, spiIndex), _cspin(cspin) {} + Pin& _cspin; + }; + + class MyTMC5160Stepper : public TMC5160Stepper { + public: + MyTMC5160Stepper(Pin& cspin, float rsense, int spiIndex) : TMC5160Stepper(5160, rsense, spiIndex), _cspin(cspin) {} + Pin& _cspin; + }; + } +} // Override default functions to use our pin framework void TMC2130Stepper::switchCSpin(bool state) { - csPinTMC->write(state); + // We use the _pinCS to figure out which derived class we have + switch (this->_pinCS) { + case 2130: + static_cast(this)->_cspin.write(state); + break; + case 5160: + static_cast(this)->_cspin.write(state); + break; + } } + void IRAM_ATTR pinMode(uint8_t pin, uint8_t mode) {} namespace Motors { @@ -49,18 +79,10 @@ namespace Motors { void TrinamicDriver::init() { _has_errors = false; - // This pin is "fake" because it is not used. The TMCStepper - // library calls switchCSpin() which uses the pin number that - // is passed to the constructor, but we override the weak - // definition of switchCSPin() to using our own pin framework. - // Similarly, the constructors call pinMode(), but we override - // that too. Thus the "fake" pin number is never used. - uint16_t fake_spi_cs_pin = 0; - if (_driver_part_number == 2130) { - tmcstepper = new TMC2130Stepper(fake_spi_cs_pin, _r_sense, _spi_index); + tmcstepper = new Details::MyTMC2130Stepper(_cs_pin, _r_sense, _spi_index); } else if (_driver_part_number == 5160) { - tmcstepper = new TMC5160Stepper(fake_spi_cs_pin, _r_sense, _spi_index); + tmcstepper = new Details::MyTMC5160Stepper(_cs_pin, _r_sense, _spi_index); } else { grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, @@ -75,8 +97,6 @@ namespace Motors { _cs_pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - // use slower speed if I2S if (_cs_pin.capabilities().has(Pin::Capabilities::I2S)) { tmcstepper->setSPISpeed(TRINAMIC_SPI_FREQ); @@ -107,8 +127,6 @@ namespace Motors { SPI.begin(sckPin, misoPin, mosiPin, ssPin); // this will get called for each motor, but does not seem to hurt anything - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - tmcstepper->begin(); _has_errors = !test(); // Try communicating with motor. Prints an error if there is a problem. @@ -157,8 +175,6 @@ namespace Motors { return false; } - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - switch (tmcstepper->test_connection()) { case 1: grbl_msg_sendf(CLIENT_SERIAL, @@ -226,8 +242,6 @@ o Read setting and send them to the driver. Called at init() and whenever rel hold_i_percent = 1.0; } - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - tmcstepper->microsteps(_microsteps); tmcstepper->rms_current(run_i_ma, hold_i_percent); @@ -249,8 +263,6 @@ o Read setting and send them to the driver. Called at init() and whenever rel return; } - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - TrinamicMode newMode = isHoming ? TRINAMIC_HOMING_MODE : TRINAMIC_RUN_MODE; if (newMode == _mode) { @@ -299,8 +311,6 @@ o Read setting and send them to the driver. Called at init() and whenever rel return; } - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - uint32_t tstep = tmcstepper->TSTEP(); if (tstep == 0xFFFFF || tstep < 1) { // if axis is not moving return @@ -356,8 +366,6 @@ o Read setting and send them to the driver. Called at init() and whenever rel return; } - csPinTMC = &_cs_pin; // Prepare for switchCSpin callback - _disabled = disable; _disable_pin.write(_disabled); diff --git a/Grbl_Esp32/src/Pin.cpp b/Grbl_Esp32/src/Pin.cpp index 3a370f7b..6702a53f 100644 --- a/Grbl_Esp32/src/Pin.cpp +++ b/Grbl_Esp32/src/Pin.cpp @@ -41,7 +41,7 @@ #endif Pins::PinDetail* Pin::undefinedPin = new Pins::VoidPinDetail(); -Pins::PinDetail* Pin::errorPin = new Pins::ErrorPinDetail(); +Pins::PinDetail* Pin::errorPin = new Pins::ErrorPinDetail(); bool Pin::parse(StringRange tmp, Pins::PinDetail*& pinImplementation) { String str = tmp.str(); @@ -168,14 +168,14 @@ bool Pin::validate(const String& str) { void Pin::report(const char* legend) { if (defined()) { - #if ESP32 +#if ESP32 grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "%s on %s", legend, name().c_str()); - #endif +#endif } } Pin::~Pin() { - if (_detail != undefinedPin) { + if (_detail != undefinedPin && _detail != errorPin) { delete _detail; } } diff --git a/Grbl_Esp32/src/Pin.h b/Grbl_Esp32/src/Pin.h index 9c4fb758..f45886f0 100644 --- a/Grbl_Esp32/src/Pin.h +++ b/Grbl_Esp32/src/Pin.h @@ -36,6 +36,23 @@ // Forward declarations: class String; +// Pin class. A pin is basically a thing that can 'output', 'input' or do both. GPIO on an ESP32 comes to mind, +// but there are way more possible pins. Think about I2S/I2C/SPI extenders, RS485 driven pin devices and even +// WiFi wall sockets. +// +// Normally people define a pin using the yaml configuration, and specify there how a pin should behave. For +// example, ':low' is normally supported, which means 'default low'. For output pins this is relevant, because +// it basically means 'off is a high (3.3V) signal, on is a low (GND) signal'. By doing it like this, there is +// no longer a need for invert flags, and things like that. +// +// Pins are supposed to be fields during the lifetime of the MachineConfig. In normal operations, the pin is +// created by the parser (using the 'create' methods), then kept in fields from the configurable, and eventually +// cleaned up by the destructor. Pins cannot be copied, there always has to be 1 owner of a pin, and they shouldn't +// be thrown around in the application. +// +// Pins internally use PinDetail classes. PinDetail's are just implementation details for a certain type of pin. +// PinDetail is not exposed to developers, because they should never be used directly. Pin class is your +// one-stop-go-to-shop for an pin. class Pin { // Helper for handling callbacks and mapping them to the proper class: template @@ -50,10 +67,15 @@ class Pin { static void IRAM_ATTR callback(void* /*ptr*/) { Callback(); } }; + // The undefined pin and error pin are two special pins. Error pins always throw an error when they are used. + // These are useful for unit testing, and for initializing pins that _always_ have to be defined by a user + // (or else). Undefined pins are basically pins with no functionality. They don't have to be defined, but also + // have no functionality when they are used. static Pins::PinDetail* undefinedPin; static Pins::PinDetail* errorPin; - Pins::PinDetail* _detail; + // Implementation details of this pin. + Pins::PinDetail* _detail; static bool parse(StringRange str, Pins::PinDetail*& detail); @@ -63,6 +85,7 @@ public: using Capabilities = Pins::PinCapabilities; using Attr = Pins::PinAttributes; + // A default pin is an undefined pin. inline Pin() : _detail(undefinedPin) {} static const bool On = true; @@ -70,11 +93,14 @@ public: // inline static Pins::PinDetail* create(const char* str) { return create(StringRange(str)); }; - static Pin create(const char* str) { return create(StringRange(str)); } // ensure it's not ambiguous + static Pin create(const char* str) { return create(StringRange(str)); } // ensure it's not ambiguous static Pin create(const StringRange& str); static Pin create(const String& str); static bool validate(const String& str); + // We delete the copy constructor, and implement the move constructor. The move constructor is required to support + // the correct execution of 'return' in f.ex. `create` calls. It basically transfers ownership from the callee to the + // caller of the Pin. inline Pin(const Pin& o) = delete; inline Pin(Pin&& o) : _detail(nullptr) { std::swap(_detail, o._detail); } @@ -84,13 +110,15 @@ public: return *this; } - // Some convenience operators: + // Some convenience operators and functions: inline bool operator==(const Pin& o) const { return _detail == o._detail; } inline bool operator!=(const Pin& o) const { return _detail != o._detail; } inline bool undefined() const { return _detail == undefinedPin; } inline bool defined() const { return !undefined(); } + // External libraries normally use digitalWrite, digitalRead and setMode. Since we cannot handle that behavior, we + // just give back the uint8_t for getNative. inline uint8_t getNative(Capabilities expectedBehavior) const { Assert(_detail->capabilities().has(expectedBehavior), "Requested pin does not have the expected behavior."); return _detail->_index;