From f930462588dd61c90067ca153d41506ede5b86e4 Mon Sep 17 00:00:00 2001 From: Paul Guenette Date: Sat, 7 Nov 2020 15:11:33 -0500 Subject: [PATCH 01/28] Add enum generation to makefile --- Firmware/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Firmware/Makefile b/Firmware/Makefile index a40ae89f..d12efb32 100644 --- a/Firmware/Makefile +++ b/Firmware/Makefile @@ -12,6 +12,7 @@ OPENOCD := openocd -f interface/stlink-v2.cfg \ all: @tup --quiet --no-environ-check + @python interface_generator_stub.py --definitions odrive-interface.yaml --template ../tools/enums_template.j2 --output ../tools/odrive/enums.py flash: all $(OPENOCD) -c init \ From ed75acb905e6a3d90a9f46d1e5a27e8c18c67842 Mon Sep 17 00:00:00 2001 From: Paul Guenette Date: Mon, 9 Nov 2020 21:12:35 -0500 Subject: [PATCH 02/28] Update issue templates --- .github/ISSUE_TEMPLATE/bug_report.md | 24 +++++++++++++++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 20 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..f29b8674 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,24 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: bug +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +Steps and configuration necessary to reproduce the behavior. + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Desktop (please complete the following information):** + - OS: [e.g. Windows 10] + - odrivetool Version (`odrivetool --version`) + +**Additional context** +Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000..bbcbbe7d --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: '' +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. From ff3a3b2078eccd2348115c8f1524166f4a89f9f0 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 5 Oct 2020 13:58:52 +0200 Subject: [PATCH 03/28] use pip3 in nightly install tests --- .github/workflows/nightly.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nightly.yaml b/.github/workflows/nightly.yaml index 4b3e3294..8769d92d 100644 --- a/.github/workflows/nightly.yaml +++ b/.github/workflows/nightly.yaml @@ -16,8 +16,8 @@ jobs: steps: - name: Install odrivetool run: | - pip install monotonic # TODO: this is dishonest. Must be removed as soon as v0.5.0 is published! - pip install odrive + pip3 install monotonic # TODO: this is dishonest. Must be removed as soon as v0.5.0 is published! + pip3 install odrive # This one currently fails because Github Actions runs pip as non-root #- name: Check if udev rules were set up properly From 6d22b35f990d654139bfcb7dc04ef3b16438b8c8 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 4 Nov 2020 10:37:22 +0100 Subject: [PATCH 04/28] add `config.enable_brake_resistor` --- CHANGELOG.md | 1 + Firmware/MotorControl/low_level.cpp | 48 ++++++++++++++++---------- Firmware/MotorControl/odrive_main.h | 1 + Firmware/odrive-interface.yaml | 18 +++++++++- tools/odrive/enums.py | 1 + tools/odrive/tests/calibration_test.py | 2 ++ tools/odrive/tests/closed_loop_test.py | 6 +++- tools/test-rig-rpi.yaml | 2 +- 8 files changed, 57 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04cc36d6..22034ba4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Please add a note of your changes below this heading if you make a Pull Request. * Several properties were changed to readonly. * `.encoder.config.offset` was renamed to ``.encoder.config.phase_offset` * `.encoder.config.offset_float` was renamed to ``.encoder.config.phase_offset_float` +* `.config.brake_resistance == 0.0` is no longer a valid way to disable the brake resistor. Use `.config.enable_brake_resistor` instead. # Releases ## [0.5.1] - 2020-09-27 diff --git a/Firmware/MotorControl/low_level.cpp b/Firmware/MotorControl/low_level.cpp index 066cd533..4c9cf23e 100644 --- a/Firmware/MotorControl/low_level.cpp +++ b/Firmware/MotorControl/low_level.cpp @@ -310,31 +310,41 @@ void update_brake_current() { Ibus_sum += axes[i].motor_.I_bus_; } } + + float brake_duty; + + if (odrv.config_.enable_brake_resistor) { + if (!(odrv.config_.brake_resistance > 0.0f)) { + odrv.disarm_with_error(ODrive::ERROR_INVALID_BRAKE_RESISTANCE); + return; + } - // Don't start braking until -Ibus > regen_current_allowed - float brake_current = -Ibus_sum - odrv.config_.max_regen_current; - float brake_duty = brake_current * odrv.config_.brake_resistance / vbus_voltage; - - if (odrv.config_.enable_dc_bus_overvoltage_ramp && (odrv.config_.brake_resistance > 0.0f) && (odrv.config_.dc_bus_overvoltage_ramp_start < odrv.config_.dc_bus_overvoltage_ramp_end)) { - brake_duty += std::max((vbus_voltage - odrv.config_.dc_bus_overvoltage_ramp_start) / (odrv.config_.dc_bus_overvoltage_ramp_end - odrv.config_.dc_bus_overvoltage_ramp_start), 0.0f); - } + // Don't start braking until -Ibus > regen_current_allowed + float brake_current = -Ibus_sum - odrv.config_.max_regen_current; + brake_duty = brake_current * odrv.config_.brake_resistance / vbus_voltage; + + if (odrv.config_.enable_dc_bus_overvoltage_ramp && (odrv.config_.brake_resistance > 0.0f) && (odrv.config_.dc_bus_overvoltage_ramp_start < odrv.config_.dc_bus_overvoltage_ramp_end)) { + brake_duty += std::max((vbus_voltage - odrv.config_.dc_bus_overvoltage_ramp_start) / (odrv.config_.dc_bus_overvoltage_ramp_end - odrv.config_.dc_bus_overvoltage_ramp_start), 0.0f); + } - if (is_nan(brake_duty)) { - // Shuts off all motors AND brake resistor, sets error code on all motors. - odrv.disarm_with_error(ODrive::ERROR_BRAKE_DUTY_CYCLE_NAN); - return; - } + if (is_nan(brake_duty)) { + // Shuts off all motors AND brake resistor, sets error code on all motors. + odrv.disarm_with_error(ODrive::ERROR_BRAKE_DUTY_CYCLE_NAN); + return; + } - if (brake_duty >= 0.95f) { - brake_resistor_saturated = true; - } + if (brake_duty >= 0.95f) { + brake_resistor_saturated = true; + } - // Duty limit at 95% to allow bootstrap caps to charge - brake_duty = std::clamp(brake_duty, 0.0f, 0.95f); + // Duty limit at 95% to allow bootstrap caps to charge + brake_duty = std::clamp(brake_duty, 0.0f, 0.95f); - // Special handling to avoid the case 0.0/0.0 == NaN, or divide by 0. - if (odrv.config_.brake_resistance > 0.0f) { + // This cannot result in NaN (safe for race conditions) because we check + // brake_resistance != 0 further up. Ibus_sum += brake_duty * vbus_voltage / odrv.config_.brake_resistance; + } else { + brake_duty = 0; } ibus_ += odrv.ibus_report_filter_k_ * (Ibus_sum - ibus_); diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h index 886fba1b..88549e74 100644 --- a/Firmware/MotorControl/odrive_main.h +++ b/Firmware/MotorControl/odrive_main.h @@ -74,6 +74,7 @@ struct BoardConfig_t { bool enable_ascii_protocol_on_usb = true; float max_regen_current = 0.0f; float brake_resistance = DEFAULT_BRAKE_RESISTANCE; + bool enable_brake_resistor = false; float dc_bus_undervoltage_trip_level = 8.0f; // Date: Thu, 10 Sep 2020 20:04:41 +0200 Subject: [PATCH 05/28] minor tweaks to v3 code --- Firmware/Board/v3/board.cpp | 21 +++++---------------- Firmware/Drivers/DRV8301/drv8301.hpp | 2 +- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/Firmware/Board/v3/board.cpp b/Firmware/Board/v3/board.cpp index 17f0e9f0..dada4c12 100644 --- a/Firmware/Board/v3/board.cpp +++ b/Firmware/Board/v3/board.cpp @@ -297,7 +297,7 @@ bool board_init() { HAL_NVIC_SetPriority(EXTI15_10_IRQn, 1, 0); HAL_NVIC_EnableIRQ(EXTI15_10_IRQn); - HAL_NVIC_SetPriority(ControlLoop_IRQn, 5, 0); // must be on the same level as ADC interrupt + HAL_NVIC_SetPriority(ControlLoop_IRQn, 5, 0); HAL_NVIC_EnableIRQ(ControlLoop_IRQn); HAL_NVIC_SetPriority(TIM8_UP_TIM13_IRQn, 0, 0); @@ -364,19 +364,10 @@ void start_timers() { hadc3.Instance->CR2 &= ~(ADC_CR2_EXTEN | ADC_CR2_JEXTEN); /* - * Initial intention of the synchronization: * Synchronize TIM1, TIM8 and TIM13 such that: * 1. The triangle waveform of TIM1 leads the triangle waveform of TIM8 by a * 90° phase shift. - * 2. The timer update events of TIM1 and TIM8 are symmetrically interleaved. - * 3. Each TIM13 reload coincides with a TIM1 lower update event. - * - * However right now this synchronization only ensures point (1) and (3) but because - * TIM1 and TIM3 only trigger an update on every third reload, this does not - * allow for (2). - * - * TODO: revisit the timing topic in general. - * + * 2. Each TIM13 reload coincides with a TIM1 lower update event. */ Stm32Timer::start_synchronously<3>( {&htim1, &htim8, &htim13}, @@ -396,12 +387,8 @@ void start_timers() { __HAL_ADC_CLEAR_FLAG(&hadc1, ADC_FLAG_OVR); __HAL_ADC_CLEAR_FLAG(&hadc2, ADC_FLAG_OVR); __HAL_ADC_CLEAR_FLAG(&hadc3, ADC_FLAG_OVR); + __HAL_TIM_CLEAR_IT(&htim8, TIM_IT_UPDATE); - - // it's sufficient to enable interrupts for one ADC only because they all trigger simultaneously - //__HAL_ADC_ENABLE_IT(&hadc3, ADC_IT_JEOC); - //__HAL_ADC_ENABLE_IT(&hadc3, ADC_IT_EOC); - __HAL_TIM_ENABLE_IT(&htim8, TIM_IT_UPDATE); } } @@ -466,6 +453,8 @@ volatile uint32_t timestamp_ = 0; volatile bool counting_down_ = false; void TIM8_UP_TIM13_IRQHandler(void) { + COUNT_IRQ(TIM8_UP_TIM13_IRQn); + // Entry into this function happens at 21-23 clock cycles after the timer // update event. __HAL_TIM_CLEAR_IT(&htim8, TIM_IT_UPDATE); diff --git a/Firmware/Drivers/DRV8301/drv8301.hpp b/Firmware/Drivers/DRV8301/drv8301.hpp index 45f693bd..29a04465 100644 --- a/Firmware/Drivers/DRV8301/drv8301.hpp +++ b/Firmware/Drivers/DRV8301/drv8301.hpp @@ -42,7 +42,7 @@ public: * If the gate driver was in ready state and the new configuration is * different from the old one then the gate driver will exit ready state. * - * In any case cnahges to the configuration only take effect with a call to + * In any case changes to the configuration only take effect with a call to * init(). */ bool config(float requested_gain, float* actual_gain); From d653abfac2ecc401f103b8011b24bdc4fe2eda2c Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Sat, 10 Oct 2020 12:29:57 +0200 Subject: [PATCH 06/28] update dev guide --- docs/developer-guide.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 54daa887..6fac13a8 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -143,11 +143,7 @@ If the flashing worked, you can connect to the board using the [odrivetool](gett

## Testing -The script `tools/run_tests.py` runs a sequence of automated tests for several firmware features as well as high power burn-in tests. Some tests only need one ODrive and one motor/encoder pair while other tests need a back-to-back test rig such as [this one](https://cad.onshape.com/documents/026bda35ad5dff4d73c1d37f/w/ae302174f402737e1fdb3783/e/5ca143a6e5e24daf1fe8e434). In any case, to run the tests you need to provide a YAML file that lists the parameters of your test setup. An example can be found at [`tools/test-rig-parallel.yaml`](tools/test-rig-parallel.yaml`). The programmer serial number can be found by running `Firmware/find_programmer.sh` (make sure it has the latest firmware from STM). - -
The test script commands the ODrive to high currents and high motor speeds so if your ODrive is connected to anything other than a stirdy test-rig (or free spinning motors), it will probably break your machine.
- -Example usage: `./run_tests.py --test-rig-yaml ../tools/test-rig-parallel.yaml` +_Main article: [Testing](testing.md)_

## Debugging From a6587f8d8ae1441a924dd3769b823379465f9b37 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 11 Nov 2020 21:02:25 +0100 Subject: [PATCH 07/28] fix various issues - overcurrent error during motor calibration (this is caused by overshoot. For now we just ignore the current limit during motor calibration as we did before) - factor 2 error at encoder calibration - set error flag if Motor::arm() is called while the brake resistor is enabled but disarmed. - only arm brake resistor if enabled - auto-arm brake resistor on clear_errors() --- Firmware/MotorControl/encoder.cpp | 8 ++++---- Firmware/MotorControl/low_level.cpp | 7 ++++++- Firmware/MotorControl/main.cpp | 3 +++ Firmware/MotorControl/motor.cpp | 11 ++++++++--- Firmware/MotorControl/open_loop_controller.cpp | 2 +- Firmware/MotorControl/open_loop_controller.hpp | 1 + 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/Firmware/MotorControl/encoder.cpp b/Firmware/MotorControl/encoder.cpp index 7c40a185..7132100e 100644 --- a/Firmware/MotorControl/encoder.cpp +++ b/Firmware/MotorControl/encoder.cpp @@ -228,7 +228,7 @@ bool Encoder::run_offset_calibration() { axis_->open_loop_controller_.target_voltage_ = axis_->motor_.config_.motor_type != Motor::MOTOR_TYPE_GIMBAL ? 0.0f : axis_->motor_.config_.calibration_current; axis_->open_loop_controller_.target_vel_ = 0.0f; axis_->open_loop_controller_.total_distance_ = 0.0f; - axis_->open_loop_controller_.phase_ = wrap_pm_pi(0 - config_.calib_scan_distance / 2.0f); + axis_->open_loop_controller_.phase_ = axis_->open_loop_controller_.initial_phase_ = wrap_pm_pi(0 - config_.calib_scan_distance / 2.0f); axis_->motor_.current_control_.enable_current_control_src_ = (axis_->motor_.config_.motor_type != Motor::MOTOR_TYPE_GIMBAL); axis_->motor_.current_control_.Idq_setpoint_src_.connect_to(&axis_->open_loop_controller_.Idq_setpoint_); @@ -324,9 +324,9 @@ bool Encoder::run_offset_calibration() { axis_->motor_.disarm(); - config_.phase_offset = encvaluesum / (num_steps * 2); - int32_t residual = encvaluesum - ((int64_t)config_.phase_offset * (int64_t)(num_steps * 2)); - config_.phase_offset_float = (float)residual / (float)(num_steps * 2) + 0.5f; // add 0.5 to center-align state to phase + config_.phase_offset = encvaluesum / num_steps; + int32_t residual = encvaluesum - ((int64_t)config_.phase_offset * (int64_t)num_steps); + config_.phase_offset_float = (float)residual / (float)num_steps + 0.5f; // add 0.5 to center-align state to phase is_ready_ = true; return true; diff --git a/Firmware/MotorControl/low_level.cpp b/Firmware/MotorControl/low_level.cpp index 4c9cf23e..f0a2bb64 100644 --- a/Firmware/MotorControl/low_level.cpp +++ b/Firmware/MotorControl/low_level.cpp @@ -76,6 +76,9 @@ bool brake_resistor_saturated = false; // @brief Arms the brake resistor void safety_critical_arm_brake_resistor() { CRITICAL_SECTION() { + for (size_t i = 0; i < AXIS_COUNT; ++i) { + axes[i].motor_.I_bus_ = 0.0f; + } brake_resistor_armed = true; htim2.Instance->CCR3 = 0; htim2.Instance->CCR4 = TIM_APB1_PERIOD_CLOCKS + 1; @@ -164,7 +167,9 @@ void start_adc_pwm() { HAL_TIM_PWM_Start(&htim2, TIM_CHANNEL_3); HAL_TIM_PWM_Start(&htim2, TIM_CHANNEL_4); - safety_critical_arm_brake_resistor(); + if (odrv.config_.enable_brake_resistor) { + safety_critical_arm_brake_resistor(); + } } // @brief ADC1 measurements are written to this buffer by DMA diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index ae24e3d4..49a91efd 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -161,6 +161,9 @@ void ODrive::clear_errors() { axis.error_ = Axis::ERROR_NONE; } error_ = ERROR_NONE; + if (odrv.config_.enable_brake_resistor) { + safety_critical_arm_brake_resistor(); + } } extern "C" { diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index 557cde9c..a81cdc2a 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -198,8 +198,10 @@ bool Motor::arm(PhaseControlLaw<3>* control_law) { control_law_->reset(); } - if (brake_resistor_armed) { + if (!odrv.config_.enable_brake_resistor || brake_resistor_armed) { is_armed_ = true; + } else { + error_ |= Motor::ERROR_BRAKE_RESISTOR_DISARMED; } } @@ -217,7 +219,7 @@ bool Motor::arm(PhaseControlLaw<3>* control_law) { */ void Motor::apply_pwm_timings(uint16_t timings[3], bool tentative) { CRITICAL_SECTION() { - if (!brake_resistor_armed) { + if (odrv.config_.enable_brake_resistor && !brake_resistor_armed) { disarm_with_error(ERROR_BRAKE_RESISTOR_DISARMED); } @@ -609,7 +611,10 @@ void Motor::current_meas_cb(uint32_t timestamp, std::optional current float Inorm_sq = 2.0f / 3.0f * (SQ(current_meas_->phA) + SQ(current_meas_->phB) + SQ(current_meas_->phC)); - if (Inorm_sq > SQ(Itrip)) { + + // Hack: we disable the current check during motor calibration because + // it tends to briefly overshoot when the motor moves to align flux with I_alpha + if (Inorm_sq > SQ(Itrip) && (axis_->current_state_ != Axis::AXIS_STATE_MOTOR_CALIBRATION)) { disarm_with_error(ERROR_CURRENT_LIMIT_VIOLATION); } } else if (is_armed_) { diff --git a/Firmware/MotorControl/open_loop_controller.cpp b/Firmware/MotorControl/open_loop_controller.cpp index 3fe9dc24..16d2d5db 100644 --- a/Firmware/MotorControl/open_loop_controller.cpp +++ b/Firmware/MotorControl/open_loop_controller.cpp @@ -5,7 +5,7 @@ void OpenLoopController::update(uint32_t timestamp) { auto [prev_Id, prev_Iq] = Idq_setpoint_.get_previous().value_or(float2D{0.0f, 0.0f}); auto [prev_Vd, prev_Vq] = Vdq_setpoint_.get_previous().value_or(float2D{0.0f, 0.0f}); - float phase = phase_.get_previous().value_or(0.0f); + float phase = phase_.get_previous().value_or(initial_phase_); float phase_vel = phase_vel_.get_previous().value_or(0.0f); (void)prev_Iq; // unused diff --git a/Firmware/MotorControl/open_loop_controller.hpp b/Firmware/MotorControl/open_loop_controller.hpp index 82356a23..54371bd3 100644 --- a/Firmware/MotorControl/open_loop_controller.hpp +++ b/Firmware/MotorControl/open_loop_controller.hpp @@ -18,6 +18,7 @@ public: float target_vel_ = 0.0f; float target_current_ = 0.0f; float target_voltage_ = 0.0f; + float initial_phase_ = 0.0f; // State/Outputs uint32_t timestamp_ = 0; From 69897ece9c05521bd7627fed47753c0167813b43 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 11 Nov 2020 21:12:30 +0100 Subject: [PATCH 08/28] improve debug instrumentation - add ERROR output on GPIO - add exported get_gpio_states() function - add task timer for DC calib ADC wait time - fix oscilloscope --- Firmware/Board/v3/Inc/board.h | 2 ++ Firmware/Board/v3/board.cpp | 4 +++- Firmware/MotorControl/main.cpp | 27 ++++++++++++++++++++++++++ Firmware/MotorControl/motor.cpp | 2 +- Firmware/MotorControl/odrive_main.h | 10 ++++++++-- Firmware/MotorControl/oscilloscope.cpp | 22 ++++++++++----------- Firmware/MotorControl/oscilloscope.hpp | 6 ++++-- Firmware/odrive-interface.yaml | 15 ++++++++++---- tools/odrive/enums.py | 1 + 9 files changed, 67 insertions(+), 22 deletions(-) diff --git a/Firmware/Board/v3/Inc/board.h b/Firmware/Board/v3/Inc/board.h index 3443b90c..4820b93b 100644 --- a/Firmware/Board/v3/Inc/board.h +++ b/Firmware/Board/v3/Inc/board.h @@ -41,6 +41,8 @@ #define DEFAULT_BRAKE_RESISTANCE (0.47f) // [ohm] #endif +#define DEFAULT_ERROR_PIN 0 + #define DEFAULT_GPIO_MODES \ ODriveIntf::GPIO_MODE_DIGITAL, \ ODriveIntf::GPIO_MODE_UART_A, \ diff --git a/Firmware/Board/v3/board.cpp b/Firmware/Board/v3/board.cpp index dada4c12..bfc56bea 100644 --- a/Firmware/Board/v3/board.cpp +++ b/Firmware/Board/v3/board.cpp @@ -513,7 +513,9 @@ void ControlLoop_IRQHandler(void) { // By this time the ADCs for both M0 and M1 should have fired again. But // let's wait for them just to be sure. - while (!(ADC2->SR & ADC_SR_EOC)); + MEASURE_TIME(odrv.task_times_.dc_calib_wait) { + while (!(ADC2->SR & ADC_SR_EOC)); + } if (!fetch_and_reset_adcs(¤t0, ¤t1)) { motors[0].disarm_with_error(Motor::ERROR_BAD_TIMING); diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 49a91efd..c2342ebe 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -151,6 +151,17 @@ void ODrive::enter_dfu_mode() { } } +bool ODrive::any_error() { + return error_ != ODrive::ERROR_NONE + || std::any_of(axes.begin(), axes.end(), [](Axis& axis){ + return axis.error_ != Axis::ERROR_NONE + || axis.motor_.error_ != Motor::ERROR_NONE + || axis.sensorless_estimator_.error_ != SensorlessEstimator::ERROR_NONE + || axis.encoder_.error_ != Encoder::ERROR_NONE + || axis.controller_.error_ != Controller::ERROR_NONE; + }); +} + void ODrive::clear_errors() { for (auto& axis: axes) { axis.motor_.error_ = Motor::ERROR_NONE; @@ -366,6 +377,8 @@ void ODrive::control_loop_cb(uint32_t timestamp) { MEASURE_TIME(axis.task_times_.current_controller_update) axis.motor_.current_control_.update(timestamp); // uses the output of controller_ or open_loop_contoller_ and encoder_ or sensorless_estimator_ or async_estimator_ } + + get_gpio(odrv.config_.error_gpio_pin).write(odrv.any_error()); } @@ -407,6 +420,14 @@ uint32_t ODrive::get_dma_status(uint8_t stream_num) { return (is_reset ? 0 : 0x80000000) | ((channel & 0x7) << 2) | (priority & 0x3); } +uint32_t ODrive::get_gpio_states() { + // TODO: get values that were sampled synchronously with the control loop + uint32_t val = 0; + for (size_t i = 0; i < GPIO_COUNT; ++i) { + val |= ((gpios[i].read() ? 1UL : 0UL) << i); + } + return val; +} /** * @brief Main thread started from main(). @@ -580,6 +601,7 @@ extern "C" int main(void) { mode == ODriveIntf::GPIO_MODE_DIGITAL_PULL_UP || mode == ODriveIntf::GPIO_MODE_DIGITAL_PULL_DOWN || mode == ODriveIntf::GPIO_MODE_MECH_BRAKE || + mode == ODriveIntf::GPIO_MODE_STATUS || mode == ODriveIntf::GPIO_MODE_ANALOG_IN) { GPIO_InitStruct.Alternate = 0; } else { @@ -681,6 +703,11 @@ extern "C" int main(void) { GPIO_InitStruct.Pull = GPIO_NOPULL; GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW; } break; + case ODriveIntf::GPIO_MODE_STATUS: { + GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP; + GPIO_InitStruct.Pull = GPIO_NOPULL; + GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW; + } break; default: { odrv.misconfigured_ = true; continue; diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index a81cdc2a..be41bd3b 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -256,7 +256,7 @@ void Motor::apply_pwm_timings(uint16_t timings[3], bool tentative) { * arm() is called. */ bool Motor::disarm(bool* p_was_armed) { - bool was_armed; + bool was_armed = false; CRITICAL_SECTION() { was_armed = is_armed_; diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h index 88549e74..c88618bf 100644 --- a/Firmware/MotorControl/odrive_main.h +++ b/Firmware/MotorControl/odrive_main.h @@ -104,6 +104,7 @@ struct BoardConfig_t { float dc_max_positive_current = INFINITY; // Max current [A] the power supply can source float dc_max_negative_current = -0.000001f; // Max current [A] the power supply can sink. You most likely want a non-positive value here. Set to -INFINITY to disable. + uint32_t error_gpio_pin = DEFAULT_ERROR_PIN; PWMMapping_t pwm_mappings[4]; PWMMapping_t analog_mappings[GPIO_COUNT]; }; @@ -112,6 +113,7 @@ struct TaskTimes { TaskTimer sampling; TaskTimer control_loop_misc; TaskTimer control_loop_checks; + TaskTimer dc_calib_wait; }; @@ -169,6 +171,7 @@ public: void erase_configuration() override; void reboot() override { NVIC_SystemReset(); } void enter_dfu_mode() override; + bool any_error(); void clear_errors() override; float get_adc_voltage(uint32_t gpio) override { @@ -189,6 +192,7 @@ public: uint32_t get_interrupt_status(int32_t irqn); uint32_t get_dma_status(uint8_t stream_num); + uint32_t get_gpio_states(); void disarm_with_error(Error error); Error error_ = ERROR_NONE; @@ -230,10 +234,12 @@ public: bool& brake_resistor_saturated_ = ::brake_resistor_saturated; // TODO: make this the actual variable SystemStats_t system_stats_; + + // Edit these to suit your capture needs Oscilloscope oscilloscope_{ - &axes[0].motor_.current_control_.v_current_control_integral_d_, // trigger_src + nullptr, // trigger_src 0.5f, // trigger_threshold - nullptr // &axes[0].motor_.current_control_.Ialpha_measured_ // data_src TODO: change data type + nullptr // data_src TODO: change data type }; BoardConfig_t config_; diff --git a/Firmware/MotorControl/oscilloscope.cpp b/Firmware/MotorControl/oscilloscope.cpp index 05d2038f..21320fd6 100644 --- a/Firmware/MotorControl/oscilloscope.cpp +++ b/Firmware/MotorControl/oscilloscope.cpp @@ -5,25 +5,23 @@ #define OSCILLOSCOPE_SIZE 4096 void Oscilloscope::update() { - // Edit these to suit your capture needs float trigger_data = trigger_src_ ? *trigger_src_ : 0.0f; float trigger_threshold = trigger_threshold_; - float sample_data = data_src_ ? *data_src_ : 0.0f; + float sample_data = data_src_ ? **data_src_ : 0.0f; - static bool ready = false; - static bool capturing = false; if (trigger_data < trigger_threshold) { - ready = true; + ready_ = true; } - if (ready && trigger_data >= trigger_threshold) { - capturing = true; - ready = false; + if (ready_ && trigger_data >= trigger_threshold) { + capturing_ = true; + ready_ = false; } - if (capturing) { - data_[pos_] = sample_data; - if (++pos_ >= OSCILLOSCOPE_SIZE) { + if (capturing_) { + if (pos_ < OSCILLOSCOPE_SIZE) { + data_[pos_++] = sample_data; + } else { pos_ = 0; - capturing = false; + capturing_ = false; } } } diff --git a/Firmware/MotorControl/oscilloscope.hpp b/Firmware/MotorControl/oscilloscope.hpp index b1a5e016..df6dcd2a 100644 --- a/Firmware/MotorControl/oscilloscope.hpp +++ b/Firmware/MotorControl/oscilloscope.hpp @@ -8,7 +8,7 @@ class Oscilloscope : public ODriveIntf::OscilloscopeIntf { public: - Oscilloscope(float* trigger_src, float trigger_threshold, float* data_src) + Oscilloscope(float* trigger_src, float trigger_threshold, float** data_src) : trigger_src_(trigger_src), trigger_threshold_(trigger_threshold), data_src_(data_src) {} float get_val(uint32_t index) override { @@ -20,10 +20,12 @@ public: const uint32_t size_ = OSCILLOSCOPE_SIZE; const float* trigger_src_; const float trigger_threshold_; - const float* data_src_; + float* const * data_src_; float data_[OSCILLOSCOPE_SIZE] = {0}; size_t pos_ = 0; + bool ready_ = false; + bool capturing_ = false; }; #endif // __OSCILLOSCOPE_HPP \ No newline at end of file diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index f4ba60b5..bc982ceb 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -122,6 +122,7 @@ interfaces: sampling: TaskTimer control_loop_misc: TaskTimer control_loop_checks: TaskTimer + dc_calib_wait: TaskTimer system_stats: c_is_class: False attributes: @@ -311,10 +312,12 @@ interfaces: brief: Max current the power supply can sink. doc: You most likely want a non-positive value here. Set to -INFINITY to disable. - gpio1_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[0]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} - gpio2_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[1]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} - gpio3_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[2]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} - gpio4_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} + error_gpio_pin: {type: uint32} + + gpio1_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[0]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} + gpio2_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[1]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} + gpio3_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[2]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} + gpio4_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} gpio3_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} gpio4_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[4]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} user_config_loaded: readonly uint32 @@ -374,6 +377,9 @@ interfaces: bits 1:0: priority (3 is highest priority) 0xffffffff if the specified number is not a valid DMA stream number. doc: Returns information about the specified DMA stream. + get_gpio_states: + out: {status: {type: uint32}} + doc: Returns the logic states of all GPIOs. Bit i represents the state of GPIOi. clear_errors: doc: Clear all the errors of this device including all contained submodules. @@ -1122,6 +1128,7 @@ valuetypes: Enc1: {doc: The pin is used by quadrature encoder 1.} Enc2: {doc: This mode is not supported on ODrive v3.x.} MechBrake: {doc: This is to support external mechanical brakes.} + Status: {doc: The pin is used for status output (see `config.error_gpio_pin`)} ODrive.Can.Protocol: values: {Simple: } diff --git a/tools/odrive/enums.py b/tools/odrive/enums.py index 92620a0a..79649de8 100644 --- a/tools/odrive/enums.py +++ b/tools/odrive/enums.py @@ -19,6 +19,7 @@ GPIO_MODE_ENC0 = 11 GPIO_MODE_ENC1 = 12 GPIO_MODE_ENC2 = 13 GPIO_MODE_MECH_BRAKE = 14 +GPIO_MODE_STATUS = 15 # ODrive.Can.Protocol PROTOCOL_SIMPLE = 0 From 6f99ce478eb6f8f25550d55b096ac906f5540469 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Fri, 13 Nov 2020 10:25:50 +0100 Subject: [PATCH 09/28] use rtos signals in wait_for_control_iteration --- Firmware/MotorControl/axis.cpp | 10 ++++++---- Firmware/MotorControl/axis.hpp | 2 +- Firmware/MotorControl/main.cpp | 7 +++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 8971e415..2a1d5e57 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -104,10 +104,12 @@ void Axis::start_thread() { * @brief Blocks until at least one complete control loop has been executed. */ bool Axis::wait_for_control_iteration() { - uint16_t control_iteration_num = odrv.n_evt_control_loop_; - while (odrv.n_evt_control_loop_ == control_iteration_num) { - osDelay(1); - } + osSignalWait(0x0001, osWaitForever); // this might return instantly + osSignalWait(0x0001, osWaitForever); // this might be triggered at the + // end of a control loop iteration + // which was started before we entered + // this function + osSignalWait(0x0001, osWaitForever); return true; } diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index e9477e0b..50d506d3 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -172,7 +172,7 @@ public: MechanicalBrake& mechanical_brake_; TaskTimes task_times_; - osThreadId thread_id_; + osThreadId thread_id_ = 0; const uint32_t stack_size_ = 2048; // Bytes volatile bool thread_id_valid_ = false; diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index c2342ebe..3daae2d5 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -378,6 +378,13 @@ void ODrive::control_loop_cb(uint32_t timestamp) { axis.motor_.current_control_.update(timestamp); // uses the output of controller_ or open_loop_contoller_ and encoder_ or sensorless_estimator_ or async_estimator_ } + // Tell the axis threads that the control loop has finished + for (auto& axis: axes) { + if (axis.thread_id_) { + osSignalSet(axis.thread_id_, 0x0001); + } + } + get_gpio(odrv.config_.error_gpio_pin).write(odrv.any_error()); } From 8133d6d8aa8cf2517cc0e132575351ec8034fd5b Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Fri, 13 Nov 2020 10:27:44 +0100 Subject: [PATCH 10/28] [odrivetool] make matplotlib optional --- tools/odrive/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/odrive/utils.py b/tools/odrive/utils.py index 0a6ae81e..ca64c758 100755 --- a/tools/odrive/utils.py +++ b/tools/odrive/utils.py @@ -7,7 +7,6 @@ import platform import subprocess import os import numpy as np -import matplotlib.pyplot as plt from fibre.utils import Event import odrive.enums from odrive.enums import * @@ -49,6 +48,7 @@ def calculate_thermistor_coeffs(degree, Rload, R_25, Beta, Tmin, Tmax, plot = Fa fit_temps = p1(V) if plot: + import matplotlib.pyplot as plt print(fit) plt.plot(V, temps, label='actual') plt.plot(V, fit_temps, label='fit') @@ -547,6 +547,8 @@ def dump_dma(odrv): "*" if (status & 0x80000000) else " ")) def dump_timing(odrv, n_samples=100, path='/tmp/timings.png'): + import matplotlib.pyplot as plt + timings = [] for attr in dir(odrv.task_times): @@ -574,7 +576,7 @@ def dump_timing(odrv, n_samples=100, path='/tmp/timings.png'): plt.rcParams['figure.figsize'] = 21, 9 plt.figure() - plt.grid('both') + plt.grid(True) plt.barh( [-i for i in range(len(timings))], # y positions [np.mean(lengths) for name, obj, start_times, lengths in timings], # lengths From 9d37b7516ddb0732809763f69c0dcb6a50c29b4a Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Fri, 13 Nov 2020 18:41:58 +0100 Subject: [PATCH 11/28] add DEFAULT_MIN_DC_VOLTAGE define --- Firmware/Board/v3/Inc/board.h | 1 + Firmware/MotorControl/odrive_main.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Firmware/Board/v3/Inc/board.h b/Firmware/Board/v3/Inc/board.h index 4820b93b..a6fbab36 100644 --- a/Firmware/Board/v3/Inc/board.h +++ b/Firmware/Board/v3/Inc/board.h @@ -42,6 +42,7 @@ #endif #define DEFAULT_ERROR_PIN 0 +#define DEFAULT_MIN_DC_VOLTAGE 8.0f #define DEFAULT_GPIO_MODES \ ODriveIntf::GPIO_MODE_DIGITAL, \ diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h index c88618bf..20074a93 100644 --- a/Firmware/MotorControl/odrive_main.h +++ b/Firmware/MotorControl/odrive_main.h @@ -75,7 +75,7 @@ struct BoardConfig_t { float max_regen_current = 0.0f; float brake_resistance = DEFAULT_BRAKE_RESISTANCE; bool enable_brake_resistor = false; - float dc_bus_undervoltage_trip_level = 8.0f; // Date: Mon, 16 Nov 2020 16:30:58 +0100 Subject: [PATCH 12/28] abort save_configuration when armed --- Firmware/MotorControl/main.cpp | 30 +++++++++++++++++------------ Firmware/MotorControl/odrive_main.h | 2 +- Firmware/odrive-interface.yaml | 2 +- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 3daae2d5..d8bc9849 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -107,19 +107,25 @@ static bool config_apply_all() { return success; } -void ODrive::save_configuration(void) { - size_t config_size = 0; - bool success = config_manager.prepare_store() - && config_write_all() - && config_manager.start_store(&config_size) - && config_write_all() - && config_manager.finish_store(); - if (success) { - user_config_loaded_ = config_size; - } else { - printf("saving configuration failed\r\n"); - osDelay(5); +bool ODrive::save_configuration(void) { + bool success = false; + + CRITICAL_SECTION() { + bool any_armed = std::any_of(axes.begin(), axes.end(), + [](auto& axis){ return axis.motor_.is_armed_; }); + if (any_armed) { + return false; + } + + size_t config_size = 0; + success = config_manager.prepare_store() + && config_write_all() + && config_manager.start_store(&config_size) + && config_write_all() + && config_manager.finish_store(); } + + return success; } void ODrive::erase_configuration(void) { diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h index 20074a93..17b5aa01 100644 --- a/Firmware/MotorControl/odrive_main.h +++ b/Firmware/MotorControl/odrive_main.h @@ -167,7 +167,7 @@ static Stm32Gpio get_gpio(size_t gpio_num) { // general system functions defined in main.cpp class ODrive : public ODriveIntf { public: - void save_configuration() override; + bool save_configuration() override; void erase_configuration() override; void reboot() override { NVIC_SystemReset(); } void enter_dfu_mode() override; diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index bc982ceb..372be420 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -351,7 +351,7 @@ interfaces: functions: test_function: {in: {delta: int32}, out: {cnt: int32}} get_adc_voltage: {in: {gpio: uint32}, out: {voltage: float32}, doc: Reads the ADC voltage of the specified GPIO. The GPIO should be in `GPIO_MODE_ANALOG_IN`.} - save_configuration: + save_configuration: {out: {success: bool}} erase_configuration: reboot: enter_dfu_mode: From 60b433700fddfdc38f830b6529fb3bbed5905543 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 17:15:00 +0100 Subject: [PATCH 13/28] fix uninitialized warning after CRITICAL_SECTION --- Firmware/Drivers/STM32/stm32_system.h | 5 +++++ Firmware/MotorControl/main.cpp | 2 +- Firmware/MotorControl/motor.cpp | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Firmware/Drivers/STM32/stm32_system.h b/Firmware/Drivers/STM32/stm32_system.h index 5a95e590..e065cb24 100644 --- a/Firmware/Drivers/STM32/stm32_system.h +++ b/Firmware/Drivers/STM32/stm32_system.h @@ -50,13 +50,18 @@ struct CriticalSectionContext { CriticalSectionContext(const CriticalSectionContext&&) = delete; void operator=(const CriticalSectionContext&) = delete; void operator=(const CriticalSectionContext&&) = delete; + operator bool() { return true; }; CriticalSectionContext() : mask_(cpu_enter_critical()) {} ~CriticalSectionContext() { cpu_exit_critical(mask_); } uint32_t mask_; bool exit_ = false; }; +#ifdef __clang__ #define CRITICAL_SECTION() for (CriticalSectionContext __critical_section_context; !__critical_section_context.exit_; __critical_section_context.exit_ = true) +#else +#define CRITICAL_SECTION() if (CriticalSectionContext __critical_section_context{}) +#endif #endif diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index d8bc9849..67e2c46d 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -108,7 +108,7 @@ static bool config_apply_all() { } bool ODrive::save_configuration(void) { - bool success = false; + bool success; CRITICAL_SECTION() { bool any_armed = std::any_of(axes.begin(), axes.end(), diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index be41bd3b..a81cdc2a 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -256,7 +256,7 @@ void Motor::apply_pwm_timings(uint16_t timings[3], bool tentative) { * arm() is called. */ bool Motor::disarm(bool* p_was_armed) { - bool was_armed = false; + bool was_armed; CRITICAL_SECTION() { was_armed = is_armed_; From b265a3388f668a0d53e36d4953382ace86fab932 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 16:20:52 +0100 Subject: [PATCH 14/28] ignore current readings when MOE==0 --- Firmware/Board/v3/board.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Firmware/Board/v3/board.cpp b/Firmware/Board/v3/board.cpp index bfc56bea..d9f768a8 100644 --- a/Firmware/Board/v3/board.cpp +++ b/Firmware/Board/v3/board.cpp @@ -506,6 +506,18 @@ void ControlLoop_IRQHandler(void) { motors[1].disarm_with_error(Motor::ERROR_BAD_TIMING); } + // If the motor FETs are not switching then we can't measure the current + // because for this we need the low side FET to conduct. + // So for now we guess the current to be 0 (this is not correct shortly after + // disarming and when the motor spins fast in idle). Passing an invalid + // current reading would create problems with starting FOC. + if (!(TIM1->BDTR & TIM_BDTR_MOE_Msk)) { + current0 = {0.0f, 0.0f}; + } + if (!(TIM8->BDTR & TIM_BDTR_MOE_Msk)) { + current1 = {0.0f, 0.0f}; + } + motors[0].current_meas_cb(timestamp - TIM1_INIT_COUNT, current0); motors[1].current_meas_cb(timestamp, current1); From b32a455dda4fde2936e0534fb70132d869733e1c Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 18:14:16 +0100 Subject: [PATCH 15/28] fix HWIL tests --- Firmware/odrive-interface.yaml | 1 + tools/odrive/tests/closed_loop_test.py | 3 +++ tools/odrive/tests/integration_test.py | 4 +++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index 372be420..fee4f9eb 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -258,6 +258,7 @@ interfaces: Setting this to False even though a brake resistor is connected is harmless. Setting this to True even though no brake resistor is connected can break the power supply. + Changes to this value require a reboot to take effect. dc_bus_undervoltage_trip_level: type: float32 diff --git a/tools/odrive/tests/closed_loop_test.py b/tools/odrive/tests/closed_loop_test.py index 26663fd0..9b64d1bf 100644 --- a/tools/odrive/tests/closed_loop_test.py +++ b/tools/odrive/tests/closed_loop_test.py @@ -40,6 +40,9 @@ class TestClosedLoopControlBase(): # Set brake resistor settings axis_ctx.parent.handle.config.brake_resistance = float(axis_ctx.parent.yaml['brake-resistance']) + # The docs say this requires a reboot but here's a small secret: + # Since the brake resistor is also started in clear_errors() this + # circumvents the need for a reboot. axis_ctx.parent.handle.config.enable_brake_resistor = True # Set calibration settings diff --git a/tools/odrive/tests/integration_test.py b/tools/odrive/tests/integration_test.py index a142e15a..e3d76007 100644 --- a/tools/odrive/tests/integration_test.py +++ b/tools/odrive/tests/integration_test.py @@ -106,6 +106,8 @@ class TestSimpleCANClosedLoop(): # Make sure there are no funny configurations active logger.debug('Setting up clean configuration...') axis_ctx.parent.erase_config_and_reboot() + axis_ctx.parent.handle.config.enable_brake_resistor = True + axis_ctx.parent.save_config_and_reboot() # run calibration axis_ctx.handle.requested_state = AXIS_STATE_FULL_CALIBRATION_SEQUENCE @@ -162,7 +164,7 @@ class TestSimpleCANClosedLoop(): test_assert_eq(axis_ctx.handle.config.can.node_id, node_id+20) # Reset node ID to default value - asyncio.run(command(canbus.handle, node_id+20, extended_id, 'set_node_id', node_id=node_id)) + command(canbus.handle, node_id+20, extended_id, 'set_node_id', node_id=node_id) fence() test_assert_eq(axis_ctx.handle.config.can.node_id, node_id) From 18a391c092a2dfcd190fe25077f8b3c119427ed2 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 18:41:28 +0100 Subject: [PATCH 16/28] Reduce kI in motor calibration This prevents current overshoot if the motor snaps to align with I_alpha. Consequently we can reenable current limit checks during motor calibration. --- Firmware/MotorControl/motor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index a81cdc2a..ca9753c3 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -63,7 +63,7 @@ struct ResistanceMeasurementControlLaw : AlphaBetaFrameController { return test_voltage_ / target_current_; } - const float kI = 10.0f; // [(V/s)/A] + const float kI = 1.0f; // [(V/s)/A] float max_voltage_ = 0.0f; float actual_current_ = 0.0f; float target_current_ = 0.0f; @@ -614,7 +614,7 @@ void Motor::current_meas_cb(uint32_t timestamp, std::optional current // Hack: we disable the current check during motor calibration because // it tends to briefly overshoot when the motor moves to align flux with I_alpha - if (Inorm_sq > SQ(Itrip) && (axis_->current_state_ != Axis::AXIS_STATE_MOTOR_CALIBRATION)) { + if (Inorm_sq > SQ(Itrip)) { disarm_with_error(ERROR_CURRENT_LIMIT_VIOLATION); } } else if (is_armed_) { From c32cbec58cca0aa92dc3f7d09402d1f162cb36a6 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 20:58:26 +0100 Subject: [PATCH 17/28] rename some of the new functions and variables --- ...async_estimator.cpp => acim_estimator.cpp} | 10 +++---- ...async_estimator.hpp => acim_estimator.hpp} | 8 +++--- Firmware/MotorControl/axis.cpp | 14 +++++----- Firmware/MotorControl/axis.hpp | 6 ++--- Firmware/MotorControl/component.hpp | 24 ++++++++--------- Firmware/MotorControl/controller.cpp | 18 ++++++------- Firmware/MotorControl/encoder.cpp | 12 ++++----- Firmware/MotorControl/foc.cpp | 8 +++--- Firmware/MotorControl/main.cpp | 10 +++---- Firmware/MotorControl/motor.cpp | 16 ++++++------ .../MotorControl/open_loop_controller.cpp | 10 +++---- .../MotorControl/sensorless_estimator.cpp | 2 +- Firmware/Tupfile.lua | 2 +- Firmware/communication/ascii_protocol.cpp | 4 +-- Firmware/communication/can_simple.cpp | 6 ++--- Firmware/odrive-interface.yaml | 26 +++++++++---------- 16 files changed, 88 insertions(+), 88 deletions(-) rename Firmware/MotorControl/{async_estimator.cpp => acim_estimator.cpp} (86%) rename Firmware/MotorControl/{async_estimator.hpp => acim_estimator.hpp} (85%) diff --git a/Firmware/MotorControl/async_estimator.cpp b/Firmware/MotorControl/acim_estimator.cpp similarity index 86% rename from Firmware/MotorControl/async_estimator.cpp rename to Firmware/MotorControl/acim_estimator.cpp index e2edefa4..55cc1be8 100644 --- a/Firmware/MotorControl/async_estimator.cpp +++ b/Firmware/MotorControl/acim_estimator.cpp @@ -1,11 +1,11 @@ -#include "async_estimator.hpp" +#include "acim_estimator.hpp" #include -void AsyncEstimator::update(uint32_t timestamp) { - std::optional rotor_phase = rotor_phase_src_.get_current(); - std::optional rotor_phase_vel = rotor_phase_vel_src_.get_current(); - std::optional idq = idq_src_.get_current(); +void AcimEstimator::update(uint32_t timestamp) { + std::optional rotor_phase = rotor_phase_src_.present(); + std::optional rotor_phase_vel = rotor_phase_vel_src_.present(); + std::optional idq = idq_src_.present(); if (!rotor_phase.has_value() || !rotor_phase_vel.has_value() || !idq.has_value()) { active_ = false; diff --git a/Firmware/MotorControl/async_estimator.hpp b/Firmware/MotorControl/acim_estimator.hpp similarity index 85% rename from Firmware/MotorControl/async_estimator.hpp rename to Firmware/MotorControl/acim_estimator.hpp index 3505ce53..242b5d86 100644 --- a/Firmware/MotorControl/async_estimator.hpp +++ b/Firmware/MotorControl/acim_estimator.hpp @@ -1,11 +1,11 @@ -#ifndef __ASYNC_ESTIMATOR_HPP -#define __ASYNC_ESTIMATOR_HPP +#ifndef __ACIM_ESTIMATOR_HPP +#define __ACIM_ESTIMATOR_HPP #include #include #include -class AsyncEstimator : public ComponentBase { +class AcimEstimator : public ComponentBase { public: struct Config_t { float slip_velocity = 14.706f; // [rad/s electrical] = 1/rotor_tau @@ -33,4 +33,4 @@ public: OutputPort stator_phase_ = 0.0f; // [rad] rotor flux phase angle estimate }; -#endif // __ASYNC_ESTIMATOR_HPP \ No newline at end of file +#endif // __ACIM_ESTIMATOR_HPP \ No newline at end of file diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 2a1d5e57..6c8e6663 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -204,11 +204,11 @@ bool Axis::run_lockin_spin(const LockinConfig_t &lockin_config, bool remain_arme motor_.current_control_.Vdq_setpoint_src_.connect_to(&open_loop_controller_.Vdq_setpoint_); motor_.current_control_.phase_src_.connect_to(&open_loop_controller_.phase_); - async_estimator_.rotor_phase_src_.connect_to(&open_loop_controller_.phase_); + acim_estimator_.rotor_phase_src_.connect_to(&open_loop_controller_.phase_); motor_.phase_vel_src_.connect_to(&open_loop_controller_.phase_vel_); motor_.current_control_.phase_vel_src_.connect_to(&open_loop_controller_.phase_vel_); - async_estimator_.rotor_phase_vel_src_.connect_to(&open_loop_controller_.phase_vel_); + acim_estimator_.rotor_phase_vel_src_.connect_to(&open_loop_controller_.phase_vel_); } wait_for_control_iteration(); @@ -219,8 +219,8 @@ bool Axis::run_lockin_spin(const LockinConfig_t &lockin_config, bool remain_arme float dir = lockin_config.vel >= 0.0f ? 1.0f : -1.0f; while ((requested_state_ == AXIS_STATE_UNDEFINED) && motor_.is_armed_) { - bool reached_target_vel = std::abs(open_loop_controller_.phase_vel_.get_any().value_or(0.0f) - lockin_config.vel) <= std::numeric_limits::epsilon(); - bool reached_target_dist = open_loop_controller_.total_distance_.get_any().value_or(0.0f) * dir >= lockin_config.finish_distance * dir; + bool reached_target_vel = std::abs(open_loop_controller_.phase_vel_.any().value_or(0.0f) - lockin_config.vel) <= std::numeric_limits::epsilon(); + bool reached_target_dist = open_loop_controller_.total_distance_.any().value_or(0.0f) * dir >= lockin_config.finish_distance * dir; // Check if terminal condition is reached bool terminal_condition = (reached_target_vel && lockin_config.finish_on_vel) @@ -286,7 +286,7 @@ bool Axis::start_closed_loop_control() { if (controller_.config_.control_mode >= Controller::CONTROL_MODE_POSITION_CONTROL) { std::optional pos_init = (controller_.config_.circular_setpoints ? controller_.pos_estimate_circular_src_ : - controller_.pos_estimate_linear_src_).get_any(); + controller_.pos_estimate_linear_src_).any(); if (!pos_init.has_value()) { return false; } else { @@ -308,12 +308,12 @@ bool Axis::start_closed_loop_control() { OutputPort* phase_src = sensorless_mode ? &sensorless_estimator_.phase_ : &encoder_.phase_; motor_.current_control_.phase_src_.connect_to(phase_src); - async_estimator_.rotor_phase_src_.connect_to(phase_src); + acim_estimator_.rotor_phase_src_.connect_to(phase_src); OutputPort* phase_vel_src = sensorless_mode ? &sensorless_estimator_.phase_vel_ : &encoder_.phase_vel_; motor_.phase_vel_src_.connect_to(phase_vel_src); motor_.current_control_.phase_vel_src_.connect_to(phase_vel_src); - async_estimator_.rotor_phase_vel_src_.connect_to(phase_vel_src); + acim_estimator_.rotor_phase_vel_src_.connect_to(phase_vel_src); if (sensorless_mode) { // Make the final velocity of the loĉk-in spin the setpoint of the diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index 50d506d3..c79c91dd 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -4,7 +4,7 @@ class Axis; #include "encoder.hpp" -#include "async_estimator.hpp" +#include "acim_estimator.hpp" #include "sensorless_estimator.hpp" #include "controller.hpp" #include "open_loop_controller.hpp" @@ -39,7 +39,7 @@ public: TaskTimer can_heartbeat; TaskTimer controller_update; TaskTimer open_loop_controller_update; - TaskTimer async_estimator_update; + TaskTimer acim_estimator_update; TaskTimer motor_update; TaskTimer current_controller_update; TaskTimer dc_calib; @@ -161,7 +161,7 @@ public: Config_t config_; Encoder& encoder_; - AsyncEstimator async_estimator_; + AcimEstimator acim_estimator_; SensorlessEstimator& sensorless_estimator_; Controller& controller_; OpenLoopController open_loop_controller_; diff --git a/Firmware/MotorControl/component.hpp b/Firmware/MotorControl/component.hpp index aa156e1c..4569de99 100644 --- a/Firmware/MotorControl/component.hpp +++ b/Firmware/MotorControl/component.hpp @@ -40,8 +40,8 @@ public: /** * @brief Initializes the output port with the specified value. * - * An initialization value is required for get_any() to work properly. - * get_current() and get_previous() cannot be used to fetch the + * An initialization value is required for any() to work properly. + * present() and previous() cannot be used to fetch the * initialization value. */ OutputPort(T val) : content_(val) {} @@ -60,7 +60,7 @@ public: * of this class. */ void reset() { - // This will eventually overflow to 0 so get_current() could + // This will eventually overflow to 0 so present() could // theoretically return a very old value however it is very likely that // the motor will be long disarmed by then. age_++; @@ -70,7 +70,7 @@ public: * @brief Returns the value from this control loop iteration or std::nullopt * if the value was not yet set during this control loop iteration. */ - std::optional get_current() { + std::optional present() { if (age_ == 0) { return content_; } else { @@ -85,7 +85,7 @@ public: * overwritten during this control loop iteration then this function returns * std::nullopt. */ - std::optional get_previous() { + std::optional previous() { if (age_ == 1) { return content_; } else { @@ -99,7 +99,7 @@ public: * * This function is thread-safe if load/store operations of T are atomic. */ - std::optional get_any() { + std::optional any() { return content_; } @@ -134,10 +134,10 @@ public: content_ = (OutputPort*)nullptr; } - std::optional get_current() { + std::optional present() { if (content_.index() == 2) { OutputPort* ptr = std::get<2>(content_); - return ptr ? ptr->get_current() : std::nullopt; + return ptr ? ptr->present() : std::nullopt; } else if (content_.index() == 1) { T* ptr = std::get<1>(content_); return ptr ? std::make_optional(*ptr) : std::nullopt; @@ -150,10 +150,10 @@ public: // ok for this input port to fetch the value from the last iteration. // This would provide a general way to resolve same-iteration data path cycles. - //std::optional get_previous() { + //std::optional previous() { // if (content_.index() == 2) { // OutputPort* ptr = std::get<2>(content_); - // return ptr ? ptr->get_previous() : std::nullopt; + // return ptr ? ptr->previous() : std::nullopt; // } else if (content_.index() == 1) { // T* ptr = std::get<1>(content_); // return ptr ? std::make_optional(*ptr) : std::nullopt; @@ -162,10 +162,10 @@ public: // } //} - std::optional get_any() { + std::optional any() { if (content_.index() == 2) { OutputPort* ptr = std::get<2>(content_); - return ptr ? ptr->get_any() : std::nullopt; + return ptr ? ptr->any() : std::nullopt; } else if (content_.index() == 1) { T* ptr = std::get<1>(content_); return ptr ? std::make_optional(*ptr) : std::nullopt; diff --git a/Firmware/MotorControl/controller.cpp b/Firmware/MotorControl/controller.cpp index 51c64983..9c4e97ca 100644 --- a/Firmware/MotorControl/controller.cpp +++ b/Firmware/MotorControl/controller.cpp @@ -97,13 +97,13 @@ static float limitVel(const float vel_limit, const float vel_estimate, const flo } bool Controller::update() { - std::optional pos_estimate_linear = pos_estimate_linear_src_.get_current(); - std::optional pos_estimate_circular = pos_estimate_circular_src_.get_current(); - std::optional pos_wrap = pos_wrap_src_.get_current(); - std::optional vel_estimate = vel_estimate_src_.get_current(); + std::optional pos_estimate_linear = pos_estimate_linear_src_.present(); + std::optional pos_estimate_circular = pos_estimate_circular_src_.present(); + std::optional pos_wrap = pos_wrap_src_.present(); + std::optional vel_estimate = vel_estimate_src_.present(); - std::optional anticogging_pos_estimate = axis_->encoder_.pos_estimate_.get_current(); - std::optional anticogging_vel_estimate = axis_->encoder_.vel_estimate_.get_current(); + std::optional anticogging_pos_estimate = axis_->encoder_.pos_estimate_.present(); + std::optional anticogging_vel_estimate = axis_->encoder_.vel_estimate_.present(); if (config_.anticogging.calib_anticogging) { if (!anticogging_pos_estimate.has_value() || !anticogging_vel_estimate.has_value()) { @@ -156,8 +156,8 @@ bool Controller::update() { } break; case INPUT_MODE_MIRROR: { if (config_.axis_to_mirror < AXIS_COUNT) { - std::optional other_pos = axes[config_.axis_to_mirror].encoder_.pos_estimate_.get_current(); - std::optional other_vel = axes[config_.axis_to_mirror].encoder_.vel_estimate_.get_current(); + std::optional other_pos = axes[config_.axis_to_mirror].encoder_.pos_estimate_.present(); + std::optional other_vel = axes[config_.axis_to_mirror].encoder_.vel_estimate_.present(); if (!other_pos.has_value() || !other_vel.has_value()) { set_error(ERROR_INVALID_ESTIMATE); @@ -262,7 +262,7 @@ bool Controller::update() { float vel_gain = config_.vel_gain; float vel_integrator_gain = config_.vel_integrator_gain; if (axis_->motor_.config_.motor_type == Motor::MOTOR_TYPE_ACIM) { - float effective_flux = axis_->async_estimator_.rotor_flux_; + float effective_flux = axis_->acim_estimator_.rotor_flux_; float minflux = axis_->motor_.config_.acim_gain_min_flux; if (std::abs(effective_flux) < minflux) effective_flux = std::copysignf(minflux, effective_flux); diff --git a/Firmware/MotorControl/encoder.cpp b/Firmware/MotorControl/encoder.cpp index 7132100e..358dca72 100644 --- a/Firmware/MotorControl/encoder.cpp +++ b/Firmware/MotorControl/encoder.cpp @@ -235,11 +235,11 @@ bool Encoder::run_offset_calibration() { axis_->motor_.current_control_.Vdq_setpoint_src_.connect_to(&axis_->open_loop_controller_.Vdq_setpoint_); axis_->motor_.current_control_.phase_src_.connect_to(&axis_->open_loop_controller_.phase_); - axis_->async_estimator_.rotor_phase_src_.connect_to(&axis_->open_loop_controller_.phase_); + axis_->acim_estimator_.rotor_phase_src_.connect_to(&axis_->open_loop_controller_.phase_); axis_->motor_.phase_vel_src_.connect_to(&axis_->open_loop_controller_.phase_vel_); axis_->motor_.current_control_.phase_vel_src_.connect_to(&axis_->open_loop_controller_.phase_vel_); - axis_->async_estimator_.rotor_phase_vel_src_.connect_to(&axis_->open_loop_controller_.phase_vel_); + axis_->acim_estimator_.rotor_phase_vel_src_.connect_to(&axis_->open_loop_controller_.phase_vel_); } axis_->wait_for_control_iteration(); @@ -269,7 +269,7 @@ bool Encoder::run_offset_calibration() { // scan forward while ((axis_->requested_state_ == Axis::AXIS_STATE_UNDEFINED) && axis_->motor_.is_armed_) { - bool reached_target_dist = axis_->open_loop_controller_.total_distance_.get_any().value_or(-INFINITY) >= config_.calib_scan_distance; + bool reached_target_dist = axis_->open_loop_controller_.total_distance_.any().value_or(-INFINITY) >= config_.calib_scan_distance; if (reached_target_dist) { break; } @@ -308,7 +308,7 @@ bool Encoder::run_offset_calibration() { // scan backwards while ((axis_->requested_state_ == Axis::AXIS_STATE_UNDEFINED) && axis_->motor_.is_armed_) { - bool reached_target_dist = axis_->open_loop_controller_.total_distance_.get_any().value_or(INFINITY) <= 0.0f; + bool reached_target_dist = axis_->open_loop_controller_.total_distance_.any().value_or(INFINITY) <= 0.0f; if (reached_target_dist) { break; } @@ -590,7 +590,7 @@ bool Encoder::update() { // TODO: we should strictly require that this value is from the previous iteration // to avoid spinout scenarios. However that requires a proper way to reset // the encoder from error states. - float pos_circular = pos_circular_.get_any().value_or(0.0f); + float pos_circular = pos_circular_.any().value_or(0.0f); pos_circular += wrap_pm((pos_cpr_counts_ - pos_cpr_counts_last) / (float)config_.cpr, 1.0f); pos_circular = fmodf_pos(pos_circular, axis_->controller_.config_.circular_setpoint_range); pos_circular_ = pos_circular; @@ -622,7 +622,7 @@ bool Encoder::update() { if (is_ready_) { phase_ = wrap_pm_pi(ph) * config_.direction; - phase_vel_ = (2*M_PI) * *vel_estimate_.get_current() * axis_->motor_.config_.pole_pairs * config_.direction; + phase_vel_ = (2*M_PI) * *vel_estimate_.present() * axis_->motor_.config_.pole_pairs * config_.direction; } return true; diff --git a/Firmware/MotorControl/foc.cpp b/Firmware/MotorControl/foc.cpp index ae4a05af..219f4076 100644 --- a/Firmware/MotorControl/foc.cpp +++ b/Firmware/MotorControl/foc.cpp @@ -183,9 +183,9 @@ void FieldOrientedController::update(uint32_t timestamp) { CRITICAL_SECTION() { ctrl_timestamp_ = timestamp; enable_current_control_ = enable_current_control_src_; - Idq_setpoint_ = Idq_setpoint_src_.get_current(); - Vdq_setpoint_ = Vdq_setpoint_src_.get_current(); - phase_ = phase_src_.get_current(); - phase_vel_ = phase_vel_src_.get_current(); + Idq_setpoint_ = Idq_setpoint_src_.present(); + Vdq_setpoint_ = Vdq_setpoint_src_.present(); + phase_ = phase_src_.present(); + phase_vel_ = phase_vel_src_.present(); } } diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 67e2c46d..834ce743 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -306,9 +306,9 @@ void ODrive::control_loop_cb(uint32_t timestamp) { // TODO: maybe we should add a check to output ports that prevents // double-setting the value. for (auto& axis: axes) { - axis.async_estimator_.slip_vel_.reset(); - axis.async_estimator_.stator_phase_vel_.reset(); - axis.async_estimator_.stator_phase_.reset(); + axis.acim_estimator_.slip_vel_.reset(); + axis.acim_estimator_.stator_phase_vel_.reset(); + axis.acim_estimator_.stator_phase_.reset(); axis.controller_.torque_output_.reset(); axis.encoder_.phase_.reset(); axis.encoder_.phase_vel_.reset(); @@ -381,7 +381,7 @@ void ODrive::control_loop_cb(uint32_t timestamp) { axis.motor_.update(timestamp); // uses torque from controller and phase_vel from encoder MEASURE_TIME(axis.task_times_.current_controller_update) - axis.motor_.current_control_.update(timestamp); // uses the output of controller_ or open_loop_contoller_ and encoder_ or sensorless_estimator_ or async_estimator_ + axis.motor_.current_control_.update(timestamp); // uses the output of controller_ or open_loop_contoller_ and encoder_ or sensorless_estimator_ or acim_estimator_ } // Tell the axis threads that the control loop has finished @@ -480,7 +480,7 @@ static void rtos_main(void*) { } for(auto& axis: axes){ - axis.async_estimator_.idq_src_.connect_to(&axis.motor_.Idq_setpoint_); + axis.acim_estimator_.idq_src_.connect_to(&axis.motor_.Idq_setpoint_); } // Start PWM and enable adc interrupts/callbacks diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index ca9753c3..6585cccc 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -193,7 +193,7 @@ bool Motor::arm(PhaseControlLaw<3>* control_law) { // Reset controller states, integrators, setpoints, etc. axis_->controller_.reset(); - axis_->async_estimator_.rotor_flux_ = 0.0f; + axis_->acim_estimator_.rotor_flux_ = 0.0f; if (control_law_) { control_law_->reset(); } @@ -373,7 +373,7 @@ float Motor::effective_current_lim() { //Note - for ACIM motors, available torque is allowed to be 0. float Motor::max_available_torque() { if (config_.motor_type == Motor::MOTOR_TYPE_ACIM) { - float max_torque = effective_current_lim_ * config_.torque_constant * axis_->async_estimator_.rotor_flux_; + float max_torque = effective_current_lim_ * config_.torque_constant * axis_->acim_estimator_.rotor_flux_; max_torque = std::clamp(max_torque, 0.0f, config_.torque_lim); return max_torque; } else { @@ -494,19 +494,19 @@ bool Motor::run_calibration() { } void Motor::update(uint32_t timestamp) { - std::optional torque = torque_setpoint_src_.get_current(); + std::optional torque = torque_setpoint_src_.present(); if (!torque.has_value()) { error_ |= ERROR_UNKNOWN_TORQUE; return; } - auto [id, iq] = Idq_setpoint_.get_previous() + auto [id, iq] = Idq_setpoint_.previous() .value_or(float2D{0.0f, 0.0f}); // Id doubles as a state variable // Convert torque to current if (axis_->motor_.config_.motor_type == Motor::MOTOR_TYPE_ACIM) { - iq = *torque / (axis_->motor_.config_.torque_constant * std::max(axis_->async_estimator_.rotor_flux_, config_.acim_gain_min_flux)); + iq = *torque / (axis_->motor_.config_.torque_constant * std::max(axis_->acim_estimator_.rotor_flux_, config_.acim_gain_min_flux)); } else { iq = *torque / axis_->motor_.config_.torque_constant; } @@ -534,13 +534,13 @@ void Motor::update(uint32_t timestamp) { // in this function. // A cleaner fix would be to take the feedforward calculation out of here // and turn it into a separate component. - MEASURE_TIME(axis_->task_times_.async_estimator_update) - axis_->async_estimator_.update(timestamp); + MEASURE_TIME(axis_->task_times_.acim_estimator_update) + axis_->acim_estimator_.update(timestamp); float vd = 0.0f; float vq = 0.0f; - std::optional phase_vel = phase_vel_src_.get_current(); + std::optional phase_vel = phase_vel_src_.present(); if (config_.R_wL_FF_enable) { if (!phase_vel.has_value()) { diff --git a/Firmware/MotorControl/open_loop_controller.cpp b/Firmware/MotorControl/open_loop_controller.cpp index 16d2d5db..7d21df98 100644 --- a/Firmware/MotorControl/open_loop_controller.cpp +++ b/Firmware/MotorControl/open_loop_controller.cpp @@ -3,10 +3,10 @@ #include void OpenLoopController::update(uint32_t timestamp) { - auto [prev_Id, prev_Iq] = Idq_setpoint_.get_previous().value_or(float2D{0.0f, 0.0f}); - auto [prev_Vd, prev_Vq] = Vdq_setpoint_.get_previous().value_or(float2D{0.0f, 0.0f}); - float phase = phase_.get_previous().value_or(initial_phase_); - float phase_vel = phase_vel_.get_previous().value_or(0.0f); + auto [prev_Id, prev_Iq] = Idq_setpoint_.previous().value_or(float2D{0.0f, 0.0f}); + auto [prev_Vd, prev_Vq] = Vdq_setpoint_.previous().value_or(float2D{0.0f, 0.0f}); + float phase = phase_.previous().value_or(initial_phase_); + float phase_vel = phase_vel_.previous().value_or(0.0f); (void)prev_Iq; // unused (void)prev_Vq; // unused @@ -25,6 +25,6 @@ void OpenLoopController::update(uint32_t timestamp) { phase_vel = std::clamp(target_vel_, phase_vel - max_phase_vel_ramp_ * dt, phase_vel + max_phase_vel_ramp_ * dt); phase_vel_ = phase_vel; phase_ = wrap_pm_pi(phase + phase_vel * dt); - total_distance_ = total_distance_.get_previous().value_or(0.0f) + phase_vel * dt; + total_distance_ = total_distance_.previous().value_or(0.0f) + phase_vel * dt; timestamp_ = timestamp; } diff --git a/Firmware/MotorControl/sensorless_estimator.cpp b/Firmware/MotorControl/sensorless_estimator.cpp index c3951056..84f12f55 100644 --- a/Firmware/MotorControl/sensorless_estimator.cpp +++ b/Firmware/MotorControl/sensorless_estimator.cpp @@ -81,7 +81,7 @@ bool SensorlessEstimator::update() { V_alpha_beta_memory_[0] = axis_->motor_.current_control_.final_v_alpha_; V_alpha_beta_memory_[1] = axis_->motor_.current_control_.final_v_beta_; - float phase_vel = phase_vel_.get_previous().value_or(0.0f); + float phase_vel = phase_vel_.previous().value_or(0.0f); // predict PLL phase with velocity pll_pos_ = wrap_pm_pi(pll_pos_ + current_meas_period * phase_vel); diff --git a/Firmware/Tupfile.lua b/Firmware/Tupfile.lua index 611e174c..90eecd2c 100644 --- a/Firmware/Tupfile.lua +++ b/Firmware/Tupfile.lua @@ -200,7 +200,7 @@ sources = { 'MotorControl/thermistor.cpp', 'MotorControl/encoder.cpp', 'MotorControl/endstop.cpp', - 'MotorControl/async_estimator.cpp', + 'MotorControl/acim_estimator.cpp', 'MotorControl/mechanical_brake.cpp', 'MotorControl/controller.cpp', 'MotorControl/foc.cpp', diff --git a/Firmware/communication/ascii_protocol.cpp b/Firmware/communication/ascii_protocol.cpp index 6e62e8a6..578c3f7b 100644 --- a/Firmware/communication/ascii_protocol.cpp +++ b/Firmware/communication/ascii_protocol.cpp @@ -284,8 +284,8 @@ void cmd_get_feedback(char * pStr, StreamSink& response_channel, bool use_checks } else { Axis& axis = axes[motor_number]; respond(response_channel, use_checksum, "%f %f", - (double)axis.encoder_.pos_estimate_.get_any().value_or(0.0f), - (double)axis.encoder_.vel_estimate_.get_any().value_or(0.0f)); + (double)axis.encoder_.pos_estimate_.any().value_or(0.0f), + (double)axis.encoder_.vel_estimate_.any().value_or(0.0f)); } } diff --git a/Firmware/communication/can_simple.cpp b/Firmware/communication/can_simple.cpp index 650dd6e1..af897495 100644 --- a/Firmware/communication/can_simple.cpp +++ b/Firmware/communication/can_simple.cpp @@ -173,8 +173,8 @@ int32_t CANSimple::get_encoder_estimates_callback(const Axis& axis) { txmsg.isExt = axis.config_.can.is_extended; txmsg.len = 8; - can_setSignal(txmsg, axis.encoder_.pos_estimate_.get_any().value_or(0.0f), 0, 32, true); - can_setSignal(txmsg, axis.encoder_.vel_estimate_.get_any().value_or(0.0f), 32, 32, true); + can_setSignal(txmsg, axis.encoder_.pos_estimate_.any().value_or(0.0f), 0, 32, true); + can_setSignal(txmsg, axis.encoder_.vel_estimate_.any().value_or(0.0f), 32, 32, true); return odCAN->write(txmsg); } @@ -189,7 +189,7 @@ int32_t CANSimple::get_sensorless_estimates_callback(const Axis& axis) { static_assert(sizeof(float) == sizeof(axis.sensorless_estimator_.pll_pos_)); can_setSignal(txmsg, axis.sensorless_estimator_.pll_pos_, 0, 32, true); - can_setSignal(txmsg, axis.sensorless_estimator_.vel_estimate_.get_any().value_or(0.0f), 32, 32, true); + can_setSignal(txmsg, axis.sensorless_estimator_.vel_estimate_.any().value_or(0.0f), 32, 32, true); return odCAN->write(txmsg); } diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index fee4f9eb..83f294b4 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -506,7 +506,7 @@ interfaces: motor: Motor controller: Controller encoder: Encoder - async_estimator: AsyncEstimator + acim_estimator: AcimEstimator sensorless_estimator: SensorlessEstimator trap_traj: TrapezoidalTrajectory min_endstop: Endstop @@ -522,7 +522,7 @@ interfaces: can_heartbeat: TaskTimer controller_update: TaskTimer open_loop_controller_update: TaskTimer - async_estimator_update: TaskTimer + acim_estimator_update: TaskTimer motor_update: TaskTimer current_controller_update: TaskTimer dc_calib: TaskTimer @@ -811,7 +811,7 @@ interfaces: functions: get_val: {in: {index: uint32}, out: {val: float32}} - ODrive.AsyncEstimator: + ODrive.AcimEstimator: c_is_class: True attributes: rotor_flux: {type: readonly float32, unit: A, doc: estimated magnitude of the rotor flux} @@ -819,7 +819,7 @@ interfaces: type: readonly float32 unit: rad/s doc: estimated slip between physical and electrical angular velocity} - c_getter: slip_vel_.get_any().value_or(0.0f) + c_getter: slip_vel_.any().value_or(0.0f) phase_offset: type: readonly float32 unit: rad @@ -828,12 +828,12 @@ interfaces: type: readonly float32 unit: rad/s doc: calculated setpoint for the electrical velocity} - c_getter: stator_phase_vel_.get_any().value_or(0.0f) + c_getter: stator_phase_vel_.any().value_or(0.0f) stator_phase: type: readonly float32 unit: rad doc: calculated setpoint for the electrical phase} - c_getter: stator_phase_.get_any().value_or(0.0f) + c_getter: stator_phase_.any().value_or(0.0f) config: c_is_class: False attributes: @@ -992,13 +992,13 @@ interfaces: shadow_count: readonly int32 count_in_cpr: readonly int32 interpolation: readonly float32 - phase: {type: readonly float32, c_getter: phase_.get_any().value_or(0.0f)} - pos_estimate: {type: readonly float32, c_getter: pos_estimate_.get_any().value_or(0.0f)} + phase: {type: readonly float32, c_getter: phase_.any().value_or(0.0f)} + pos_estimate: {type: readonly float32, c_getter: pos_estimate_.any().value_or(0.0f)} pos_estimate_counts: readonly float32 pos_cpr_counts: readonly float32 - pos_circular: {type: readonly float32, c_getter: pos_circular_.get_any().value_or(0.0f)} + pos_circular: {type: readonly float32, c_getter: pos_circular_.any().value_or(0.0f)} hall_state: readonly uint8 - vel_estimate: {type: readonly float32, c_getter: vel_estimate_.get_any().value_or(0.0f)} + vel_estimate: {type: readonly float32, c_getter: vel_estimate_.any().value_or(0.0f)} vel_estimate_counts: readonly float32 calib_scan_response: readonly float32 pos_abs: int32 @@ -1040,10 +1040,10 @@ interfaces: flags: UnstableGain: UnknownCurrentMeasurement: - phase: {type: readonly float32, unit: rad, c_getter: phase_.get_any().value_or(0.0f)} + phase: {type: readonly float32, unit: rad, c_getter: phase_.any().value_or(0.0f)} pll_pos: {type: readonly float32, unit: rad} - phase_vel: {type: readonly float32, unit: rad/s, c_getter: phase_vel_.get_any().value_or(0.0f)} - vel_estimate: {type: readonly float32, unit: turns/s, c_getter: vel_estimate_.get_any().value_or(0.0f)} + phase_vel: {type: readonly float32, unit: rad/s, c_getter: phase_vel_.any().value_or(0.0f)} + vel_estimate: {type: readonly float32, unit: turns/s, c_getter: vel_estimate_.any().value_or(0.0f)} # pll_kp: float32 # pll_ki: float32 config: From 5a19799613e0ff6b9eefdfdd48332d2024fb16a2 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Mon, 16 Nov 2020 21:03:41 +0100 Subject: [PATCH 18/28] prevent motor arming if an error is set --- Firmware/MotorControl/axis.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 6c8e6663..9b12a3c4 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -481,10 +481,18 @@ void Axis::run_state_machine_loop() { bool status; switch (current_state_) { case AXIS_STATE_MOTOR_CALIBRATION: { + // These error checks are a hacky way to force legacy behavior + // when an error is raised. TODO: remove this when we overhaul + // the error architecture + // (https://github.com/madcowswe/ODrive/issues/526). + if (odrv.any_error()) + goto invalid_state_label; status = motor_.run_calibration(); } break; case AXIS_STATE_ENCODER_INDEX_SEARCH: { + if (odrv.any_error()) + goto invalid_state_label; if (!motor_.is_calibrated_) goto invalid_state_label; @@ -492,6 +500,8 @@ void Axis::run_state_machine_loop() { } break; case AXIS_STATE_ENCODER_DIR_FIND: { + if (odrv.any_error()) + goto invalid_state_label; if (!motor_.is_calibrated_) goto invalid_state_label; @@ -499,22 +509,30 @@ void Axis::run_state_machine_loop() { } break; case AXIS_STATE_HOMING: { + if (odrv.any_error()) + goto invalid_state_label; status = run_homing(); } break; case AXIS_STATE_ENCODER_OFFSET_CALIBRATION: { + if (odrv.any_error()) + goto invalid_state_label; if (!motor_.is_calibrated_) goto invalid_state_label; status = encoder_.run_offset_calibration(); } break; case AXIS_STATE_LOCKIN_SPIN: { + if (odrv.any_error()) + goto invalid_state_label; if (!motor_.is_calibrated_ || encoder_.config_.direction==0) goto invalid_state_label; status = run_lockin_spin(config_.general_lockin, false); } break; case AXIS_STATE_CLOSED_LOOP_CONTROL: { + if (odrv.any_error()) + goto invalid_state_label; if (!motor_.is_calibrated_ || (encoder_.config_.direction==0 && !config_.enable_sensorless_mode)) goto invalid_state_label; watchdog_feed(); From 138966aaee5d0606c3c5ce603863c993360e5539 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Tue, 17 Nov 2020 12:35:48 +0100 Subject: [PATCH 19/28] make odrivetool less hardware dependent --- tools/odrive/utils.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/odrive/utils.py b/tools/odrive/utils.py index ca64c758..f9313fa3 100755 --- a/tools/odrive/utils.py +++ b/tools/odrive/utils.py @@ -548,18 +548,18 @@ def dump_dma(odrv): def dump_timing(odrv, n_samples=100, path='/tmp/timings.png'): import matplotlib.pyplot as plt + import re timings = [] for attr in dir(odrv.task_times): if not attr.startswith('_'): timings.append((attr, getattr(odrv.task_times, attr), [], [])) # (name, obj, start_times, lengths) - for attr in dir(odrv.axis0.task_times): - if not attr.startswith('_'): - timings.append(('axis0.' + attr, getattr(odrv.axis0.task_times, attr), [], [])) # (name, obj, start_times, lengths) - for attr in dir(odrv.axis1.task_times): - if not attr.startswith('_'): - timings.append(('axis1.' + attr, getattr(odrv.axis1.task_times, attr), [], [])) # (name, obj, start_times, lengths) + for k in dir(odrv): + if re.match(r'axis[0-9]+', k): + for attr in dir(getattr(odrv, k).task_times): + if not attr.startswith('_'): + timings.append((k + '.' + attr, getattr(getattr(odrv, k).task_times, attr), [], [])) # (name, obj, start_times, lengths) # Take a couple of samples print("sampling...") From 1184c7778793288800112617bdeb401bab8f81e5 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Tue, 17 Nov 2020 18:12:55 +0100 Subject: [PATCH 20/28] fix Interrupt and DMA priority for UART B --- Firmware/Board/v3/Src/dma.c | 6 +++--- Firmware/Board/v3/Src/usart.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Firmware/Board/v3/Src/dma.c b/Firmware/Board/v3/Src/dma.c index 1a3d8f22..56d09af9 100644 --- a/Firmware/Board/v3/Src/dma.c +++ b/Firmware/Board/v3/Src/dma.c @@ -82,14 +82,14 @@ void MX_DMA_Init(void) HAL_NVIC_SetPriority(DMA1_Stream4_IRQn, 10, 0); HAL_NVIC_EnableIRQ(DMA1_Stream4_IRQn); /* DMA1_Stream5_IRQn interrupt configuration */ - HAL_NVIC_SetPriority(DMA1_Stream5_IRQn, 10, 0); // SPI TX - must have higher priority than SPI RX - // and higher priority than the control loop handler + HAL_NVIC_SetPriority(DMA1_Stream5_IRQn, 10, 0); HAL_NVIC_EnableIRQ(DMA1_Stream5_IRQn); /* DMA1_Stream6_IRQn interrupt configuration */ HAL_NVIC_SetPriority(DMA1_Stream6_IRQn, 10, 0); HAL_NVIC_EnableIRQ(DMA1_Stream6_IRQn); /* DMA1_Stream7_IRQn interrupt configuration */ - HAL_NVIC_SetPriority(DMA1_Stream7_IRQn, 3, 0); + HAL_NVIC_SetPriority(DMA1_Stream7_IRQn, 3, 0); // SPI TX - must have higher priority than SPI RX + // and higher priority than the control loop handler HAL_NVIC_EnableIRQ(DMA1_Stream7_IRQn); /* DMA2_Stream0_IRQn interrupt configuration */ // Dear STM, no we _don't_ want to fire an interrupt for this DMA diff --git a/Firmware/Board/v3/Src/usart.c b/Firmware/Board/v3/Src/usart.c index e0f1912b..cddfd4e0 100644 --- a/Firmware/Board/v3/Src/usart.c +++ b/Firmware/Board/v3/Src/usart.c @@ -203,7 +203,7 @@ void HAL_UART_MspInit(UART_HandleTypeDef* uartHandle) __HAL_LINKDMA(uartHandle,hdmatx,hdma_usart2_tx); /* USART2 interrupt Init */ - HAL_NVIC_SetPriority(USART2_IRQn, 5, 0); + HAL_NVIC_SetPriority(USART2_IRQn, 10, 0); HAL_NVIC_EnableIRQ(USART2_IRQn); /* USER CODE BEGIN USART2_MspInit 1 */ @@ -240,7 +240,7 @@ void HAL_UART_MspDeInit(UART_HandleTypeDef* uartHandle) /* Peripheral clock disable */ __HAL_RCC_USART2_CLK_DISABLE(); - /* UART4 DMA DeInit */ + /* USART2 DMA DeInit */ HAL_DMA_DeInit(uartHandle->hdmarx); HAL_DMA_DeInit(uartHandle->hdmatx); From 5d1e59f23a47f9b687bd32bab4e4236e38ca719e Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Tue, 17 Nov 2020 21:48:48 +0100 Subject: [PATCH 21/28] disable motor PWMs on hard fault --- Firmware/Board/v3/Src/stm32f4xx_it.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Firmware/Board/v3/Src/stm32f4xx_it.c b/Firmware/Board/v3/Src/stm32f4xx_it.c index 175a4380..59ab950b 100644 --- a/Firmware/Board/v3/Src/stm32f4xx_it.c +++ b/Firmware/Board/v3/Src/stm32f4xx_it.c @@ -80,6 +80,9 @@ void NMI_Handler(void) __attribute__((used)) void get_regs(void** stack_ptr) { + TIM1->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M0 PWM + TIM8->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M1 PWM + void* volatile r0 __attribute__((unused)) = stack_ptr[0]; void* volatile r1 __attribute__((unused)) = stack_ptr[1]; void* volatile r2 __attribute__((unused)) = stack_ptr[2]; @@ -127,6 +130,8 @@ void MemManage_Handler(void) while (1) { /* USER CODE BEGIN W1_MemoryManagement_IRQn 0 */ + TIM1->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M0 PWM + TIM8->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M1 PWM /* USER CODE END W1_MemoryManagement_IRQn 0 */ } /* USER CODE BEGIN MemoryManagement_IRQn 1 */ @@ -145,6 +150,8 @@ void BusFault_Handler(void) while (1) { /* USER CODE BEGIN W1_BusFault_IRQn 0 */ + TIM1->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M0 PWM + TIM8->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M1 PWM /* USER CODE END W1_BusFault_IRQn 0 */ } /* USER CODE BEGIN BusFault_IRQn 1 */ @@ -163,6 +170,8 @@ void UsageFault_Handler(void) while (1) { /* USER CODE BEGIN W1_UsageFault_IRQn 0 */ + TIM1->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M0 PWM + TIM8->BDTR &= ~(TIM_BDTR_AOE_Msk | TIM_BDTR_MOE_Msk); // disable M1 PWM /* USER CODE END W1_UsageFault_IRQn 0 */ } /* USER CODE BEGIN UsageFault_IRQn 1 */ From 2cfd63ae1f3db99c3b8d94a987723597682c4948 Mon Sep 17 00:00:00 2001 From: PAJohnson Date: Tue, 17 Nov 2020 22:03:47 -0500 Subject: [PATCH 22/28] Added test rig configuration for Patrick --- tools/test-rig-pj.yaml | 96 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tools/test-rig-pj.yaml diff --git a/tools/test-rig-pj.yaml b/tools/test-rig-pj.yaml new file mode 100644 index 00000000..640dda40 --- /dev/null +++ b/tools/test-rig-pj.yaml @@ -0,0 +1,96 @@ + +components: + - type: generalpurpose + name: homenet + net: homenet + + - type: generalpurpose + name: rpi + ssh: odrv + net: homenet + components: + - type: uart + name: uart0 + port: /dev/ttyS0 + connected-to: main_uart + - type: can + name: can0 + interface: can0 + connected-to: odrive.can + # need to specify GPIOs explicitly for the generalpurpose type + - {type: gpio, num: 16} + - {type: gpio, num: 19} + - {type: gpio, num: 20} + - {type: gpio, num: 26} + +# - type: programmer +# name: The Blue STLink/v2 +# id: '\x53\x3f\x75\x06\x49\x3f\x49\x51\x44\x54\x19\x3f' + + - type: odrive + name: odrive + board-version: v3.6-58V + serial-number: "2061398A4D4D" + brake-resistance: 0.47 + usb: auto + can: main_canbus + vbus-voltage: 24 # [V] + max-brake-power: 150 # [W] + encoder0: virtual_encoder0 + encoder1: virtual_encoder1 + motor0: D5065-270KV_0 + motor1: floating + + - type: motor + name: D5065-270KV_0 + phase-resistance: 0.039 + phase-inductance: 1.57e-05 + pole-pairs: 7 + direction: 1 + kv: 270 + max-current: 70 + max-voltage: 40 + + - type: encoder + name: real_encoder + cpr: 8192 + max-rpm: 7000 + + - type: teensy + name: teensy + + - {type: lpf, name: lpf0} + - {type: lpf, name: lpf1} + +connections: + - ['odrive.can', 'rpi.can0'] + - ['teensy.program', 'rpi.gpio26'] + - ['teensy.gpio12', 'rpi.uart0.tx'] + - ['teensy.gpio13', 'rpi.uart0.rx'] + - ['teensy.gpio11', 'odrive.gpio1'] + - ['teensy.gpio10', 'odrive.gpio2'] + - ['teensy.gpio9', 'odrive.gpio3'] + - ['teensy.gpio8', 'odrive.gpio4'] + - ['teensy.gpio14', 'odrive.gpio5'] + - ['teensy.gpio15', 'odrive.gpio6'] + - ['teensy.gpio16', 'odrive.gpio7'] + - ['teensy.gpio17', 'odrive.gpio8'] + - ['teensy.gpio6', 'rpi.gpio20'] + - ['teensy.gpio7', 'rpi.gpio19'] + - ['teensy.gpio23', 'odrive.encoder0.z'] + - ['teensy.gpio22', 'odrive.encoder0.b'] + - ['teensy.gpio21', 'odrive.encoder0.a'] + - ['teensy.gpio20', 'odrive.encoder1.z'] + - ['teensy.gpio19', 'odrive.encoder1.b'] + - ['teensy.gpio18', 'odrive.encoder1.a'] + - ['teensy.gpio0', 'real_encoder.z'] + - ['teensy.gpio1', 'real_encoder.a'] + - ['teensy.gpio2', 'real_encoder.b'] + - ['teensy.gpio3', 'odrive.spi.mosi'] + - ['teensy.gpio4', 'odrive.spi.miso'] + - ['teensy.gpio5', 'odrive.spi.sck'] + - ['odrive.axis0', 'D5065-270KV_0'] + - ['D5065-270KV_0', 'real_encoder'] + - ['odrive.gpio3', 'lpf0'] + - ['odrive.gpio4', 'lpf1'] + - ['lpf0.en', 'lpf1.en', 'rpi.gpio16'] From 92926c4d214cbefee2369ee7a4ae63e7ea62ffc2 Mon Sep 17 00:00:00 2001 From: PAJohnson Date: Wed, 18 Nov 2020 01:00:36 -0500 Subject: [PATCH 23/28] [tests] added high resistance brake test to TestRegenProtection --- tools/odrive/tests/closed_loop_test.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tools/odrive/tests/closed_loop_test.py b/tools/odrive/tests/closed_loop_test.py index 9b64d1bf..241ca442 100644 --- a/tools/odrive/tests/closed_loop_test.py +++ b/tools/odrive/tests/closed_loop_test.py @@ -210,6 +210,8 @@ class TestRegenProtection(TestClosedLoopControlBase): time.sleep(1.0) test_assert_no_error(axis_ctx) + + logger.debug(f'Brake control test with brake resistor disabled') # once more, but this time without brake resistor axis_ctx.parent.handle.config.enable_brake_resistor = False # accelerate... @@ -225,6 +227,29 @@ class TestRegenProtection(TestClosedLoopControlBase): test_assert_eq(axis_ctx.handle.motor.error & MOTOR_ERROR_SYSTEM_LEVEL, MOTOR_ERROR_SYSTEM_LEVEL) test_assert_eq(axis_ctx.handle.error, 0) + # Do test again with wrong brake resistance setting + logger.debug(f'Brake control test with brake resistor = 100') + axis_ctx.parent.handle.clear_errors() + time.sleep(1.0) + axis_ctx.parent.handle.config.brake_resistance = 100 + axis_ctx.parent.handle.config.dc_max_negative_current = -0.5 + request_state(axis_ctx, AXIS_STATE_CLOSED_LOOP_CONTROL) + + # accelerate... + axis_ctx.handle.controller.input_vel = nominal_rps + time.sleep(1.0) + test_assert_no_error(axis_ctx) + + # ... and brake + axis_ctx.handle.controller.input_vel = 0 + time.sleep(1.0) # expect DC_BUS_OVER_REGEN_CURRENT + time.sleep(0.1) + test_assert_eq(axis_ctx.parent.handle.error, ODRIVE_ERROR_DC_BUS_OVER_REGEN_CURRENT) + test_assert_eq(axis_ctx.handle.motor.error & MOTOR_ERROR_SYSTEM_LEVEL, MOTOR_ERROR_SYSTEM_LEVEL) + test_assert_eq(axis_ctx.handle.error, 0) + + + class TestVelLimitInTorqueControl(TestClosedLoopControlBase): """ From f66d83d2f6b8d4428792daf5520ed506b1d67a70 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Thu, 15 Oct 2020 19:44:45 +0200 Subject: [PATCH 24/28] add "implements" functionality to intf generator --- Firmware/Tupfile.lua | 14 +- Firmware/fibre/cpp/interfaces_template.j2 | 2 +- Firmware/fibre/cpp/type_info_template.j2 | 4 +- Firmware/fibre/tools/interface_generator.py | 289 ++++++++++------- Firmware/interface_generator_stub.py | 3 +- Firmware/odrive-interface.yaml | 343 ++++++++++---------- 6 files changed, 355 insertions(+), 300 deletions(-) diff --git a/Firmware/Tupfile.lua b/Firmware/Tupfile.lua index 90eecd2c..30c8ee2c 100644 --- a/Firmware/Tupfile.lua +++ b/Firmware/Tupfile.lua @@ -16,15 +16,10 @@ end python_command = find_python3() print('Using python command "'..python_command..'"') -tup.frule{inputs={'fibre/cpp/interfaces_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/interfaces.hpp'} -tup.frule{inputs={'fibre/cpp/function_stubs_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/function_stubs.hpp'} -tup.frule{inputs={'fibre/cpp/endpoints_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --generate-endpoints ODrive --template %f --output %o', outputs='autogen/endpoints.hpp'} -tup.frule{inputs={'fibre/cpp/type_info_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/type_info.hpp'} - +-- TODO: use CI to verify that on PRs the enums.py file is consistent with the YAML. -- Note: we currently check this file into source control for two reasons: -- - Don't require tup to run in order to use odrivetool from the repo -- - On Windows, tup is unhappy with writing outside of the tup directory --- TODO: use CI to verify that on PRs the enums.py file is consistent with the YAML. --tup.frule{command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template enums_template.j2 --output ../tools/odrive/enums.py'} tup.frule{ @@ -34,6 +29,7 @@ tup.frule{ board_v3 = { dir = 'Board/v3', + root_interface = 'ODrive3', sources = {'Drivers/DRV8301/drv8301.cpp', 'Board/v3/board.cpp'}, flags = {'-DSTM32F405xx', '-DARM_MATH_CM4', '-mcpu=cortex-m4', '-mfpu=fpv4-sp-d16', '-DFPU_FPV4'}, ldflags = {'-TBoard/v3/STM32F405RGTx_FLASH.ld', '-LBoard/v3/Drivers/CMSIS/Lib', '-larm_cortexM4lf_math', '-mcpu=cortex-m4', '-mfpu=fpv4-sp-d16'} @@ -177,6 +173,12 @@ for src in string.gmatch(vars['C_INCLUDES'] or '', "%S+") do stm_includes += board.dir..'/'..string.sub(src, 3, -1) -- remove "-I" from each include path end +-- Autogen files from YAML interface definitions +tup.frule{inputs={'fibre/cpp/interfaces_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/interfaces.hpp'} +tup.frule{inputs={'fibre/cpp/function_stubs_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/function_stubs.hpp'} +tup.frule{inputs={'fibre/cpp/endpoints_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --generate-endpoints '..board.root_interface..' --template %f --output %o', outputs='autogen/endpoints.hpp'} +tup.frule{inputs={'fibre/cpp/type_info_template.j2'}, command=python_command..' interface_generator_stub.py --definitions odrive-interface.yaml --template %f --output %o', outputs='autogen/type_info.hpp'} + -- TODO: cleaner separation of the platform code and the rest stm_includes += '.' --stm_includes += 'Drivers/DRV8301' diff --git a/Firmware/fibre/cpp/interfaces_template.j2 b/Firmware/fibre/cpp/interfaces_template.j2 index d901327e..91af9494 100644 --- a/Firmware/fibre/cpp/interfaces_template.j2 +++ b/Firmware/fibre/cpp/interfaces_template.j2 @@ -30,7 +30,7 @@ void [%- endmacro %] [%- macro render_interface(intf) %] -class [[intf.name | to_pascal_case]]Intf { +class [[intf.name | to_pascal_case]]Intf[% if intf.implements %] :[%- for base_intf in intf.implements %] public [[base_intf.c_name]][% endfor %][% endif %] { public: [%- for intf in intf.interfaces -%] [[render_interface(intf) | indent(4)]] diff --git a/Firmware/fibre/cpp/type_info_template.j2 b/Firmware/fibre/cpp/type_info_template.j2 index 7e6cda57..7efff78f 100644 --- a/Firmware/fibre/cpp/type_info_template.j2 +++ b/Firmware/fibre/cpp/type_info_template.j2 @@ -26,7 +26,7 @@ struct [[intf.fullname | to_pascal_case]]TypeInfo : TypeInfo { T* ptr = *(T**)&obj; introspectable_storage_t res; switch (idx) { -[%- for property in intf.attributes.values() %] +[%- for property in intf.get_all_attributes().values() %] case [[loop.index0]]: *(decltype([[intf.c_name]]::get_[[property.name]](std::declval()))*)(&res) = [[intf.c_name]]::get_[[property.name]](ptr); break; [%- endfor %] } @@ -38,7 +38,7 @@ struct [[intf.fullname | to_pascal_case]]TypeInfo : TypeInfo { [% for intf in interfaces.values() %][% if not intf.builtin %] template const PropertyInfo [[intf.fullname | to_pascal_case]]TypeInfo::property_table[] = { -[%- for property in intf.attributes.values() %] +[%- for property in intf.get_all_attributes().values() %] {"[[property.name]]", &[[(property.type.purename or property.type.fullname) | to_pascal_case]]TypeInfo()))>>::singleton}, [%- endfor %] }; diff --git a/Firmware/fibre/tools/interface_generator.py b/Firmware/fibre/tools/interface_generator.py index f4774b2a..ebdd72ef 100644 --- a/Firmware/fibre/tools/interface_generator.py +++ b/Firmware/fibre/tools/interface_generator.py @@ -19,6 +19,10 @@ definitions: c_name: {type: string} brief: {type: string} doc: {type: string} + implements: + anyOf: + - {"$ref": "#/definitions/intf_type_ref"} + - {type: array, items: {"$ref": "#/definitions/intf_type_ref"}} functions: type: object additionalProperties: {"$ref": "#/definitions/function"} @@ -42,6 +46,11 @@ definitions: __column__: {type: object} additionalProperties: false + intf_type_ref: + anyOf: + - {"type": "string"} + - {"$ref": "#/definitions/interface"} + intf_or_val_type: anyOf: - {"$ref": "#/definitions/interface"} @@ -166,53 +175,8 @@ enums = OrderedDict() interfaces = OrderedDict() userdata = OrderedDict() # Arbitrary data passed from the definition file to the template -def make_property_type(typeargs): - value_type = resolve_valuetype('', typeargs['fibre.Property.type']) - mode = typeargs.get('fibre.Property.mode', 'readwrite') - name = 'Property<' + value_type['fullname'] + ', ' + mode + '>' - fullname = join_name('fibre', name) - if fullname in interfaces: - return interfaces[fullname] - - c_name = 'Property<' + ('const ' if mode == 'readonly' else '') + value_type['c_name'] + '>' - prop_type = { - 'name': name, - 'fullname': fullname, - 'purename': 'fibre.Property', - 'c_name': c_name, - 'value_type': value_type, # TODO: should be a metaarg - 'mode': mode, # TODO: should be a metaarg - 'builtin': True, - 'attributes': OrderedDict(), - 'functions': OrderedDict() - } - if mode != 'readonly': - prop_type['functions']['exchange'] = { - 'name': 'exchange', - 'fullname': join_name(fullname, 'exchange'), - 'in': OrderedDict([('obj', {'name': 'obj', 'type': {'c_name': c_name}}), ('value', {'name': 'value', 'type': value_type, 'optional': True})]), - 'out': OrderedDict([('value', {'name': 'value', 'type': value_type})]), - #'implementation': 'fibre_property_exchange<' + value_type['c_name'] + '>' - } - else: - prop_type['functions']['read'] = { - 'name': 'read', - 'fullname': join_name(fullname, 'read'), - 'in': OrderedDict([('obj', {'name': 'obj', 'type': {'c_name': c_name}})]), - 'out': OrderedDict([('value', {'name': 'value', 'type': value_type})]), - #'implementation': 'fibre_property_read<' + value_type['c_name'] + '>' - } - - interfaces[fullname] = prop_type - return prop_type - -generics = { - 'fibre.Property': make_property_type # TODO: improve generic support -} - - def make_ref_type(interface): - name = 'Ref<' + interface['fullname'] + '>' + name = 'Ref<' + interface.fullname + '>' fullname = join_name('fibre', name) if fullname in interfaces: return interfaces[fullname] @@ -221,7 +185,7 @@ def make_ref_type(interface): 'builtin': True, 'name': name, 'fullname': fullname, - 'c_name': interface['fullname'].replace('.', 'Intf::') + 'Intf*' + 'c_name': interface.fullname.replace('.', 'Intf::') + 'Intf*' } value_types[fullname] = ref_type @@ -260,13 +224,14 @@ def regularize_attribute(parent, name, elem, c_is_class): elem['type'] = {} if 'attributes' in elem: elem['type']['attributes'] = elem.pop('attributes') if 'functions' in elem: elem['type']['functions'] = elem.pop('functions') + if 'implements' in elem: elem['type']['implements'] = elem.pop('implements') if 'c_is_class' in elem: elem['type']['c_is_class'] = elem.pop('c_is_class') if 'values' in elem: elem['type']['values'] = elem.pop('values') if 'flags' in elem: elem['type']['flags'] = elem.pop('flags') if 'nullflag' in elem: elem['type']['nullflag'] = elem.pop('nullflag') elem['name'] = name - elem['fullname'] = join_name(parent['fullname'], name) + elem['fullname'] = join_name(parent.fullname, name) elem['parent'] = parent elem['typeargs'] = elem.get('typeargs', {}) elem['c_name'] = elem.get('c_name', None) or (elem['name'] + ('_' if c_is_class else '')) @@ -277,40 +242,145 @@ def regularize_attribute(parent, name, elem, c_is_class): if isinstance(elem['type'], str) and elem['type'].startswith('readonly '): elem['typeargs']['fibre.Property.mode'] = 'readonly' elem['typeargs']['fibre.Property.type'] = elem['type'][len('readonly '):] - elem['type'] = 'fibre.Property' + elem['type'] = InterfaceRefElement(parent.fullname, None, 'fibre.Property', elem['typeargs']) if elem['typeargs']['fibre.Property.mode'] == 'readonly' and 'c_setter' in elem: elem.pop('c_setter') elif ('flags' in elem['type']) or ('values' in elem['type']): elem['typeargs']['fibre.Property.mode'] = elem['typeargs'].get('fibre.Property.mode', None) or 'readwrite' - elem['typeargs']['fibre.Property.type'] = regularize_valuetype(parent['fullname'], to_pascal_case(name), elem['type']) - elem['type'] = 'fibre.Property' + elem['typeargs']['fibre.Property.type'] = regularize_valuetype(parent.fullname, to_pascal_case(name), elem['type']) + elem['type'] = InterfaceRefElement(parent.fullname, None, 'fibre.Property', elem['typeargs']) if elem['typeargs']['fibre.Property.mode'] == 'readonly' and 'c_setter' in elem: elem.pop('c_setter') else: - elem['type'] = regularize_interface(parent['fullname'], to_pascal_case(name), elem['type']) + elem['type'] = InterfaceRefElement(parent.fullname, to_pascal_case(name), elem['type'], elem['typeargs']) return elem +class InterfaceRefElement(): + def __init__(self, scope, name, elem, typeargs): + if isinstance(elem, str): + self._intf = None + self._scope = scope + self._name = elem + else: + self._intf = InterfaceElement(scope, name, elem) + self._scope = None + self._name = None + self._typeargs = typeargs -def regularize_interface(path, name, elem): - if elem is None: - elem = {} - if isinstance(elem, str): - return elem # will be resolved during type resolution - #if path is None: - # max_anonymous_type = max([int((re.findall('^' + join_name(path, 'AnonymousType') + '([1-9]+)$', x) + ['0'])[0]) for x in interfaces.keys()]) - # path = 'AnonymousType' + str(max_anonymous_type + 1) - elem['name'] = split_name(name)[-1] - elem['fullname'] = path = join_name(path, name) - elem['c_name'] = elem.get('c_name', elem['fullname'].replace('.', 'Intf::')) + 'Intf' - interfaces[path] = elem - elem['functions'] = OrderedDict((name, regularize_func(path, name, func, {'obj': {'type': make_ref_type(elem)}})) - for name, func in get_dict(elem, 'functions').items()) - if not 'c_is_class' in elem: - raise Exception(elem) - treat_as_class = elem['c_is_class'] # TODO: add command line arg to make this selectively optional - elem['attributes'] = OrderedDict((name, regularize_attribute(elem, name, prop, treat_as_class)) - for name, prop in get_dict(elem, 'attributes').items()) - elem['interfaces'] = [] - elem['enums'] = [] - return elem + def resolve(self): + """ + Resolves this interface reference to an actual InterfaceElement instance. + The innermost scope is searched first. + At every scope level, if no matching interface is found, it is checked if a + matching value type exists. If so, the interface type fibre.Property + is returned. + """ + if not self._intf is None: + return self._intf + + typeargs = self._typeargs + if 'fibre.Property.type' in typeargs: + typeargs['fibre.Property.type'] = resolve_valuetype(self._scope, typeargs['fibre.Property.type']) + + scope = self._scope.split('.') + for probe_scope in [join_name(*scope[:(len(scope)-i)]) for i in range(len(scope)+1)]: + probe_name = join_name(probe_scope, self._name) + #print('probing ' + probe_name) + if probe_name in interfaces: + return interfaces[probe_name] + elif probe_name in value_types: + typeargs['fibre.Property.type'] = value_types[probe_name] + return make_property_type(typeargs) + elif probe_name in generics: + return generics[probe_name](typeargs) + + raise Exception('could not resolve type {} in {}. Known interfaces are: {}. Known value types are: {}'.format(self._name, self._scope, list(interfaces.keys()), list(value_types.keys()))) + +class InterfaceElement(): + def __init__(self, path, name, elem): + if elem is None: + elem = {} + assert(isinstance(elem, dict)) + + path = join_name(path, name) + interfaces[path] = self + + self.name = split_name(name)[-1] + self.fullname = path + self.c_name = elem.get('c_name', self.fullname.replace('.', 'Intf::')) + 'Intf' + + if not 'implements' in elem: + elem['implements'] = [] + elif isinstance(elem['implements'], str): + elem['implements'] = [elem['implements']] + self.implements = [InterfaceRefElement(path, None, elem, {}) for elem in elem['implements']] + self.functions = OrderedDict((name, regularize_func(path, name, func, {'obj': {'type': make_ref_type(self)}})) + for name, func in get_dict(elem, 'functions').items()) + if not 'c_is_class' in elem: + raise Exception(elem) + treat_as_class = elem['c_is_class'] # TODO: add command line arg to make this selectively optional + self.attributes = OrderedDict((name, regularize_attribute(self, name, prop, treat_as_class)) + for name, prop in get_dict(elem, 'attributes').items()) + self.interfaces = [] + self.enums = [] + + def get_all_attributes(self, stack=[]): + result = OrderedDict() + for intf in self.implements: + assert(not self in stack) + result.update(intf.get_all_attributes(stack + [self])) + result.update(self.attributes) + return result + + def get_all_functions(self, stack=[]): + result = OrderedDict() + for intf in self.implements: + assert(not self in stack) + result.update(intf.get_all_functions(stack + [self])) + result.update(self.functions) + return result + +class PropertyInterfaceElement(InterfaceElement): + def __init__(self, name, fullname, mode, value_type): + self.name = name + self.fullname = fullname + self.purename = 'fibre.Property' + self.c_name = 'Property<' + ('const ' if mode == 'readonly' else '') + value_type['c_name'] + '>' + self.value_type = value_type # TODO: should be a metaarg + self.mode = mode # TODO: should be a metaarg + self.builtin = True + self.attributes = OrderedDict() + self.functions = OrderedDict() + if mode != 'readonly': + self.functions['exchange'] = { + 'name': 'exchange', + 'fullname': join_name(fullname, 'exchange'), + 'in': OrderedDict([('obj', {'name': 'obj', 'type': {'c_name': self.c_name}}), ('value', {'name': 'value', 'type': value_type, 'optional': True})]), + 'out': OrderedDict([('value', {'name': 'value', 'type': value_type})]), + #'implementation': 'fibre_property_exchange<' + value_type['c_name'] + '>' + } + else: + self.functions['read'] = { + 'name': 'read', + 'fullname': join_name(fullname, 'read'), + 'in': OrderedDict([('obj', {'name': 'obj', 'type': {'c_name': self.c_name}})]), + 'out': OrderedDict([('value', {'name': 'value', 'type': value_type})]), + #'implementation': 'fibre_property_read<' + value_type['c_name'] + '>' + } + + interfaces[fullname] = self # TODO: not good to write to a global here + +def make_property_type(typeargs): + value_type = resolve_valuetype('', typeargs['fibre.Property.type']) + mode = typeargs.get('fibre.Property.mode', 'readwrite') + name = 'Property<' + value_type['fullname'] + ', ' + mode + '>' + fullname = join_name('fibre', name) + if fullname in interfaces: + return interfaces[fullname] + else: + return PropertyInterfaceElement(name, fullname, mode, value_type) + +generics = { + 'fibre.Property': make_property_type # TODO: improve generic support +} def regularize_valuetype(path, name, elem): if elem is None: @@ -351,34 +421,6 @@ def regularize_valuetype(path, name, elem): return elem -def resolve_interface(scope, name, typeargs): - """ - Resolves a type name (i.e. interface name or value type name) given as a - string to an interface object. The innermost scope is searched first. - At every scope level, if no matching interface is found, it is checked if a - matching value type exists. If so, the interface type fibre.Property - is returned. - """ - if not isinstance(name, str): - return name - - if 'fibre.Property.type' in typeargs: - typeargs['fibre.Property.type'] = resolve_valuetype(scope, typeargs['fibre.Property.type']) - - scope = scope.split('.') - for probe_scope in [join_name(*scope[:(len(scope)-i)]) for i in range(len(scope)+1)]: - probe_name = join_name(probe_scope, name) - #print('probing ' + probe_name) - if probe_name in interfaces: - return interfaces[probe_name] - elif probe_name in value_types: - typeargs['fibre.Property.type'] = value_types[probe_name] - return make_property_type(typeargs) - elif probe_name in generics: - return generics[probe_name](typeargs) - - raise Exception('could not resolve type {} in {}. Known interfaces are: {}. Known value types are: {}'.format(name, join_name(*scope), list(interfaces.keys()), list(value_types.keys()))) - def resolve_valuetype(scope, name): """ Resolves a type name given as a string to the type object. @@ -404,19 +446,19 @@ def map_to_fibre01_type(t): return t['fullname'] def generate_endpoint_for_property(prop, attr_bindto, idx): - prop_intf = interfaces[prop['type']['fullname']] + prop_intf = interfaces[prop['type'].fullname] endpoint = { 'id': idx, - 'function': prop_intf['functions']['read' if prop['type']['mode'] == 'readonly' else 'exchange'], + 'function': prop_intf.functions['read' if prop['type'].mode == 'readonly' else 'exchange'], 'in_bindings': OrderedDict([('obj', attr_bindto)]), 'out_bindings': OrderedDict() } endpoint_definition = { 'name': prop['name'], 'id': idx, - 'type': map_to_fibre01_type(prop['type']['value_type']), - 'access': 'r' if prop['type']['mode'] == 'readonly' else 'rw', + 'type': map_to_fibre01_type(prop['type'].value_type), + 'access': 'r' if prop['type'].mode == 'readonly' else 'rw', } return endpoint, endpoint_definition @@ -430,10 +472,10 @@ def generate_endpoint_table(intf, bindto, idx): endpoint_definitions = [] cnt = 0 - for k, prop in intf['attributes'].items(): - property_value_type = re.findall('^fibre\.Property<([^>]*), (readonly|readwrite)>$', prop['type']['fullname']) + for k, prop in intf.get_all_attributes().items(): + property_value_type = re.findall('^fibre\.Property<([^>]*), (readonly|readwrite)>$', prop['type'].fullname) #attr_bindto = join_name(bindto, bindings_map.get(join_name(intf['fullname'], k), k + ('_' if len(intf['functions']) or (intf['fullname'] in treat_as_classes) else ''))) - attr_bindto = intf['c_name'] + '::get_' + prop['name'] + '(' + bindto + ')' + attr_bindto = intf.c_name + '::get_' + prop['name'] + '(' + bindto + ')' if len(property_value_type): # Special handling for Property<...> attributes: they resolve to one single endpoint endpoint, endpoint_definition = generate_endpoint_for_property(prop, attr_bindto, idx + cnt) @@ -450,7 +492,7 @@ def generate_endpoint_table(intf, bindto, idx): }) cnt += inner_cnt - for k, func in intf['functions'].items(): + for k, func in intf.get_all_functions().items(): endpoints.append({ 'id': idx + cnt, 'function': func, @@ -463,14 +505,14 @@ def generate_endpoint_table(intf, bindto, idx): endpoint, endpoint_definition = generate_endpoint_for_property({ 'name': arg['name'], 'type': make_property_type({'fibre.Property.type': arg['type'], 'fibre.Property.mode': 'readwrite'}) - }, intf['c_name'] + '::get_' + func['name'] + '_in_' + k_arg + '_' + '(' + bindto + ')', idx + cnt + 1 + i) + }, intf.c_name + '::get_' + func['name'] + '_in_' + k_arg + '_' + '(' + bindto + ')', idx + cnt + 1 + i) endpoints.append(endpoint) in_def.append(endpoint_definition) for i, (k_arg, arg) in enumerate(func['out'].items()): endpoint, endpoint_definition = generate_endpoint_for_property({ 'name': arg['name'], 'type': make_property_type({'fibre.Property.type': arg['type'], 'fibre.Property.mode': 'readonly'}) - }, intf['c_name'] + '::get_' + func['name'] + '_out_' + k_arg + '_' + '(' + bindto + ')', idx + cnt + len(func['in']) + i) + }, intf.c_name + '::get_' + func['name'] + '_out_' + k_arg + '_' + '(' + bindto + ')', idx + cnt + len(func['in']) + i) endpoints.append(endpoint) out_def.append(endpoint_definition) @@ -541,7 +583,7 @@ for definition_file in definition_files: # Regularize everything into a wellknown form for k, item in list(interfaces.items()): - regularize_interface('', k, item) + InterfaceElement('', k, item) for k, item in list(value_types.items()): regularize_valuetype('', k, item) @@ -554,15 +596,16 @@ if len(clashing_names): print("**Error**: Found both an interface and a value type with the name {}. This is not allowed, interfaces and value types (such as enums) share the same namespace.".format(clashing_names[0]), file=sys.stderr) sys.exit(1) -# Resolve all types into references +# Resolve all types to references for _, item in list(interfaces.items()): - for _, prop in item['attributes'].items(): - prop['type'] = resolve_interface(item['fullname'], prop['type'], prop['typeargs']) - for _, func in item['functions'].items(): + item.implements = [ref.resolve() for ref in item.implements] + for _, prop in item.attributes.items(): + prop['type'] = prop['type'].resolve() + for _, func in item.functions.items(): for _, arg in func['in'].items(): - arg['type'] = resolve_valuetype(item['fullname'], arg['type']) + arg['type'] = resolve_valuetype(item.fullname, arg['type']) for _, arg in func['out'].items(): - arg['type'] = resolve_valuetype(item['fullname'], arg['type']) + arg['type'] = resolve_valuetype(item.fullname, arg['type']) # Attach interfaces to their parents toplevel_interfaces = [] @@ -573,8 +616,8 @@ for k, item in list(interfaces.items()): else: if k[:-1] != ['fibre']: # TODO: remove special handling parent = interfaces[join_name(*k[:-1])] - parent['interfaces'].append(item) - item['parent'] = parent + parent.interfaces.append(item) + item.parent = parent toplevel_enums = [] for k, item in list(enums.items()): k = split_name(k) @@ -583,7 +626,7 @@ for k, item in list(enums.items()): else: if k[:-1] != ['fibre']: # TODO: remove special handling parent = interfaces[join_name(*k[:-1])] - parent['enums'].append(item) + parent.enums.append(item) item['parent'] = parent @@ -643,7 +686,7 @@ def tokenize(text, interface, interface_transform, value_type_transform, attribu if not attr is None: return attribute_transform(token, attr) - print('Warning: cannot resolve "{}" in {}'.format(token, interface['fullname'])) + print('Warning: cannot resolve "{}" in {}'.format(token, interface.fullname)) return "`" + token + "`" return re.sub(r'`([A-Za-z\._]+)`', token_transform, text) diff --git a/Firmware/interface_generator_stub.py b/Firmware/interface_generator_stub.py index d5b22093..b55a89b8 100644 --- a/Firmware/interface_generator_stub.py +++ b/Firmware/interface_generator_stub.py @@ -4,7 +4,8 @@ import sys import os try: - exec(open(os.path.join(os.path.dirname(os.path.realpath(__file__)), 'fibre', 'tools', 'interface_generator.py')).read()) + path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'fibre', 'tools', 'interface_generator.py') + exec(compile(open(path).read(), path, 'exec')) except ImportError as ex: print(str(ex), file=sys.stderr) print("Note that there are new compile-time dependencies since around v0.5.1.", file=sys.stderr) diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index 83f294b4..d88af102 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -156,171 +156,6 @@ interfaces: addr_match_cnt: readonly uint32 rx_cnt: readonly uint32 error_cnt: readonly uint32 - - config: - c_is_class: False - attributes: - # TODO: add support for arrays - gpio1_mode: {type: GpioMode, doc: Mode of GPIO1 (changes take effect after reboot), c_name: 'gpio_modes[1]'} - gpio2_mode: {type: GpioMode, doc: Mode of GPIO2 (changes take effect after reboot), c_name: 'gpio_modes[2]'} - gpio3_mode: {type: GpioMode, doc: Mode of GPIO3 (changes take effect after reboot), c_name: 'gpio_modes[3]'} - gpio4_mode: {type: GpioMode, doc: Mode of GPIO4 (changes take effect after reboot), c_name: 'gpio_modes[4]'} - gpio5_mode: {type: GpioMode, doc: Mode of GPIO5 (changes take effect after reboot), c_name: 'gpio_modes[5]'} - gpio6_mode: {type: GpioMode, doc: Mode of GPIO6 (changes take effect after reboot), c_name: 'gpio_modes[6]'} - gpio7_mode: {type: GpioMode, doc: Mode of GPIO7 (changes take effect after reboot), c_name: 'gpio_modes[7]'} - gpio8_mode: {type: GpioMode, doc: Mode of GPIO8 (changes take effect after reboot), c_name: 'gpio_modes[8]'} - gpio9_mode: {type: GpioMode, doc: Mode of GPIO9 (changes take effect after reboot), c_name: 'gpio_modes[9]'} - gpio10_mode: {type: GpioMode, doc: Mode of GPIO10 (changes take effect after reboot), c_name: 'gpio_modes[10]'} - gpio11_mode: {type: GpioMode, doc: Mode of GPIO11 (changes take effect after reboot), c_name: 'gpio_modes[11]'} - gpio12_mode: {type: GpioMode, doc: Mode of GPIO12 (changes take effect after reboot), c_name: 'gpio_modes[12]'} - gpio13_mode: {type: GpioMode, doc: Mode of GPIO13 (changes take effect after reboot), c_name: 'gpio_modes[13]'} - gpio14_mode: {type: GpioMode, doc: Mode of GPIO14 (changes take effect after reboot), c_name: 'gpio_modes[14]'} - gpio15_mode: {type: GpioMode, doc: Mode of GPIO15 (changes take effect after reboot), c_name: 'gpio_modes[15]'} - gpio16_mode: {type: GpioMode, doc: Mode of GPIO16 (changes take effect after reboot), c_name: 'gpio_modes[16]'} - - enable_uart_a: - type: bool - brief: Enables/disables UART_A. - doc: | - You also need to set the corresponding GPIOs to GPIO_MODE_UART_A. - Refer to [interfaces](interfaces.md) to see which pins support UART_A. - Changing this requires a reboot. - enable_uart_b: - type: bool - brief: Enables/disables UART_B. - doc: | - You also need to set the corresponding GPIOs to GPIO_MODE_UART_B. - Refer to [interfaces](interfaces.md) to see which pins support UART_B. - Changing this requires a reboot. - enable_uart_c: {type: bool, doc: Not supported on ODrive v3.x.} - uart_a_baudrate: - type: uint32 - unit: baud/s - brief: Defines the baudrate used on the UART interface. - doc: | - Some baudrates will have a small timing error due to hardware limitations. - - Here's an (incomplete) list of baudrates for ODrive v3.x: - - Configured | Actual | Error [%] - -------------|---------------|----------- - 1.2 KBps | 1.2 KBps | 0 - 2.4 KBps | 2.4 KBps | 0 - 9.6 KBps | 9.6 KBps | 0 - 19.2 KBps | 19.195 KBps | 0.02 - 38.4 KBps | 38.391 KBps | 0.02 - 57.6 KBps | 57.613 KBps | 0.02 - 115.2 KBps | 115.068 KBps | 0.11 - 230.4 KBps | 230.769 KBps | 0.16 - 460.8 KBps | 461.538 KBps | 0.16 - 921.6 KBps | 913.043 KBps | 0.93 - 1.792 MBps | 1.826 MBps | 1.9 - 1.8432 MBps | 1.826 MBps | 0.93 - - For more information refer to Section 30.3.4 and Table 142 (the column with f_PCLK = 42 MHz) in the - [STM datasheet](https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf). - uart_b_baudrate: - type: uint32 - unit: baud/s - brief: Defines the baudrate used on the UART interface. - doc: See `uart_a_baudrate` for details. - uart_c_baudrate: {type: uint32, doc: Not supported on ODrive v3.x.} - enable_can_a: - type: bool - doc: | - Enables CAN. Changing this setting requires a reboot. - enable_i2c_a: - type: bool - doc: | - Enables I2C. The I2C pins on ODrive v3.x are in conflict with CAN. - This setting has no effect if `enable_can_a` is also true. - This setting has no effect on ODrive v3.2 or earlier. - Changing this setting requires a reboot. - enable_ascii_protocol_on_usb: bool - max_regen_current: float32 - brake_resistance: - type: float32 - unit: Ohm - brief: Value of the brake resistor connected to the ODrive. - doc: | - If you set this to a lower value than the true brake resistance - then the ODrive will not meed the `max_regen_current` constraint - during braking, that is it will sink more than `max_regen_current` - into the power supply. Some power supplies don't like this. - - If you set this to a higher value than the true brake resistance - then the ODrive will unnecessarily burn more power than required - during braking. - enable_brake_resistor: - type: bool - brief: Enable/disable the use of a brake resistor. - doc: | - Setting this to False even though a brake resistor is connected is - harmless. Setting this to True even though no brake resistor is - connected can break the power supply. - Changes to this value require a reboot to take effect. - - dc_bus_undervoltage_trip_level: - type: float32 - unit: V - brief: Minimum voltage below which the motor stops operating. - dc_bus_overvoltage_trip_level: - type: float32 - unit: V - brief: Maximum voltage above which the motor stops operating. - doc: | - This protects against cases in which the power supply fails to dissipate - the brake power if the brake resistor is disabled. - The default is 26V for the 24V board version and 52V for the 48V board version. - - enable_dc_bus_overvoltage_ramp: - type: bool - status: experimental - brief: Enables the DC bus overvoltage ramp feature. - doc: | - If enabled, if the measured DC voltage exceeds `dc_bus_overvoltage_ramp_start`, - the ODrive will sink more power than usual into the the brake resistor - in an attempt to bring the voltage down again. - - The brake duty cycle is increased by the following amount: - - * `vbus_voltage` == `dc_bus_overvoltage_ramp_start` => brake_duty_cycle += 0% - * `vbus_voltage` == `dc_bus_overvoltage_ramp_end` => brake_duty_cycle += 100% - - Remarks: - - This feature is active even when all motors are disarmed. - - This feature is disabled if `brake_resistance` is non-positive. - dc_bus_overvoltage_ramp_start: - type: float32 - status: experimental - brief: See `enable_dc_bus_overvoltage_ramp`. - doc: Do not set this lower than your usual `vbus_voltage`, - unless you like fried brake resistors. - dc_bus_overvoltage_ramp_end: - type: float32 - status: experimental - brief: See `enable_dc_bus_overvoltage_ramp`. - doc: Must be larger than `dc_bus_overvoltage_ramp_start`, - otherwise the ramp feature is disabled. - - dc_max_positive_current: - type: float32 - unit: A - brief: Max current the power supply can source. - dc_max_negative_current: - type: float32 - unit: A - brief: Max current the power supply can sink. - doc: You most likely want a non-positive value here. Set to -INFINITY to disable. - - error_gpio_pin: {type: uint32} - - gpio1_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[0]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} - gpio2_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[1]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} - gpio3_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[2]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} - gpio4_pwm_mapping: {type: Endpoint, c_name: 'pwm_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM0`.} - gpio3_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} - gpio4_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[4]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} user_config_loaded: readonly uint32 misconfigured: # TODO: make this a system error @@ -343,8 +178,6 @@ interfaces: capability were both used as interrupt input. Example: `step_gpio_pin` of both axes were set to the same GPIO. - axis0: {type: Axis, c_name: get_axis(0)} - axis1: {type: Axis, c_name: get_axis(1)} oscilloscope: {type: Oscilloscope} can: {type: Can, c_name: get_can()} test_property: uint32 @@ -384,6 +217,149 @@ interfaces: clear_errors: doc: Clear all the errors of this device including all contained submodules. + ODrive.Config: + c_is_class: False + attributes: + enable_uart_a: + type: bool + brief: Enables/disables UART_A. + doc: | + You also need to set the corresponding GPIOs to GPIO_MODE_UART_A. + Refer to [interfaces](interfaces.md) to see which pins support UART_A. + Changing this requires a reboot. + enable_uart_b: + type: bool + brief: Enables/disables UART_B. + doc: | + You also need to set the corresponding GPIOs to GPIO_MODE_UART_B. + Refer to [interfaces](interfaces.md) to see which pins support UART_B. + Changing this requires a reboot. + enable_uart_c: {type: bool, doc: Not supported on ODrive v3.x.} + uart_a_baudrate: + type: uint32 + unit: baud/s + brief: Defines the baudrate used on the UART interface. + doc: | + Some baudrates will have a small timing error due to hardware limitations. + + Here's an (incomplete) list of baudrates for ODrive v3.x: + + Configured | Actual | Error [%] + -------------|---------------|----------- + 1.2 KBps | 1.2 KBps | 0 + 2.4 KBps | 2.4 KBps | 0 + 9.6 KBps | 9.6 KBps | 0 + 19.2 KBps | 19.195 KBps | 0.02 + 38.4 KBps | 38.391 KBps | 0.02 + 57.6 KBps | 57.613 KBps | 0.02 + 115.2 KBps | 115.068 KBps | 0.11 + 230.4 KBps | 230.769 KBps | 0.16 + 460.8 KBps | 461.538 KBps | 0.16 + 921.6 KBps | 913.043 KBps | 0.93 + 1.792 MBps | 1.826 MBps | 1.9 + 1.8432 MBps | 1.826 MBps | 0.93 + + For more information refer to Section 30.3.4 and Table 142 (the column with f_PCLK = 42 MHz) in the + [STM datasheet](https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf). + uart_b_baudrate: + type: uint32 + unit: baud/s + brief: Defines the baudrate used on the UART interface. + doc: See `uart_a_baudrate` for details. + uart_c_baudrate: {type: uint32, doc: Not supported on ODrive v3.x.} + enable_can_a: + type: bool + doc: | + Enables CAN. Changing this setting requires a reboot. + enable_i2c_a: + type: bool + doc: | + Enables I2C. The I2C pins on ODrive v3.x are in conflict with CAN. + This setting has no effect if `enable_can_a` is also true. + This setting has no effect on ODrive v3.2 or earlier. + Changing this setting requires a reboot. + enable_ascii_protocol_on_usb: bool + max_regen_current: float32 + brake_resistance: + type: float32 + unit: Ohm + brief: Value of the brake resistor connected to the ODrive. + doc: | + If you set this to a lower value than the true brake resistance + then the ODrive will not meed the `max_regen_current` constraint + during braking, that is it will sink more than `max_regen_current` + into the power supply. Some power supplies don't like this. + + If you set this to a higher value than the true brake resistance + then the ODrive will unnecessarily burn more power than required + during braking. + enable_brake_resistor: + type: bool + brief: Enable/disable the use of a brake resistor. + doc: | + Setting this to False even though a brake resistor is connected is + harmless. Setting this to True even though no brake resistor is + connected can break the power supply. + Changes to this value require a reboot to take effect. + + dc_bus_undervoltage_trip_level: + type: float32 + unit: V + brief: Minimum voltage below which the motor stops operating. + dc_bus_overvoltage_trip_level: + type: float32 + unit: V + brief: Maximum voltage above which the motor stops operating. + doc: | + This protects against cases in which the power supply fails to dissipate + the brake power if the brake resistor is disabled. + The default is 26V for the 24V board version and 52V for the 48V board version. + + enable_dc_bus_overvoltage_ramp: + type: bool + status: experimental + brief: Enables the DC bus overvoltage ramp feature. + doc: | + If enabled, if the measured DC voltage exceeds `dc_bus_overvoltage_ramp_start`, + the ODrive will sink more power than usual into the the brake resistor + in an attempt to bring the voltage down again. + + The brake duty cycle is increased by the following amount: + + * `vbus_voltage` == `dc_bus_overvoltage_ramp_start` => brake_duty_cycle += 0% + * `vbus_voltage` == `dc_bus_overvoltage_ramp_end` => brake_duty_cycle += 100% + + Remarks: + - This feature is active even when all motors are disarmed. + - This feature is disabled if `brake_resistance` is non-positive. + dc_bus_overvoltage_ramp_start: + type: float32 + status: experimental + brief: See `enable_dc_bus_overvoltage_ramp`. + doc: Do not set this lower than your usual `vbus_voltage`, + unless you like fried brake resistors. + dc_bus_overvoltage_ramp_end: + type: float32 + status: experimental + brief: See `enable_dc_bus_overvoltage_ramp`. + doc: Must be larger than `dc_bus_overvoltage_ramp_start`, + otherwise the ramp feature is disabled. + + dc_max_positive_current: + type: float32 + unit: A + brief: Max current the power supply can source. + dc_max_negative_current: + type: float32 + unit: A + brief: Max current the power supply can sink. + doc: You most likely want a non-positive value here. Set to -INFINITY to disable. + + error_gpio_pin: {type: uint32} + + gpio3_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} + gpio4_analog_mapping: {type: Endpoint, c_name: 'analog_mappings[4]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_ANALOG_IN`.} + ODrive.Can: c_is_class: True attributes: @@ -1101,6 +1077,39 @@ interfaces: length: readonly uint32 max_length: uint32 + ODrive3: + c_is_class: True + implements: ODrive + attributes: + config: + c_is_class: False + implements: ODrive.Config + attributes: + # TODO: add support for arrays + gpio1_mode: {type: ODrive.GpioMode, doc: Mode of GPIO1 (changes take effect after reboot), c_name: 'gpio_modes[1]'} + gpio2_mode: {type: ODrive.GpioMode, doc: Mode of GPIO2 (changes take effect after reboot), c_name: 'gpio_modes[2]'} + gpio3_mode: {type: ODrive.GpioMode, doc: Mode of GPIO3 (changes take effect after reboot), c_name: 'gpio_modes[3]'} + gpio4_mode: {type: ODrive.GpioMode, doc: Mode of GPIO4 (changes take effect after reboot), c_name: 'gpio_modes[4]'} + gpio5_mode: {type: ODrive.GpioMode, doc: Mode of GPIO5 (changes take effect after reboot), c_name: 'gpio_modes[5]'} + gpio6_mode: {type: ODrive.GpioMode, doc: Mode of GPIO6 (changes take effect after reboot), c_name: 'gpio_modes[6]'} + gpio7_mode: {type: ODrive.GpioMode, doc: Mode of GPIO7 (changes take effect after reboot), c_name: 'gpio_modes[7]'} + gpio8_mode: {type: ODrive.GpioMode, doc: Mode of GPIO8 (changes take effect after reboot), c_name: 'gpio_modes[8]'} + gpio9_mode: {type: ODrive.GpioMode, doc: Mode of GPIO9 (changes take effect after reboot), c_name: 'gpio_modes[9]'} + gpio10_mode: {type: ODrive.GpioMode, doc: Mode of GPIO10 (changes take effect after reboot), c_name: 'gpio_modes[10]'} + gpio11_mode: {type: ODrive.GpioMode, doc: Mode of GPIO11 (changes take effect after reboot), c_name: 'gpio_modes[11]'} + gpio12_mode: {type: ODrive.GpioMode, doc: Mode of GPIO12 (changes take effect after reboot), c_name: 'gpio_modes[12]'} + gpio13_mode: {type: ODrive.GpioMode, doc: Mode of GPIO13 (changes take effect after reboot), c_name: 'gpio_modes[13]'} + gpio14_mode: {type: ODrive.GpioMode, doc: Mode of GPIO14 (changes take effect after reboot), c_name: 'gpio_modes[14]'} + gpio15_mode: {type: ODrive.GpioMode, doc: Mode of GPIO15 (changes take effect after reboot), c_name: 'gpio_modes[15]'} + gpio16_mode: {type: ODrive.GpioMode, doc: Mode of GPIO16 (changes take effect after reboot), c_name: 'gpio_modes[16]'} + + gpio1_pwm_mapping: {type: ODrive.Endpoint, c_name: 'pwm_mappings[0]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} + gpio2_pwm_mapping: {type: ODrive.Endpoint, c_name: 'pwm_mappings[1]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} + gpio3_pwm_mapping: {type: ODrive.Endpoint, c_name: 'pwm_mappings[2]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} + gpio4_pwm_mapping: {type: ODrive.Endpoint, c_name: 'pwm_mappings[3]', doc: Make sure the corresponding GPIO is in `GPIO_MODE_PWM`.} + axis0: {type: ODrive.Axis, c_name: get_axis(0)} + axis1: {type: ODrive.Axis, c_name: get_axis(1)} + valuetypes: ODrive.GpioMode: values: From df83782bd4d7d08142442a8d1d410f4e000a61b8 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Wed, 18 Nov 2020 18:51:19 +0100 Subject: [PATCH 25/28] re-add missing objects in ASCII protocol --- Firmware/communication/ascii_protocol.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Firmware/communication/ascii_protocol.cpp b/Firmware/communication/ascii_protocol.cpp index 578c3f7b..811efac6 100644 --- a/Firmware/communication/ascii_protocol.cpp +++ b/Firmware/communication/ascii_protocol.cpp @@ -28,7 +28,9 @@ /* Private variables ---------------------------------------------------------*/ -static Introspectable root_obj = ODriveTypeInfo::make_introspectable(odrv); +#if HW_VERSION_MAJOR == 3 +static Introspectable root_obj = ODrive3TypeInfo::make_introspectable(odrv); +#endif /* Private function prototypes -----------------------------------------------*/ From 287dd47b8f71f6faf6950a3ab844f06ddff5d429 Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Thu, 19 Nov 2020 12:19:02 +0100 Subject: [PATCH 26/28] move startup delay to fix race condition --- Firmware/MotorControl/axis.cpp | 14 -------------- Firmware/MotorControl/main.cpp | 21 +++++++++++++++++++-- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 9b12a3c4..07fd263f 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -428,20 +428,6 @@ bool Axis::run_idle_loop() { // Infinite loop that does calibration and enters main control loop as appropriate void Axis::run_state_machine_loop() { - - // Wait for up to 2s for motor to become ready to allow for error-free - // startup. This delay gives the current sensor calibration time to - // converge. If the DRV chip is unpowered, the motor will not become ready - // but we still enter idle state. - for (size_t i = 0; i < 2000; ++i) { - if (motor_.current_meas_.has_value()) { - break; - } - osDelay(1); - } - - sensorless_estimator_.error_ &= ~SensorlessEstimator::ERROR_UNKNOWN_CURRENT_MEASUREMENT; - for (;;) { // Load the task chain if a specific request is pending if (requested_state_ != AXIS_STATE_UNDEFINED) { diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 834ce743..469f71a0 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -485,6 +485,25 @@ static void rtos_main(void*) { // Start PWM and enable adc interrupts/callbacks start_adc_pwm(); + start_analog_thread(); + + // Wait for up to 2s for motor to become ready to allow for error-free + // startup. This delay gives the current sensor calibration time to + // converge. If the DRV chip is unpowered, the motor will not become ready + // but we still enter idle state. + for (size_t i = 0; i < 2000; ++i) { + bool motors_ready = std::all_of(axes.begin(), axes.end(), [](auto& axis) { + return axis.motor_.current_meas_.has_value(); + }); + if (motors_ready) { + break; + } + osDelay(1); + } + + for (auto& axis: axes) { + axis.sensorless_estimator_.error_ &= ~SensorlessEstimator::ERROR_UNKNOWN_CURRENT_MEASUREMENT; + } // Start state machine threads. Each thread will go through various calibration // procedures and then run the actual controller loops. @@ -493,8 +512,6 @@ static void rtos_main(void*) { axes[i].start_thread(); } - start_analog_thread(); - odrv.system_stats_.fully_booted = true; // Main thread finished starting everything and can delete itself now (yes this is legal). From f150114d7707dab3f2182d1deee29b5495c6210b Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Thu, 19 Nov 2020 15:32:50 +0100 Subject: [PATCH 27/28] fix CI --- .github/workflows/compile.yaml | 2 +- .github/workflows/nightly.yaml | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/compile.yaml b/.github/workflows/compile.yaml index f8468433..380730df 100644 --- a/.github/workflows/compile.yaml +++ b/.github/workflows/compile.yaml @@ -76,7 +76,7 @@ jobs: run: | Invoke-WebRequest -Uri "http://gittup.org/tup/win32/tup-latest.zip" -OutFile ".\tup-latest.zip" Expand-Archive ".\tup-latest.zip" -DestinationPath ".\tup-latest" -Force - echo "::add-path::$(Resolve-Path .)\tup-latest" + echo "$(Resolve-Path .)\tup-latest" >> $GITHUB_PATH choco install gcc-arm-embedded # downloads https://developer.arm.com/-/media/Files/downloads/gnu-rm/9-2019q4/gcc-arm-none-eabi-9-2019-q4-major-win32.zip diff --git a/.github/workflows/nightly.yaml b/.github/workflows/nightly.yaml index 8769d92d..42951d0f 100644 --- a/.github/workflows/nightly.yaml +++ b/.github/workflows/nightly.yaml @@ -16,7 +16,11 @@ jobs: steps: - name: Install odrivetool run: | - pip3 install monotonic # TODO: this is dishonest. Must be removed as soon as v0.5.0 is published! + # TODO: this is a workaround for https://github.com/pypa/setuptools/issues/2353 and we + # can remove it as soon as python3-setuptools on GitHub's ubuntu-latest + # moves to version 50.1+. + pip3 list | grep setuptools # show version + export SETUPTOOLS_USE_DISTUTILS=stdlib pip3 install odrive # This one currently fails because Github Actions runs pip as non-root @@ -27,7 +31,7 @@ jobs: # This step is mentioned in the user guide - name: Add ~/.local/bin to path if: matrix.os == 'ubuntu-latest' - run: echo "::add-path::~/.local/bin" + run: echo "~/.local/bin" >> $GITHUB_PATH - name: Launch odrivetool # This returns a non-zero exit code if the odrivetool throws an exception From 84c8ead0829fa41f9c2cdc5b7ff0c75b48d63a4f Mon Sep 17 00:00:00 2001 From: Paul Guenette Date: Fri, 27 Nov 2020 00:38:38 -0500 Subject: [PATCH 28/28] Add index offset Make index_offset in turns instead of counts Get rid of *-1 in the index offset Fix --- Firmware/MotorControl/encoder.cpp | 4 ++-- Firmware/MotorControl/encoder.hpp | 26 ++++++++++++++------------ Firmware/odrive-interface.yaml | 3 ++- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Firmware/MotorControl/encoder.cpp b/Firmware/MotorControl/encoder.cpp index 358dca72..05318b49 100644 --- a/Firmware/MotorControl/encoder.cpp +++ b/Firmware/MotorControl/encoder.cpp @@ -80,8 +80,8 @@ bool Encoder::do_checks(){ void Encoder::enc_index_cb() { if (config_.use_index) { set_circular_count(0, false); - if (config_.zero_count_on_find_idx) - set_linear_count(0); // Avoid position control transient after search + if (config_.use_index_offset) + set_linear_count((int32_t)(config_.index_offset * config_.cpr)); if (config_.pre_calibrated) { is_ready_ = true; if(axis_->controller_.config_.anticogging.pre_calibrated){ diff --git a/Firmware/MotorControl/encoder.hpp b/Firmware/MotorControl/encoder.hpp index 841f7a4f..9676b396 100644 --- a/Firmware/MotorControl/encoder.hpp +++ b/Firmware/MotorControl/encoder.hpp @@ -14,27 +14,29 @@ public: struct Config_t { Mode mode = MODE_INCREMENTAL; + float calib_range = 0.02f; // Accuracy required to pass encoder cpr check + float calib_scan_distance = 16.0f * M_PI; // rad electrical + float calib_scan_omega = 4.0f * M_PI; // rad/s electrical + float bandwidth = 1000.0f; + int32_t phase_offset = 0; // Offset between encoder count and rotor electrical phase + float phase_offset_float = 0.0f; // Sub-count phase alignment offset + int32_t cpr = (2048 * 4); // Default resolution of CUI-AMT102 encoder, + float index_offset = 0.0f; + uint16_t abs_spi_cs_gpio_pin = 1; + uint16_t sincos_gpio_pin_sin = 3; + uint16_t sincos_gpio_pin_cos = 4; bool use_index = false; bool pre_calibrated = false; // If true, this means the offset stored in // configuration is valid and does not need // be determined by run_offset_calibration. // In this case the encoder will enter ready // state as soon as the index is found. - bool zero_count_on_find_idx = true; - int32_t cpr = (2048 * 4); // Default resolution of CUI-AMT102 encoder, - int32_t phase_offset = 0; // Offset between encoder count and rotor electrical phase - float phase_offset_float = 0.0f; // Sub-count phase alignment offset - int32_t direction = 0.0f; // direction with respect to motor + int32_t direction = 0; // direction with respect to motor + bool use_index_offset = true; bool enable_phase_interpolation = true; // Use velocity to interpolate inside the count state - float calib_range = 0.02f; // Accuracy required to pass encoder cpr check - float calib_scan_distance = 16.0f * M_PI; // rad electrical - float calib_scan_omega = 4.0f * M_PI; // rad/s electrical - float bandwidth = 1000.0f; bool find_idx_on_lockin_only = false; // Only be sensitive during lockin scan constant vel state bool ignore_illegal_hall_state = false; // dont error on bad states like 000 or 111 - uint16_t abs_spi_cs_gpio_pin = 1; - uint16_t sincos_gpio_pin_sin = 3; - uint16_t sincos_gpio_pin_cos = 4; + // custom setters Encoder* parent = nullptr; diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml index d88af102..c186b678 100644 --- a/Firmware/odrive-interface.yaml +++ b/Firmware/odrive-interface.yaml @@ -984,9 +984,10 @@ interfaces: attributes: mode: Mode use_index: {type: bool, c_setter: set_use_index} + index_offset: float32 + use_index_offset: bool find_idx_on_lockin_only: {type: bool, c_setter: set_find_idx_on_lockin_only} abs_spi_cs_gpio_pin: {type: uint16, c_setter: set_abs_spi_cs_gpio_pin, doc: Make sure that the GPIO is in `GPIO_MODE_DIGITAL`.} - zero_count_on_find_idx: bool cpr: int32 phase_offset: int32 phase_offset_float: float32