From d9c76505c0539d4166960c30904192d17ac127db Mon Sep 17 00:00:00 2001 From: Stefan de Bruijn Date: Sat, 3 Jul 2021 21:36:16 +0200 Subject: [PATCH] Changed trinamic to use pinmapper [untested]. Fixed a few minor bugs. --- Grbl_Esp32/src/GCode.cpp | 2 +- Grbl_Esp32/src/Limits.cpp | 3 +- Grbl_Esp32/src/Motors/TrinamicDriver.cpp | 46 +++--------------------- Grbl_Esp32/src/Motors/TrinamicDriver.h | 2 ++ Grbl_Esp32/src/NutsBolts.cpp | 6 ---- Grbl_Esp32/src/NutsBolts.h | 2 -- Grbl_Esp32/src/Settings.cpp | 7 +++- Grbl_Esp32/src/System.cpp | 2 +- X86TestSupport/freertos/FreeRTOS.h | 21 ++--------- X86TestSupport/freertos/task.h | 3 +- 10 files changed, 20 insertions(+), 74 deletions(-) diff --git a/Grbl_Esp32/src/GCode.cpp b/Grbl_Esp32/src/GCode.cpp index e4dac43c..74eabc17 100644 --- a/Grbl_Esp32/src/GCode.cpp +++ b/Grbl_Esp32/src/GCode.cpp @@ -945,7 +945,7 @@ Error gc_execute_line(char* line, uint8_t client) { pValue = trunc(gc_block.values.p); // Convert p value to integer if (pValue > 0) { // P1 means G54, P2 means G55, etc. - coord_select = static_cast(pValue - 1 + CoordIndex::G54); + coord_select = static_cast(pValue - 1 + int(CoordIndex::G54)); } else { // P0 means use currently-selected system coord_select = gc_block.modal.coord_select; diff --git a/Grbl_Esp32/src/Limits.cpp b/Grbl_Esp32/src/Limits.cpp index f8152fbc..509b8c13 100644 --- a/Grbl_Esp32/src/Limits.cpp +++ b/Grbl_Esp32/src/Limits.cpp @@ -42,7 +42,8 @@ #include #include // IRAM_ATTR #include // memset, memcpy -#include +#include // min, max +#include // fence AxisMask limitAxes = 0; // Axes that have limit switches AxisMask homingAxes = 0; // Axes that have homing configured diff --git a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp index 901b5a32..efe5b1b0 100644 --- a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp +++ b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp @@ -25,43 +25,6 @@ #include // https://github.com/teemuatlut/TMCStepper #include -// This is global so it can be accessed from the override -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) { - // We use the _pinCS to figure out which derived class we have - switch (this->_pinCS) { - case 2130: - static_cast(this)->_cspin.synchronousWrite(state); - break; - case 5160: - static_cast(this)->_cspin.synchronousWrite(state); - break; - } -} - namespace Motors { TrinamicDriver::TrinamicDriver(uint16_t driver_part_number, int8_t spi_index) : TrinamicBase(driver_part_number), _spi_index(spi_index) {} @@ -69,10 +32,13 @@ namespace Motors { void TrinamicDriver::init() { _has_errors = false; + _cs_pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); + _cs_mapping = PinMapper(_cs_pin); + if (_driver_part_number == 2130) { - tmcstepper = new Details::MyTMC2130Stepper(_cs_pin, _r_sense, _spi_index); + tmcstepper = new TMC2130Stepper(_cs_mapping.pinId(), _r_sense, _spi_index); } else if (_driver_part_number == 5160) { - tmcstepper = new Details::MyTMC5160Stepper(_cs_pin, _r_sense, _spi_index); + tmcstepper = new TMC5160Stepper(_cs_mapping.pinId(), _r_sense, _spi_index); } else { info_serial("%s Unsupported Trinamic part number TMC%d", reportAxisNameMsg(axis_index(), dual_axis_index()), _driver_part_number); _has_errors = true; // This motor cannot be used @@ -81,8 +47,6 @@ namespace Motors { _has_errors = false; - _cs_pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); - // use slower speed if I2S if (_cs_pin.capabilities().has(Pin::Capabilities::I2S)) { tmcstepper->setSPISpeed(_spi_freq); diff --git a/Grbl_Esp32/src/Motors/TrinamicDriver.h b/Grbl_Esp32/src/Motors/TrinamicDriver.h index 431fa923..011c1ef7 100644 --- a/Grbl_Esp32/src/Motors/TrinamicDriver.h +++ b/Grbl_Esp32/src/Motors/TrinamicDriver.h @@ -21,6 +21,7 @@ #include "TrinamicBase.h" #include "../Pin.h" +#include "../PinMapper.h" #include @@ -40,6 +41,7 @@ namespace Motors { TMC2130Stepper* tmcstepper; // all other driver types are subclasses of this one Pin _cs_pin; // The chip select pin (can be the same for daisy chain) + PinMapper _cs_mapping; int8_t _spi_index; bool test(); diff --git a/Grbl_Esp32/src/NutsBolts.cpp b/Grbl_Esp32/src/NutsBolts.cpp index ea42f468..c779543c 100644 --- a/Grbl_Esp32/src/NutsBolts.cpp +++ b/Grbl_Esp32/src/NutsBolts.cpp @@ -240,9 +240,3 @@ char* trim(char* str) { end[1] = '\0'; return str; } - -// Returns the number of set number of set bits -// Would return 3 for 01100010 -int numberOfSetBits(uint32_t i) { - return __builtin_popcount(i); -} diff --git a/Grbl_Esp32/src/NutsBolts.h b/Grbl_Esp32/src/NutsBolts.h index 9cea7272..9928a6a8 100644 --- a/Grbl_Esp32/src/NutsBolts.h +++ b/Grbl_Esp32/src/NutsBolts.h @@ -112,8 +112,6 @@ float constrain_float(float in, float min, float max); bool char_is_numeric(char value); char* trim(char* value); -int numberOfSetBits(uint32_t i); - template void swap(T& a, T& b) { T c(a); diff --git a/Grbl_Esp32/src/Settings.cpp b/Grbl_Esp32/src/Settings.cpp index c50f07ee..4df84e57 100644 --- a/Grbl_Esp32/src/Settings.cpp +++ b/Grbl_Esp32/src/Settings.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #ifdef ENABLE_WIFI @@ -211,7 +212,11 @@ void StringSetting::load() { _currentValue = _defaultValue; return; } - char buf[len]; + + // TODO: Can't we allocate the string immediately? + std::vector buffer; + buffer.resize(len); + char* buf = buffer.data(); err = nvs_get_str(_handle, _keyName, buf, &len); if (err) { _storedValue = _defaultValue; diff --git a/Grbl_Esp32/src/System.cpp b/Grbl_Esp32/src/System.cpp index e3c0ad7b..92c9779c 100644 --- a/Grbl_Esp32/src/System.cpp +++ b/Grbl_Esp32/src/System.cpp @@ -106,7 +106,7 @@ float system_convert_axis_steps_to_mpos(int32_t* steps, uint8_t idx) { void system_convert_array_steps_to_mpos(float* position, int32_t* steps) { auto n_axis = config->_axes->_numberAxis; - float motors[n_axis]; + float motors[MAX_N_AXIS]; for (int idx = 0; idx < n_axis; idx++) { motors[idx] = (float)steps[idx] / config->_axes->_axis[idx]->_stepsPerMm; } diff --git a/X86TestSupport/freertos/FreeRTOS.h b/X86TestSupport/freertos/FreeRTOS.h index 2a6f337b..3add1e28 100644 --- a/X86TestSupport/freertos/FreeRTOS.h +++ b/X86TestSupport/freertos/FreeRTOS.h @@ -2,27 +2,10 @@ #include "Task.h" #include "Queue.h" +#include /* "mux" data structure (spinlock) */ -typedef struct { - /* owner field values: - * 0 - Uninitialized (invalid) - * portMUX_FREE_VAL - Mux is free, can be locked by either CPU - * CORE_ID_PRO / CORE_ID_APP - Mux is locked to the particular core - * - * Any value other than portMUX_FREE_VAL, CORE_ID_PRO, CORE_ID_APP indicates corruption - */ - uint32_t owner; - /* count field: - * If mux is unlocked, count should be zero. - * If mux is locked, count is non-zero & represents the number of recursive locks on the mux. - */ - uint32_t count; -#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG - const char* lastLockedFn; - int lastLockedLine; -#endif -} portMUX_TYPE; +using portMUX_TYPE = std::mutex; #define portMAX_DELAY (TickType_t)0xffffffffUL diff --git a/X86TestSupport/freertos/task.h b/X86TestSupport/freertos/task.h index b66247f4..598e4665 100644 --- a/X86TestSupport/freertos/task.h +++ b/X86TestSupport/freertos/task.h @@ -51,8 +51,7 @@ TickType_t xTaskGetTickCount(void); // Keep this in sync with the portMUX_TYPE struct definition please. #ifndef CONFIG_FREERTOS_PORTMUX_DEBUG -# define portMUX_INITIALIZER_UNLOCKED \ - { .owner = portMUX_FREE_VAL, .count = 0, } +# define portMUX_INITIALIZER_UNLOCKED {} #else # define portMUX_INITIALIZER_UNLOCKED \ { .owner = portMUX_FREE_VAL, .count = 0, .lastLockedFn = "(never locked)", .lastLockedLine = -1 }