From b09d1e9e256e31059c6a9faf6467586fe5e5cbdc Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Mon, 22 Oct 2018 21:03:13 -0700 Subject: [PATCH 1/4] make step/dir pins configurable --- CHANGELOG.md | 3 ++ Firmware/MotorControl/axis.cpp | 37 +++++++++++++++-------- Firmware/MotorControl/axis.hpp | 21 +++++++++++-- Firmware/MotorControl/board_config_v3.h | 40 +++++++++++++++---------- Firmware/MotorControl/main.cpp | 4 ++- 5 files changed, 74 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 742c5300..b3d59676 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Unreleased Features Please add a note of your changes below this heading if you make a Pull Request. +## Added +* Make step dir gpio pins configurable. + ## Fixed * Would ERROR_CONTROL_DEADLINE_MISSED along with every ERROR_PHASE_RESISTANCE_OUT_OF_RANGE. diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 0ab726ff..cae62223 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -26,6 +26,8 @@ Axis::Axis(const AxisHardwareConfig_t& hw_config, controller_.axis_ = this; motor_.axis_ = this; trap_.axis_ = this; + + decode_step_dir_pins(); } static void step_cb_wrapper(void* ctx) { @@ -67,24 +69,37 @@ bool Axis::wait_for_current_meas() { // step/direction interface void Axis::step_cb() { if (step_dir_active_) { - GPIO_PinState dir_pin = HAL_GPIO_ReadPin(hw_config_.dir_port, hw_config_.dir_pin); + GPIO_PinState dir_pin = HAL_GPIO_ReadPin(dir_port_, dir_pin_); float dir = (dir_pin == GPIO_PIN_SET) ? 1.0f : -1.0f; controller_.pos_setpoint_ += dir * config_.counts_per_step; } }; -// @brief Enables or disables step/dir input -void Axis::set_step_dir_enabled(bool enable) { - if (enable) { +void Axis::load_default_step_dir_pin_config( + const AxisHardwareConfig_t& hw_config, Config_t* config) { + config->step_gpio_pin = hw_config.step_gpio_pin; + config->dir_gpio_pin = hw_config.dir_gpio_pin; +} + +void Axis::decode_step_dir_pins() { + step_port_ = get_gpio_port_by_pin(config_.step_gpio_pin); + step_pin_ = get_gpio_pin_by_pin(config_.step_gpio_pin); + dir_port_ = get_gpio_port_by_pin(config_.dir_gpio_pin); + dir_pin_ = get_gpio_pin_by_pin(config_.dir_gpio_pin); +} + +// @brief (de)activates step/dir input +void Axis::set_step_dir_active(bool active) { + if (active) { // Set up the direction GPIO as input GPIO_InitTypeDef GPIO_InitStruct; - GPIO_InitStruct.Pin = hw_config_.dir_pin; + GPIO_InitStruct.Pin = dir_pin_; GPIO_InitStruct.Mode = GPIO_MODE_INPUT; GPIO_InitStruct.Pull = GPIO_NOPULL; - HAL_GPIO_Init(hw_config_.dir_port, &GPIO_InitStruct); + HAL_GPIO_Init(dir_port_, &GPIO_InitStruct); // Subscribe to rising edges of the step GPIO - GPIO_subscribe(hw_config_.step_port, hw_config_.step_pin, GPIO_PULLDOWN, + GPIO_subscribe(step_port_, step_pin_, GPIO_PULLDOWN, step_cb_wrapper, this); step_dir_active_ = true; @@ -92,7 +107,7 @@ void Axis::set_step_dir_enabled(bool enable) { step_dir_active_ = false; // Unsubscribe from step GPIO - GPIO_unsubscribe(hw_config_.step_port, hw_config_.step_pin); + GPIO_unsubscribe(step_port_, step_pin_); } } @@ -167,7 +182,6 @@ bool Axis::run_sensorless_spin_up() { // Note run_sensorless_control_loop and run_closed_loop_control_loop are very similar and differ only in where we get the estimate from. bool Axis::run_sensorless_control_loop() { - set_step_dir_enabled(config_.enable_step_dir); run_control_loop([this](){ if (controller_.config_.control_mode >= Controller::CTRL_MODE_POSITION_CONTROL) return error_ |= ERROR_POS_CTRL_DURING_SENSORLESS, false; @@ -180,12 +194,11 @@ bool Axis::run_sensorless_control_loop() { return false; // set_error should update axis.error_ return true; }); - set_step_dir_enabled(false); return check_for_errors(); } bool Axis::run_closed_loop_control_loop() { - set_step_dir_enabled(config_.enable_step_dir); + set_step_dir_active(config_.enable_step_dir); run_control_loop([this](){ // Note that all estimators are updated in the loop prefix in run_control_loop float current_setpoint; @@ -195,7 +208,7 @@ bool Axis::run_closed_loop_control_loop() { return false; // set_error should update axis.error_ return true; }); - set_step_dir_enabled(false); + set_step_dir_active(false); return check_for_errors(); } diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index 6e9b8bfc..d9f95ad8 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -45,9 +45,12 @@ public: bool startup_sensorless_control = false; //(ctx)->decode_step_dir_pins(); }, this), + make_protocol_property("dir_gpio_pin", &config_.dir_gpio_pin, + [](void* ctx) { static_cast(ctx)->decode_step_dir_pins(); }, this), make_protocol_property("ramp_up_time", &config_.ramp_up_time), make_protocol_property("ramp_up_distance", &config_.ramp_up_distance), make_protocol_property("spin_up_current", &config_.spin_up_current), diff --git a/Firmware/MotorControl/board_config_v3.h b/Firmware/MotorControl/board_config_v3.h index 1e170fed..afda10f6 100644 --- a/Firmware/MotorControl/board_config_v3.h +++ b/Firmware/MotorControl/board_config_v3.h @@ -21,10 +21,12 @@ typedef struct { - GPIO_TypeDef* step_port; - uint16_t step_pin; - GPIO_TypeDef* dir_port; - uint16_t dir_pin; + // GPIO_TypeDef* step_port; + // uint16_t step_pin; + // GPIO_TypeDef* dir_port; + // uint16_t dir_pin; + uint16_t step_gpio_pin; + uint16_t dir_gpio_pin; size_t thermistor_adc_ch; osPriority thread_priority; } AxisHardwareConfig_t; @@ -74,10 +76,12 @@ const size_t thermistor_num_coeffs = sizeof(thermistor_poly_coeffs)/sizeof(therm const BoardHardwareConfig_t hw_configs[2] = { { //M0 .axis_config = { - .step_port = GPIO_1_GPIO_Port, - .step_pin = GPIO_1_Pin, - .dir_port = GPIO_2_GPIO_Port, - .dir_pin = GPIO_2_Pin, + // .step_port = GPIO_1_GPIO_Port, + // .step_pin = GPIO_1_Pin, + // .dir_port = GPIO_2_GPIO_Port, + // .dir_pin = GPIO_2_Pin, + .step_gpio_pin = 1, + .dir_gpio_pin = 2, .thermistor_adc_ch = 15, .thread_priority = (osPriority)(osPriorityHigh + (osPriority)1), }, @@ -111,15 +115,19 @@ const BoardHardwareConfig_t hw_configs[2] = { { //M1 .axis_config = { #if HW_VERSION_MAJOR == 3 && HW_VERSION_MINOR >= 5 - .step_port = GPIO_7_GPIO_Port, - .step_pin = GPIO_7_Pin, - .dir_port = GPIO_8_GPIO_Port, - .dir_pin = GPIO_8_Pin, + // .step_port = GPIO_7_GPIO_Port, + // .step_pin = GPIO_7_Pin, + // .dir_port = GPIO_8_GPIO_Port, + // .dir_pin = GPIO_8_Pin, + .step_gpio_pin = 7, + .dir_gpio_pin = 8, #else - .step_port = GPIO_3_GPIO_Port, - .step_pin = GPIO_3_Pin, - .dir_port = GPIO_4_GPIO_Port, - .dir_pin = GPIO_4_Pin, + // .step_port = GPIO_3_GPIO_Port, + // .step_pin = GPIO_3_Pin, + // .dir_port = GPIO_4_GPIO_Port, + // .dir_pin = GPIO_4_Pin, + .step_gpio_pin = 3, + .dir_gpio_pin = 4, #endif #if HW_VERSION_MAJOR == 3 && HW_VERSION_MINOR >= 3 .thermistor_adc_ch = 4, diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 4550605e..738836a3 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -65,6 +65,8 @@ void load_configuration(void) { motor_configs[i] = Motor::Config_t(); trap_configs[i] = TrapezoidalTrajectory::Config_t(); axis_configs[i] = Axis::Config_t(); + // Default step/dir pins are different, so we need to explicitly load them + Axis::load_default_step_dir_pin_config(hw_configs[i].axis_config, &axis_configs[i]); } } else { user_config_loaded_ = true; @@ -179,7 +181,7 @@ int odrive_main(void) { #if HW_VERSION_MAJOR == 3 && HW_VERSION_MINOR >= 3 if (board_config.enable_uart) { axes[0]->config_.enable_step_dir = false; - axes[0]->set_step_dir_enabled(false); + axes[0]->set_step_dir_active(false); SetGPIO12toUART(); } #endif From 209839d2a796a198fa8f64f7f6f33a4130481539 Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Mon, 22 Oct 2018 21:28:15 -0700 Subject: [PATCH 2/4] remve uart stepdir interlock: stepdir now takes priority --- Firmware/MotorControl/main.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 738836a3..4146850c 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -180,8 +180,6 @@ int odrive_main(void) { // TODO: make dynamically reconfigurable #if HW_VERSION_MAJOR == 3 && HW_VERSION_MINOR >= 3 if (board_config.enable_uart) { - axes[0]->config_.enable_step_dir = false; - axes[0]->set_step_dir_active(false); SetGPIO12toUART(); } #endif From ae30439d14f7519b252392cbef09242231cf140c Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Mon, 22 Oct 2018 21:32:20 -0700 Subject: [PATCH 3/4] clean up commented parts --- Firmware/MotorControl/board_config_v3.h | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/Firmware/MotorControl/board_config_v3.h b/Firmware/MotorControl/board_config_v3.h index afda10f6..7aee4881 100644 --- a/Firmware/MotorControl/board_config_v3.h +++ b/Firmware/MotorControl/board_config_v3.h @@ -21,10 +21,6 @@ typedef struct { - // GPIO_TypeDef* step_port; - // uint16_t step_pin; - // GPIO_TypeDef* dir_port; - // uint16_t dir_pin; uint16_t step_gpio_pin; uint16_t dir_gpio_pin; size_t thermistor_adc_ch; @@ -76,10 +72,6 @@ const size_t thermistor_num_coeffs = sizeof(thermistor_poly_coeffs)/sizeof(therm const BoardHardwareConfig_t hw_configs[2] = { { //M0 .axis_config = { - // .step_port = GPIO_1_GPIO_Port, - // .step_pin = GPIO_1_Pin, - // .dir_port = GPIO_2_GPIO_Port, - // .dir_pin = GPIO_2_Pin, .step_gpio_pin = 1, .dir_gpio_pin = 2, .thermistor_adc_ch = 15, @@ -115,17 +107,9 @@ const BoardHardwareConfig_t hw_configs[2] = { { //M1 .axis_config = { #if HW_VERSION_MAJOR == 3 && HW_VERSION_MINOR >= 5 - // .step_port = GPIO_7_GPIO_Port, - // .step_pin = GPIO_7_Pin, - // .dir_port = GPIO_8_GPIO_Port, - // .dir_pin = GPIO_8_Pin, .step_gpio_pin = 7, .dir_gpio_pin = 8, #else - // .step_port = GPIO_3_GPIO_Port, - // .step_pin = GPIO_3_Pin, - // .dir_port = GPIO_4_GPIO_Port, - // .dir_pin = GPIO_4_Pin, .step_gpio_pin = 3, .dir_gpio_pin = 4, #endif From 5958ae23f4be042f393a7fd3f7c059f8bd352a2e Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Mon, 22 Oct 2018 21:39:57 -0700 Subject: [PATCH 4/4] update docs --- docs/interfaces.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/interfaces.md b/docs/interfaces.md index ba6ccfb0..ff0ec9ac 100644 --- a/docs/interfaces.md +++ b/docs/interfaces.md @@ -32,7 +32,10 @@ The ODrive can be controlled over various ports and protocols. If you're comfort (+) on ODrive v3.4 and earlier
(*) ODrive v3.5 and later -ODrive v3.3 and onward have 5V tolerant GPIO pins. +Notes: +* ODrive v3.3 and onward have 5V tolerant GPIO pins. +* ODrive v3.5 and later have some noise supression filters on the default step/dir pins +* You can change the step/dir pins using `axis.config._gpio_pin`. ### Pin function priorities 1. PWM in, if enabled. Disabled by default. @@ -89,7 +92,7 @@ There is an Arduino library that gives some expamples on how to use the ASCII pr This is the simplest possible way of controlling the ODrive. It is also the most primitive and fragile one. So don't use it unless you must interoperate with other hardware that you don't control. Pinout: -* Step/dir signals: see [Pinout](#pinout) above. +* Step/dir signals: see [Pinout](#pinout) above. Note in that section how to reassign the pins. * GND: you must connect the grounds of the devices together. Use any GND pin on J3 of the ODrive. To enable step/dir mode for the GPIO, set `.config.enable_step_dir` to true for each axis that you wish to use this on.