From b794178f0e4ec7efcd2c57ff65187b77e0707ed8 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Tue, 30 Jun 2020 15:07:21 +0200 Subject: [PATCH 1/4] fix HWIL tests --- tools/odrive/tests/closed_loop_test.py | 5 +++-- tools/odrive/tests/uart_ascii_test.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/odrive/tests/closed_loop_test.py b/tools/odrive/tests/closed_loop_test.py index 31adce5d..a06f75e4 100644 --- a/tools/odrive/tests/closed_loop_test.py +++ b/tools/odrive/tests/closed_loop_test.py @@ -177,7 +177,7 @@ class TestRegenProtection(TestClosedLoopControlBase): max_current = 15.0 # Accept a bit of noise on Ibus - axis_ctx.parent.handle.config.dc_max_negative_current = -0.2 + axis_ctx.parent.handle.config.dc_max_negative_current = -0.5 logger.debug(f'Brake control test from {nominal_rps} rounds/s...') @@ -227,6 +227,7 @@ class TestVelLimitInTorqueControl(TestClosedLoopControlBase): axis_ctx.handle.controller.config.vel_gain /= 10 # reduce the slope to make it easier to see what's going on vel_gain = axis_ctx.handle.controller.config.vel_gain + direction = axis_ctx.handle.motor.config.direction logger.debug(f'vel gain is {vel_gain}') axis_ctx.handle.controller.config.vel_limit = max_vel @@ -237,7 +238,7 @@ class TestVelLimitInTorqueControl(TestClosedLoopControlBase): # Returns the expected limited setpoint for a given velocity and current def get_expected_setpoint(input_setpoint, velocity): - return clamp(clamp(input_setpoint / torque_constant, (velocity + max_vel) * -vel_gain / torque_constant, (velocity - max_vel) * -vel_gain / torque_constant), -max_current, max_current) + return clamp(clamp(input_setpoint / torque_constant, (velocity + max_vel) * -vel_gain / torque_constant, (velocity - max_vel) * -vel_gain / torque_constant), -max_current, max_current) * direction def data_getter(): # sample velocity twice to avoid systematic bias diff --git a/tools/odrive/tests/uart_ascii_test.py b/tools/odrive/tests/uart_ascii_test.py index 6c650e28..ff5a41de 100644 --- a/tools/odrive/tests/uart_ascii_test.py +++ b/tools/odrive/tests/uart_ascii_test.py @@ -105,7 +105,7 @@ class TestUartAscii(): ser.write(b'c 0 12.5\n') test_assert_eq(ser.readline(), b'') test_assert_eq(odrive.handle.axis0.controller.input_torque, 12.5, accuracy=0.001) - test_assert_eq(odrive.handle.axis0.controller.config.control_mode, CONTROL_MODE_CURRENT_CONTROL) + test_assert_eq(odrive.handle.axis0.controller.config.control_mode, CONTROL_MODE_TORQUE_CONTROL) odrive.handle.axis0.controller.input_vel = 0 odrive.handle.axis0.controller.input_torque = 0 @@ -132,7 +132,7 @@ class TestUartAscii(): test_assert_eq(ser.readline(), b'') test_assert_eq(odrive.handle.axis0.controller.input_pos, 123.4, accuracy=0.001) test_assert_eq(odrive.handle.axis0.controller.config.vel_limit, 567.8, accuracy=0.001) - test_assert_eq(odrive.handle.axis0.motor.config.current_lim, 12.5, accuracy=0.001) + test_assert_eq(odrive.handle.axis0.motor.config.torque_lim, 12.5, accuracy=0.001) test_assert_eq(odrive.handle.axis0.controller.config.control_mode, CONTROL_MODE_POSITION_CONTROL) ser.write(b'f 0\n') From 2d05a50e8928394a98e043fa129926c095b7f8c6 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 1 Jul 2020 12:42:01 +0200 Subject: [PATCH 2/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 0ad7dc38..2a01e220 100644 --- a/Firmware/fibre/cpp/include/fibre/protocol.hpp +++ b/Firmware/fibre/cpp/include/fibre/protocol.hpp @@ -513,7 +513,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 fc82ed6610f5512807466bf206560c462a78b114 Mon Sep 17 00:00:00 2001 From: Cam Buss Date: Wed, 1 Jul 2020 14:50:25 -0600 Subject: [PATCH 3/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 172f3dc4..ffd6fffc 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::CONTROL_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 480de844..39870ddb 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -106,6 +106,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_ = 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 b36e352ee0684185a0b1f5dd689e5a8007172909 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 2 Jul 2020 21:34:51 -0400 Subject: [PATCH 4/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;