From 183998d64c7a818d2c111bd086ec17c9dc28b1ac Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Fri, 5 Oct 2018 23:00:57 -0700 Subject: [PATCH 1/3] odrive_main.h include scheme note --- Firmware/MotorControl/odrive_main.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h index 0165c673..776f1590 100644 --- a/Firmware/MotorControl/odrive_main.h +++ b/Firmware/MotorControl/odrive_main.h @@ -1,6 +1,13 @@ #ifndef __ODRIVE_MAIN_H #define __ODRIVE_MAIN_H +// Note on central include scheme by Samuel: +// there are circular dependencies between some of the header files, +// e.g. the Motor header needs a forward declaration of Axis and vice versa +// so I figured I'd make one main header that takes care of +// the forward declarations and right ordering +// btw this pattern is not so uncommon, for instance IIRC the stdlib uses it too + #ifdef __cplusplus #include extern "C" { From 85540cb1ab14498fe3994670ab0f71a7620a2270 Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Tue, 2 Oct 2018 00:44:42 -0700 Subject: [PATCH 2/3] move check_for_errors to inline --- Firmware/MotorControl/axis.cpp | 13 ------------- Firmware/MotorControl/axis.hpp | 14 +++++++++++++- Firmware/MotorControl/motor.cpp | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp index 4ff660fd..01892af1 100644 --- a/Firmware/MotorControl/axis.cpp +++ b/Firmware/MotorControl/axis.cpp @@ -96,19 +96,6 @@ void Axis::set_step_dir_enabled(bool enable) { } } -bool Axis::check_for_errors() { - // Maybe we should update this to only trigger on new errors? - // The danger with that is we could fail to bail on uncleared errors that still prevent - // correct opreation. - - // For now: we treat ERROR_INVALID_STATE in idle loop special, or we could never stay - // in idle after this kind of error. - if (current_state_ == AXIS_STATE_IDLE) - return (error_ & ~ERROR_INVALID_STATE) == ERROR_NONE; - else - return error_ == ERROR_NONE; -} - // @brief Do axis level checks and call subcomponent do_checks // Returns true if everything is ok. bool Axis::do_checks() { diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index 6063ee3a..8306d1f5 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -80,9 +80,21 @@ public: bool check_PSU_brownout(); bool do_checks(); bool do_updates(); - bool check_for_errors(); float get_temp(); + bool inline check_for_errors() { + // Maybe we should update this to only trigger on new errors? + // The danger with that is we could fail to bail on uncleared errors that still prevent + // correct opreation. + + // For now: we treat ERROR_INVALID_STATE in idle loop special, or we could never stay + // in idle after this kind of error. + if (current_state_ == AXIS_STATE_IDLE) + return (error_ & ~ERROR_INVALID_STATE) == ERROR_NONE; + else + return error_ == ERROR_NONE; + } + // @brief Runs the specified update handler at the frequency of the current measurements. // // The loop runs until one of the following conditions: diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp index 5f518adb..ee71c98a 100644 --- a/Firmware/MotorControl/motor.cpp +++ b/Firmware/MotorControl/motor.cpp @@ -343,7 +343,7 @@ bool Motor::FOC_current(float Id_des, float Iq_des, float phase) { // Inverse park transform float mod_alpha = c * mod_d - s * mod_q; - float mod_beta = c * mod_q + s * mod_d; + float mod_beta = c * mod_q + s * mod_d; // Report final applied voltage in stationary frame (for sensorles estimator) ictrl.final_v_alpha = mod_to_V * mod_alpha; From d29697255a599dba1ce5cba8c8dbc96d32346c1f Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Sat, 6 Oct 2018 01:47:36 -0700 Subject: [PATCH 3/3] fix axis state machine jumping out of idle when there is an error --- CHANGELOG.md | 1 + Firmware/MotorControl/axis.hpp | 21 +++++++++------------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1257724b..9cef2851 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Please add a note of your changes below this heading if you make a Pull Request. * `encoder.config.bandwidth` ### Fixed +* An issue where the axis state machine would jump in and out of idle when there is an error * There is a [bug](https://github.com/ARM-software/CMSIS_5/issues/267) in the arm fast math library, which gives spikes in the output of arm_cos_f32 for input values close to -pi/2. We fixed the bug locally, and hence are using "our_arm_cos_f32". # Releases diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp index 8306d1f5..5d4d0dd8 100644 --- a/Firmware/MotorControl/axis.hpp +++ b/Firmware/MotorControl/axis.hpp @@ -82,17 +82,10 @@ public: bool do_updates(); float get_temp(); - bool inline check_for_errors() { - // Maybe we should update this to only trigger on new errors? - // The danger with that is we could fail to bail on uncleared errors that still prevent - // correct opreation. - // For now: we treat ERROR_INVALID_STATE in idle loop special, or we could never stay - // in idle after this kind of error. - if (current_state_ == AXIS_STATE_IDLE) - return (error_ & ~ERROR_INVALID_STATE) == ERROR_NONE; - else - return error_ == ERROR_NONE; + // True if there are no errors + bool inline check_for_errors() { + return error_ == ERROR_NONE; } // @brief Runs the specified update handler at the frequency of the current measurements. @@ -124,8 +117,12 @@ public: // Note: updates run even if checks fail bool updates_ok = do_updates(); - if (!checks_ok || !updates_ok) - break; + if (!checks_ok || !updates_ok) { + // It's not useful to quit idle since that is the safe action + // Also leaving idle would rearm the motors + if (current_state_ != AXIS_STATE_IDLE) + break; + } // Run main loop function, defer quitting for after wait // TODO: change arming logic to arm after waiting