From 60482c9e8b3dbd067270b088629ccef5d21e25ae Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 1 Jul 2020 12:42:01 +0200 Subject: [PATCH 1/4] Fix ASCII protocol bug when writing to uint8_t When writing to uint8_t properties or equivalently sized enum properties, the ASCII protocol handler would write beyond the variable itself and overwrite adjacent memory. This manifested for instance in the following: r axis0.controller.config.input_mode 1 w axis0.controller.config.control_mode 3 r axis0.controller.config.input_mode 0 This boils down to what appears to be misbehavior of `sscanf`. This fix adds an intermediate union to provide a safe memory area for the `sscanf` call. --- Firmware/fibre/cpp/include/fibre/protocol.hpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Firmware/fibre/cpp/include/fibre/protocol.hpp b/Firmware/fibre/cpp/include/fibre/protocol.hpp index 89091f74..c1fb8d6d 100644 --- a/Firmware/fibre/cpp/include/fibre/protocol.hpp +++ b/Firmware/fibre/cpp/include/fibre/protocol.hpp @@ -591,7 +591,17 @@ static bool to_string(const T& value, char * buffer, size_t length, ...) { template::type> static bool from_string(const char * buffer, size_t length, T* property, int) { - return sscanf(buffer, format_traits_t::fmt, property) == 1; + // Note for T == uint8_t: Even though we supposedly use the correct format + // string sscanf treats our pointer as pointer-to-int instead of + // pointer-to-uint8_t. To avoid an unexpected memory access we first read + // into a union. + union { T t; int i; } val; + if (sscanf(buffer, format_traits_t::fmt, &val.t) == 1) { + *property = val.t; + return true; + } else { + return false; + } } // Special case for float because printf promotes float to double, and we get warnings template From 2571ab308d62ceedd0fd190420daa03be81dfe88 Mon Sep 17 00:00:00 2001 From: Cam Buss Date: Wed, 1 Jul 2020 14:50:25 -0600 Subject: [PATCH 2/4] Allow reversal of homing direction and ability to clear SPI Error Rate (#427) * clear spi_error_rate on error clear * negative homing speed and negative endtop offset --- Firmware/MotorControl/axis.cpp | 5 ++--- Firmware/MotorControl/axis.hpp | 1 + docs/endstops.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index d7659d35..6130f782 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -173,10 +173,9 @@ bool Axis::do_checks() { // controller_.do_checks(); // Check for endstop presses - bool vel_dependent_stopping = (current_state_ == AXIS_STATE_HOMING) && (controller_.config_.control_mode >= Controller::CTRL_MODE_VELOCITY_CONTROL); - if (min_endstop_.config_.enabled && min_endstop_.get_state() && (!vel_dependent_stopping || controller_.vel_setpoint_ < 0.0f)) { + if (min_endstop_.config_.enabled && min_endstop_.get_state() && !(current_state_ == AXIS_STATE_HOMING)) { error_ |= ERROR_MIN_ENDSTOP_PRESSED; - } else if (max_endstop_.config_.enabled && max_endstop_.get_state() && (!vel_dependent_stopping || controller_.vel_setpoint_ > 0.0f)) { + } else if (max_endstop_.config_.enabled && max_endstop_.get_state() && !(current_state_ == AXIS_STATE_HOMING)) { error_ |= ERROR_MAX_ENDSTOP_PRESSED; } diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index 6be3c60c..7335502c 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -143,6 +143,7 @@ public: controller_.error_ = Controller::ERROR_NONE; sensorless_estimator_.error_ = SensorlessEstimator::ERROR_NONE; encoder_.error_ = Encoder::ERROR_NONE; + encoder_.spi_error_rate_ = 0.0f; error_ = Axis::ERROR_NONE; } diff --git a/docs/endstops.md b/docs/endstops.md index 6379fb0b..0336b6dd 100644 --- a/docs/endstops.md +++ b/docs/endstops.md @@ -96,7 +96,7 @@ Name | Type | Default --- | -- | -- homing_speed | float | 2000.0f -`homing_speed` is the axis travel speed during homing, in counts/second. +`homing_speed` is the axis travel speed during homing, in counts/second. If you are using SPI based encoders and the axis is homing in the wrong direction, you can enter a negative value for the homing speed and a negative value for the minimum endstop offset. ### Performing the Homing Sequence From 9360c5df6571e1ec3055351b1d60a109d7f1600e Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 2 Jul 2020 21:34:51 -0400 Subject: [PATCH 3/4] Fix SPI DMA with different SPI transfer sizes --- Firmware/Board/v3/Src/spi.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Firmware/Board/v3/Src/spi.c b/Firmware/Board/v3/Src/spi.c index fe2ab4ea..a3a3311e 100644 --- a/Firmware/Board/v3/Src/spi.c +++ b/Firmware/Board/v3/Src/spi.c @@ -115,8 +115,15 @@ void HAL_SPI_MspInit(SPI_HandleTypeDef* spiHandle) hdma_spi3_tx.Init.Direction = DMA_MEMORY_TO_PERIPH; hdma_spi3_tx.Init.PeriphInc = DMA_PINC_DISABLE; hdma_spi3_tx.Init.MemInc = DMA_MINC_ENABLE; - hdma_spi3_tx.Init.PeriphDataAlignment = DMA_PDATAALIGN_HALFWORD; - hdma_spi3_tx.Init.MemDataAlignment = DMA_MDATAALIGN_HALFWORD; + + if(spiHandle->Init.DataSize == SPI_DATASIZE_8BIT){ + hdma_spi3_tx.Init.PeriphDataAlignment = DMA_PDATAALIGN_BYTE; + hdma_spi3_tx.Init.MemDataAlignment = DMA_MDATAALIGN_BYTE; + } else { + hdma_spi3_tx.Init.PeriphDataAlignment = DMA_PDATAALIGN_HALFWORD; + hdma_spi3_tx.Init.MemDataAlignment = DMA_MDATAALIGN_HALFWORD; + } + hdma_spi3_tx.Init.Mode = DMA_NORMAL; hdma_spi3_tx.Init.Priority = DMA_PRIORITY_MEDIUM; hdma_spi3_tx.Init.FIFOMode = DMA_FIFOMODE_DISABLE; @@ -133,8 +140,13 @@ void HAL_SPI_MspInit(SPI_HandleTypeDef* spiHandle) hdma_spi3_rx.Init.Direction = DMA_PERIPH_TO_MEMORY; hdma_spi3_rx.Init.PeriphInc = DMA_PINC_DISABLE; hdma_spi3_rx.Init.MemInc = DMA_MINC_ENABLE; - hdma_spi3_rx.Init.PeriphDataAlignment = DMA_PDATAALIGN_HALFWORD; - hdma_spi3_rx.Init.MemDataAlignment = DMA_MDATAALIGN_HALFWORD; + if (spiHandle->Init.DataSize == SPI_DATASIZE_8BIT) { + hdma_spi3_rx.Init.PeriphDataAlignment = DMA_PDATAALIGN_BYTE; + hdma_spi3_rx.Init.MemDataAlignment = DMA_MDATAALIGN_BYTE; + } else { + hdma_spi3_rx.Init.PeriphDataAlignment = DMA_PDATAALIGN_HALFWORD; + hdma_spi3_rx.Init.MemDataAlignment = DMA_MDATAALIGN_HALFWORD; + } hdma_spi3_rx.Init.Mode = DMA_NORMAL; hdma_spi3_rx.Init.Priority = DMA_PRIORITY_MEDIUM; hdma_spi3_rx.Init.FIFOMode = DMA_FIFOMODE_DISABLE; From ce7073baf525359df3ee8f950766955e85ad2895 Mon Sep 17 00:00:00 2001 From: Unknown Date: Wed, 8 Jul 2020 22:12:51 -0400 Subject: [PATCH 4/4] Fix ASCII 't' command --- Firmware/communication/ascii_protocol.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Firmware/communication/ascii_protocol.cpp b/Firmware/communication/ascii_protocol.cpp index f0ee9f9c..a5c5d623 100644 --- a/Firmware/communication/ascii_protocol.cpp +++ b/Firmware/communication/ascii_protocol.cpp @@ -170,7 +170,8 @@ void ASCII_protocol_process_line(const uint8_t* buffer, size_t len, StreamSink& } else { Axis* axis = axes[motor_number]; axis->controller_.config_.input_mode = Controller::INPUT_MODE_TRAP_TRAJ; - axis->controller_.move_to_pos(goal_point); + axis->controller_.input_pos_ = goal_point; + axis->controller_.input_pos_updated(); axis->watchdog_feed(); }