From 43cb91b3b1d257a13a5770b44de92ed0064f8d69 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Tue, 18 Aug 2020 23:37:44 -0700
Subject: [PATCH 01/12] Mechanical Brake feature

---
 Firmware/MotorControl/axis.cpp             |  8 ++-
 Firmware/MotorControl/axis.hpp             |  4 +-
 Firmware/MotorControl/main.cpp             |  9 ++-
 Firmware/MotorControl/mechanical_brake.cpp | 41 ++++++++++++++
 Firmware/MotorControl/mechanical_brake.hpp | 36 ++++++++++++
 Firmware/MotorControl/odrive_main.h        |  1 +
 Firmware/Tupfile.lua                       |  1 +
 Firmware/odrive-interface.yaml             | 12 +++-
 docs/_data/index.yaml                      |  2 +
 docs/mechanical-brakes.md                  | 66 ++++++++++++++++++++++
 10 files changed, 176 insertions(+), 4 deletions(-)
 create mode 100644 Firmware/MotorControl/mechanical_brake.cpp
 create mode 100644 Firmware/MotorControl/mechanical_brake.hpp
 create mode 100644 docs/mechanical-brakes.md

diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp
index f5771cd2..96df5ab0 100644
--- a/Firmware/MotorControl/axis.cpp
+++ b/Firmware/MotorControl/axis.cpp
@@ -19,7 +19,8 @@ Axis::Axis(int axis_num,
            Motor& motor,
            TrapezoidalTrajectory& trap,
            Endstop& min_endstop,
-           Endstop& max_endstop)
+           Endstop& max_endstop,
+           MechanicalBrake& mechanical_brake)
     : axis_num_(axis_num),
       hw_config_(hw_config),
       config_(config),
@@ -32,6 +33,7 @@ Axis::Axis(int axis_num,
       trap_traj_(trap),
       min_endstop_(min_endstop),
       max_endstop_(max_endstop),
+      mechanical_brake_(mechanical_brake),
       current_limiters_(make_array(
           static_cast<CurrentLimiter*>(&fet_thermistor),
           static_cast<CurrentLimiter*>(&motor_thermistor))),
@@ -48,6 +50,7 @@ Axis::Axis(int axis_num,
     trap_traj_.axis_ = this;
     min_endstop_.axis_ = this;
     max_endstop_.axis_ = this;
+    mechanical_brake_.axis_ = this;
     decode_step_dir_pins();
     watchdog_feed();
 }
@@ -462,6 +465,7 @@ bool Axis::run_idle_loop() {
     // run_control_loop ignores missed modulation timing updates
     // if and only if we're in AXIS_STATE_IDLE
     safety_critical_disarm_motor_pwm(motor_);
+    mechanical_brake_.engage();
     set_step_dir_active(config_.enable_step_dir && config_.step_dir_always_on);
     run_control_loop([this]() {
         return true;
@@ -474,6 +478,7 @@ void Axis::run_state_machine_loop() {
 
     // arm!
     motor_.arm();
+    mechanical_brake_.release();
 
     for (;;) {
         // Load the task chain if a specific request is pending
@@ -575,6 +580,7 @@ void Axis::run_state_machine_loop() {
             case AXIS_STATE_IDLE: {
                 run_idle_loop();
                 status = motor_.arm(); // done with idling - try to arm the motor
+                mechanical_brake_.release();
             } break;
 
             default:
diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp
index 3271e432..e96ab2ef 100644
--- a/Firmware/MotorControl/axis.hpp
+++ b/Firmware/MotorControl/axis.hpp
@@ -82,7 +82,8 @@ public:
             Motor& motor,
             TrapezoidalTrajectory& trap,
             Endstop& min_endstop,
-            Endstop& max_endstop);
+            Endstop& max_endstop,
+            MechanicalBrake& mechanical_brake);
 
     void setup();
     void start_thread();
@@ -210,6 +211,7 @@ public:
     TrapezoidalTrajectory& trap_traj_;
     Endstop& min_endstop_;
     Endstop& max_endstop_;
+    MechanicalBrake& mechanical_brake_;
 
     // List of current_limiters and thermistors to
     // provide easy iteration.
diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp
index 2809f5da..62f7ec1e 100644
--- a/Firmware/MotorControl/main.cpp
+++ b/Firmware/MotorControl/main.cpp
@@ -21,6 +21,7 @@ Axis::Config_t axis_configs[AXIS_COUNT];
 TrapezoidalTrajectory::Config_t trap_configs[AXIS_COUNT];
 Endstop::Config_t min_endstop_configs[AXIS_COUNT];
 Endstop::Config_t max_endstop_configs[AXIS_COUNT];
+MechanicalBrake::Config_t mechanical_brake_configs[AXIS_COUNT];
 
 std::array<Axis*, AXIS_COUNT> axes;
 ODriveCAN *odCAN = nullptr;
@@ -38,6 +39,7 @@ typedef Config<
     TrapezoidalTrajectory::Config_t[AXIS_COUNT],
     Endstop::Config_t[AXIS_COUNT],
     Endstop::Config_t[AXIS_COUNT],
+    MechanicalBrake::Config_t[AXIS_COUNT],
     Axis::Config_t[AXIS_COUNT]> ConfigFormat;
 
 void ODrive::save_configuration(void) {
@@ -53,6 +55,7 @@ void ODrive::save_configuration(void) {
             &trap_configs,
             &min_endstop_configs,
             &max_endstop_configs,
+            &mechanical_brake_configs,
             &axis_configs)) {
         printf("saving configuration failed\r\n"); osDelay(5);
     } else {
@@ -75,6 +78,7 @@ extern "C" int load_configuration(void) {
                 &trap_configs,
                 &min_endstop_configs,
                 &max_endstop_configs,
+                &mechanical_brake_configs,
                 &axis_configs)) {
         //If loading failed, restore defaults
         odrv.config_ = BoardConfig_t();
@@ -93,6 +97,7 @@ extern "C" int load_configuration(void) {
             Axis::load_default_can_id(i, axis_configs[i]);
             min_endstop_configs[i] = Endstop::Config_t();
             max_endstop_configs[i] = Endstop::Config_t();
+            mechanical_brake_configs[i] = MechanicalBrake::Config_t();
             controller_configs[i].load_encoder_axis = i;
         }
     } else {
@@ -194,9 +199,10 @@ extern "C" int construct_objects(){
         TrapezoidalTrajectory *trap = new TrapezoidalTrajectory(trap_configs[i]);
         Endstop *min_endstop = new Endstop(min_endstop_configs[i]);
         Endstop *max_endstop = new Endstop(max_endstop_configs[i]);
+        MechanicalBrake *mechanical_brake = new MechanicalBrake(mechanical_brake_configs[i]);
         axes[i] = new Axis(i, hw_configs[i].axis_config, axis_configs[i],
                 *encoder, *sensorless_estimator, *controller, *fet_thermistor,
-                *motor_thermistor, *motor, *trap, *min_endstop, *max_endstop);
+                *motor_thermistor, *motor, *trap, *min_endstop, *max_endstop, *mechanical_brake);
 
         controller_configs[i].parent = controller;
         encoder_configs[i].parent = encoder;
@@ -204,6 +210,7 @@ extern "C" int construct_objects(){
         motor_configs[i].parent = motor;
         min_endstop_configs[i].parent = min_endstop;
         max_endstop_configs[i].parent = max_endstop;
+        mechanical_brake_configs[i].parent = mechanical_brake;
         axis_configs[i].parent = axes[i];
     }
     return 0;
diff --git a/Firmware/MotorControl/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
new file mode 100644
index 00000000..27707d2a
--- /dev/null
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -0,0 +1,41 @@
+#include <odrive_main.h>
+
+MechanicalBrake::MechanicalBrake(MechanicalBrake::Config_t& config)
+    : config_(config) {
+    update_config();
+}
+
+bool MechanicalBrake::get_state() {
+    return mechanical_brake_state_;
+}
+
+void MechanicalBrake::update_config() {
+    set_enabled(config_.enabled);
+}
+
+void MechanicalBrake::engage() {
+    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_RESET : GPIO_PIN_SET );
+}
+
+void MechanicalBrake::release() {
+    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_SET : GPIO_PIN_RESET );
+}
+
+void MechanicalBrake::set_enabled(bool enable) {
+    if (config_.gpio_num != 0) {
+        uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+        GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+        if (enable) {
+            HAL_GPIO_DeInit(gpio_port, gpio_pin);
+            GPIO_InitTypeDef GPIO_InitStruct;
+            GPIO_InitStruct.Pin = gpio_pin;
+            GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP;
+            GPIO_InitStruct.Pull = config_.pulldown ? GPIO_PULLDOWN : GPIO_PULLUP;
+            HAL_GPIO_Init(gpio_port, &GPIO_InitStruct);
+        }
+    }
+}
\ No newline at end of file
diff --git a/Firmware/MotorControl/mechanical_brake.hpp b/Firmware/MotorControl/mechanical_brake.hpp
new file mode 100644
index 00000000..9586f8b3
--- /dev/null
+++ b/Firmware/MotorControl/mechanical_brake.hpp
@@ -0,0 +1,36 @@
+#ifndef __MECHANICAL_BRAKE_HPP
+#define __MECHANICAL_BRAKE_HPP
+
+class MechanicalBrake {
+   public:
+    struct Config_t {
+        uint16_t gpio_num = 0;
+        bool enabled = false;
+        bool is_active_low = true;
+        bool pulldown = true;
+
+        // custom setters
+        MechanicalBrake* parent = nullptr;
+        void set_gpio_num(uint16_t value) { gpio_num = value; parent->update_config(); }
+        void set_enabled(uint32_t value) { enabled = value; parent->update_config(); }
+    };
+
+    explicit MechanicalBrake(MechanicalBrake::Config_t& config);
+
+    MechanicalBrake::Config_t& config_;
+    Axis* axis_ = nullptr;
+
+    void update_config();
+    void set_enabled(bool enabled);
+
+    void update();
+    bool get_state();
+    void release();
+    void engage();
+
+    bool mechanical_brake_state_ = true;
+
+   private:
+    bool pin_state_ = false;
+};
+#endif // __MECHANICAL_BRAKE_HPP
\ No newline at end of file
diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h
index d0dcface..88328993 100644
--- a/Firmware/MotorControl/odrive_main.h
+++ b/Firmware/MotorControl/odrive_main.h
@@ -210,6 +210,7 @@ enum TimingLog_t {
 #include <thermistor.hpp>
 #include <trapTraj.hpp>
 #include <endstop.hpp>
+#include <mechanical_brake.hpp>
 #include <axis.hpp>
 #include <communication/communication.h>
 
diff --git a/Firmware/Tupfile.lua b/Firmware/Tupfile.lua
index 7f5380ae..f8f6f8ec 100644
--- a/Firmware/Tupfile.lua
+++ b/Firmware/Tupfile.lua
@@ -194,6 +194,7 @@ build{
         'MotorControl/thermistor.cpp',
         'MotorControl/encoder.cpp',
         'MotorControl/endstop.cpp',
+        'MotorControl/mechanical_brake.cpp',
         'MotorControl/controller.cpp',
         'MotorControl/sensorless_estimator.cpp',
         'MotorControl/trapTraj.cpp',
diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml
index 2c9060eb..2a755275 100644
--- a/Firmware/odrive-interface.yaml
+++ b/Firmware/odrive-interface.yaml
@@ -364,6 +364,7 @@ interfaces:
       trap_traj: TrapezoidalTrajectory
       min_endstop: Endstop
       max_endstop: Endstop
+      mechanical_brake: MechanicalBrake
     functions:
       watchdog_feed:
         doc: Feed the watchdog to prevent watchdog timeouts.
@@ -830,7 +831,6 @@ interfaces:
           accel_limit: float32
           decel_limit: float32
 
-
   ODrive.Endstop:
     c_is_class: True
     attributes:
@@ -845,6 +845,16 @@ interfaces:
           pullup: bool
           debounce_ms: {type: uint32, c_setter: set_debounce_ms}
 
+  ODrive.MechanicalBrake:
+    c_is_class: True
+    attributes:
+      config:
+        c_is_class: False
+        attributes:
+          gpio_num: {type: uint16, c_setter: set_gpio_num}
+          enabled: {type: bool, c_setter: set_enabled}
+          is_active_low: bool
+          pulldown: bool
 
 valuetypes:
   ODrive.Can.Protocol:
diff --git a/docs/_data/index.yaml b/docs/_data/index.yaml
index 0ff47762..273029b6 100644
--- a/docs/_data/index.yaml
+++ b/docs/_data/index.yaml
@@ -17,6 +17,8 @@ sections:
         url: /encoders
       - title: Homing & Endstops
         url: /endstops
+      - title: Mechanical Brakes
+        url: /mechanical-brakes
       - title: Thermistors
         url: /thermistors
       - title: Control & Tuning
diff --git a/docs/mechanical-brakes.md b/docs/mechanical-brakes.md
new file mode 100644
index 00000000..dc2cdaf6
--- /dev/null
+++ b/docs/mechanical-brakes.md
@@ -0,0 +1,66 @@
+# Mechanical Brake
+
+Some systems employ mechanical brakes on motors as a safety feature. These brakes can also be engaged as a power-saving function if the motor is not moving, but still under load.
+
+ODrive supports the use of its GPIO pins to connect to external brake drive electronics.
+
+When the ODrive engages the drive electronics, the brake will be disabled. When the drive enters a fault or idle state, the brake will be re-engaged.
+
+---
+
+## Mechanical Brake Configuration
+Each axis supports one mechanical brake. The following properties are accessible through `odrivetool`:
+
+Name |  Type | Default
+--- | -- | -- 
+gpio_num | int | 0
+enabled | boolean | false
+is_active_low | boolean | true
+pulldown | boolean | true
+
+### gpio_num
+The GPIO pin number, according to the silkscreen labels on ODrive. Set with these commands:
+```
+<odrv>.<axis>.mechanical_brake.config.gpio_num = <1, 2, 3, 4, 5, 6, 7, 8>
+```
+
+### enabled
+Enables/disables the operation of the mechanical brake.
+```
+<odrv>.<axis>.mechanical_brake.config.enabled = <True, False>
+```
+
+### is_active_low
+Most safety braking systems are active low, e.g. when the power is off, the brake is on. If the system uses brake drive electronics which use active high logic, flip this bit then reconsider the safety implications of your design...
+
+### pulldown
+If `true`, it enables the GPIO pulldown resistor.  If `false`, it enables the GPIO pullup resistor.
+
+### Example
+
+Let's say we're hacking away on an old ABB robotic arm. We've wired a 24V brake drive circuit triggered by GPIO5. When GPIO5 is driven, it will release the brakes on the axis we're moving.
+
+```
+<odrv>.<axis>.mechanical_brake.config.gpio_num = 5
+<odrv>.<axis>.mechanical_brake.config.enabled = True
+```
+
+Don't forget to save and reboot:
+```
+<odrv>.save_configuration()
+<odrv>.reboot()
+```
+
+### Testing The Mechanical Brakes
+Depending on your system this could be a dangerous experiment. Ensure that you have taken all necessary precautions to confirm if the wrong brake were inadvertently released it would not lead to injury or damage to equipment.
+
+```
+<odrv>.<axis>.mechanical_brake.release()
+```
+Note: If a brake is enabled, it will be engaged/disengaged during the next state machine step. 
+
+After you're satisfied with the testing, you can re-enable the brake using the command
+
+```
+<odrv>.<axis>.mechanical_brake.engage()
+```

From 46c054f3af4d71cfd3ce940fa07f1377a13ed743 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Tue, 18 Aug 2020 23:37:44 -0700
Subject: [PATCH 02/12] Mechanical Brake feature

---
 Firmware/MotorControl/axis.cpp             |  8 ++-
 Firmware/MotorControl/axis.hpp             |  4 +-
 Firmware/MotorControl/main.cpp             |  9 ++-
 Firmware/MotorControl/mechanical_brake.cpp | 41 ++++++++++++++
 Firmware/MotorControl/mechanical_brake.hpp | 36 ++++++++++++
 Firmware/MotorControl/odrive_main.h        |  1 +
 Firmware/Tupfile.lua                       |  1 +
 Firmware/odrive-interface.yaml             | 12 +++-
 docs/_data/index.yaml                      |  2 +
 docs/mechanical-brakes.md                  | 66 ++++++++++++++++++++++
 10 files changed, 176 insertions(+), 4 deletions(-)
 create mode 100644 Firmware/MotorControl/mechanical_brake.cpp
 create mode 100644 Firmware/MotorControl/mechanical_brake.hpp
 create mode 100644 docs/mechanical-brakes.md

diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp
index f5771cd2..96df5ab0 100644
--- a/Firmware/MotorControl/axis.cpp
+++ b/Firmware/MotorControl/axis.cpp
@@ -19,7 +19,8 @@ Axis::Axis(int axis_num,
            Motor& motor,
            TrapezoidalTrajectory& trap,
            Endstop& min_endstop,
-           Endstop& max_endstop)
+           Endstop& max_endstop,
+           MechanicalBrake& mechanical_brake)
     : axis_num_(axis_num),
       hw_config_(hw_config),
       config_(config),
@@ -32,6 +33,7 @@ Axis::Axis(int axis_num,
       trap_traj_(trap),
       min_endstop_(min_endstop),
       max_endstop_(max_endstop),
+      mechanical_brake_(mechanical_brake),
       current_limiters_(make_array(
           static_cast<CurrentLimiter*>(&fet_thermistor),
           static_cast<CurrentLimiter*>(&motor_thermistor))),
@@ -48,6 +50,7 @@ Axis::Axis(int axis_num,
     trap_traj_.axis_ = this;
     min_endstop_.axis_ = this;
     max_endstop_.axis_ = this;
+    mechanical_brake_.axis_ = this;
     decode_step_dir_pins();
     watchdog_feed();
 }
@@ -462,6 +465,7 @@ bool Axis::run_idle_loop() {
     // run_control_loop ignores missed modulation timing updates
     // if and only if we're in AXIS_STATE_IDLE
     safety_critical_disarm_motor_pwm(motor_);
+    mechanical_brake_.engage();
     set_step_dir_active(config_.enable_step_dir && config_.step_dir_always_on);
     run_control_loop([this]() {
         return true;
@@ -474,6 +478,7 @@ void Axis::run_state_machine_loop() {
 
     // arm!
     motor_.arm();
+    mechanical_brake_.release();
 
     for (;;) {
         // Load the task chain if a specific request is pending
@@ -575,6 +580,7 @@ void Axis::run_state_machine_loop() {
             case AXIS_STATE_IDLE: {
                 run_idle_loop();
                 status = motor_.arm(); // done with idling - try to arm the motor
+                mechanical_brake_.release();
             } break;
 
             default:
diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp
index 3271e432..e96ab2ef 100644
--- a/Firmware/MotorControl/axis.hpp
+++ b/Firmware/MotorControl/axis.hpp
@@ -82,7 +82,8 @@ public:
             Motor& motor,
             TrapezoidalTrajectory& trap,
             Endstop& min_endstop,
-            Endstop& max_endstop);
+            Endstop& max_endstop,
+            MechanicalBrake& mechanical_brake);
 
     void setup();
     void start_thread();
@@ -210,6 +211,7 @@ public:
     TrapezoidalTrajectory& trap_traj_;
     Endstop& min_endstop_;
     Endstop& max_endstop_;
+    MechanicalBrake& mechanical_brake_;
 
     // List of current_limiters and thermistors to
     // provide easy iteration.
diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp
index 2809f5da..62f7ec1e 100644
--- a/Firmware/MotorControl/main.cpp
+++ b/Firmware/MotorControl/main.cpp
@@ -21,6 +21,7 @@ Axis::Config_t axis_configs[AXIS_COUNT];
 TrapezoidalTrajectory::Config_t trap_configs[AXIS_COUNT];
 Endstop::Config_t min_endstop_configs[AXIS_COUNT];
 Endstop::Config_t max_endstop_configs[AXIS_COUNT];
+MechanicalBrake::Config_t mechanical_brake_configs[AXIS_COUNT];
 
 std::array<Axis*, AXIS_COUNT> axes;
 ODriveCAN *odCAN = nullptr;
@@ -38,6 +39,7 @@ typedef Config<
     TrapezoidalTrajectory::Config_t[AXIS_COUNT],
     Endstop::Config_t[AXIS_COUNT],
     Endstop::Config_t[AXIS_COUNT],
+    MechanicalBrake::Config_t[AXIS_COUNT],
     Axis::Config_t[AXIS_COUNT]> ConfigFormat;
 
 void ODrive::save_configuration(void) {
@@ -53,6 +55,7 @@ void ODrive::save_configuration(void) {
             &trap_configs,
             &min_endstop_configs,
             &max_endstop_configs,
+            &mechanical_brake_configs,
             &axis_configs)) {
         printf("saving configuration failed\r\n"); osDelay(5);
     } else {
@@ -75,6 +78,7 @@ extern "C" int load_configuration(void) {
                 &trap_configs,
                 &min_endstop_configs,
                 &max_endstop_configs,
+                &mechanical_brake_configs,
                 &axis_configs)) {
         //If loading failed, restore defaults
         odrv.config_ = BoardConfig_t();
@@ -93,6 +97,7 @@ extern "C" int load_configuration(void) {
             Axis::load_default_can_id(i, axis_configs[i]);
             min_endstop_configs[i] = Endstop::Config_t();
             max_endstop_configs[i] = Endstop::Config_t();
+            mechanical_brake_configs[i] = MechanicalBrake::Config_t();
             controller_configs[i].load_encoder_axis = i;
         }
     } else {
@@ -194,9 +199,10 @@ extern "C" int construct_objects(){
         TrapezoidalTrajectory *trap = new TrapezoidalTrajectory(trap_configs[i]);
         Endstop *min_endstop = new Endstop(min_endstop_configs[i]);
         Endstop *max_endstop = new Endstop(max_endstop_configs[i]);
+        MechanicalBrake *mechanical_brake = new MechanicalBrake(mechanical_brake_configs[i]);
         axes[i] = new Axis(i, hw_configs[i].axis_config, axis_configs[i],
                 *encoder, *sensorless_estimator, *controller, *fet_thermistor,
-                *motor_thermistor, *motor, *trap, *min_endstop, *max_endstop);
+                *motor_thermistor, *motor, *trap, *min_endstop, *max_endstop, *mechanical_brake);
 
         controller_configs[i].parent = controller;
         encoder_configs[i].parent = encoder;
@@ -204,6 +210,7 @@ extern "C" int construct_objects(){
         motor_configs[i].parent = motor;
         min_endstop_configs[i].parent = min_endstop;
         max_endstop_configs[i].parent = max_endstop;
+        mechanical_brake_configs[i].parent = mechanical_brake;
         axis_configs[i].parent = axes[i];
     }
     return 0;
diff --git a/Firmware/MotorControl/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
new file mode 100644
index 00000000..27707d2a
--- /dev/null
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -0,0 +1,41 @@
+#include <odrive_main.h>
+
+MechanicalBrake::MechanicalBrake(MechanicalBrake::Config_t& config)
+    : config_(config) {
+    update_config();
+}
+
+bool MechanicalBrake::get_state() {
+    return mechanical_brake_state_;
+}
+
+void MechanicalBrake::update_config() {
+    set_enabled(config_.enabled);
+}
+
+void MechanicalBrake::engage() {
+    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_RESET : GPIO_PIN_SET );
+}
+
+void MechanicalBrake::release() {
+    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_SET : GPIO_PIN_RESET );
+}
+
+void MechanicalBrake::set_enabled(bool enable) {
+    if (config_.gpio_num != 0) {
+        uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
+        GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+        if (enable) {
+            HAL_GPIO_DeInit(gpio_port, gpio_pin);
+            GPIO_InitTypeDef GPIO_InitStruct;
+            GPIO_InitStruct.Pin = gpio_pin;
+            GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP;
+            GPIO_InitStruct.Pull = config_.pulldown ? GPIO_PULLDOWN : GPIO_PULLUP;
+            HAL_GPIO_Init(gpio_port, &GPIO_InitStruct);
+        }
+    }
+}
\ No newline at end of file
diff --git a/Firmware/MotorControl/mechanical_brake.hpp b/Firmware/MotorControl/mechanical_brake.hpp
new file mode 100644
index 00000000..9586f8b3
--- /dev/null
+++ b/Firmware/MotorControl/mechanical_brake.hpp
@@ -0,0 +1,36 @@
+#ifndef __MECHANICAL_BRAKE_HPP
+#define __MECHANICAL_BRAKE_HPP
+
+class MechanicalBrake {
+   public:
+    struct Config_t {
+        uint16_t gpio_num = 0;
+        bool enabled = false;
+        bool is_active_low = true;
+        bool pulldown = true;
+
+        // custom setters
+        MechanicalBrake* parent = nullptr;
+        void set_gpio_num(uint16_t value) { gpio_num = value; parent->update_config(); }
+        void set_enabled(uint32_t value) { enabled = value; parent->update_config(); }
+    };
+
+    explicit MechanicalBrake(MechanicalBrake::Config_t& config);
+
+    MechanicalBrake::Config_t& config_;
+    Axis* axis_ = nullptr;
+
+    void update_config();
+    void set_enabled(bool enabled);
+
+    void update();
+    bool get_state();
+    void release();
+    void engage();
+
+    bool mechanical_brake_state_ = true;
+
+   private:
+    bool pin_state_ = false;
+};
+#endif // __MECHANICAL_BRAKE_HPP
\ No newline at end of file
diff --git a/Firmware/MotorControl/odrive_main.h b/Firmware/MotorControl/odrive_main.h
index d0dcface..88328993 100644
--- a/Firmware/MotorControl/odrive_main.h
+++ b/Firmware/MotorControl/odrive_main.h
@@ -210,6 +210,7 @@ enum TimingLog_t {
 #include <thermistor.hpp>
 #include <trapTraj.hpp>
 #include <endstop.hpp>
+#include <mechanical_brake.hpp>
 #include <axis.hpp>
 #include <communication/communication.h>
 
diff --git a/Firmware/Tupfile.lua b/Firmware/Tupfile.lua
index 7f5380ae..f8f6f8ec 100644
--- a/Firmware/Tupfile.lua
+++ b/Firmware/Tupfile.lua
@@ -194,6 +194,7 @@ build{
         'MotorControl/thermistor.cpp',
         'MotorControl/encoder.cpp',
         'MotorControl/endstop.cpp',
+        'MotorControl/mechanical_brake.cpp',
         'MotorControl/controller.cpp',
         'MotorControl/sensorless_estimator.cpp',
         'MotorControl/trapTraj.cpp',
diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml
index 2c9060eb..2a755275 100644
--- a/Firmware/odrive-interface.yaml
+++ b/Firmware/odrive-interface.yaml
@@ -364,6 +364,7 @@ interfaces:
       trap_traj: TrapezoidalTrajectory
       min_endstop: Endstop
       max_endstop: Endstop
+      mechanical_brake: MechanicalBrake
     functions:
       watchdog_feed:
         doc: Feed the watchdog to prevent watchdog timeouts.
@@ -830,7 +831,6 @@ interfaces:
           accel_limit: float32
           decel_limit: float32
 
-
   ODrive.Endstop:
     c_is_class: True
     attributes:
@@ -845,6 +845,16 @@ interfaces:
           pullup: bool
           debounce_ms: {type: uint32, c_setter: set_debounce_ms}
 
+  ODrive.MechanicalBrake:
+    c_is_class: True
+    attributes:
+      config:
+        c_is_class: False
+        attributes:
+          gpio_num: {type: uint16, c_setter: set_gpio_num}
+          enabled: {type: bool, c_setter: set_enabled}
+          is_active_low: bool
+          pulldown: bool
 
 valuetypes:
   ODrive.Can.Protocol:
diff --git a/docs/_data/index.yaml b/docs/_data/index.yaml
index 0ff47762..273029b6 100644
--- a/docs/_data/index.yaml
+++ b/docs/_data/index.yaml
@@ -17,6 +17,8 @@ sections:
         url: /encoders
       - title: Homing & Endstops
         url: /endstops
+      - title: Mechanical Brakes
+        url: /mechanical-brakes
       - title: Thermistors
         url: /thermistors
       - title: Control & Tuning
diff --git a/docs/mechanical-brakes.md b/docs/mechanical-brakes.md
new file mode 100644
index 00000000..dc2cdaf6
--- /dev/null
+++ b/docs/mechanical-brakes.md
@@ -0,0 +1,66 @@
+# Mechanical Brake
+
+Some systems employ mechanical brakes on motors as a safety feature. These brakes can also be engaged as a power-saving function if the motor is not moving, but still under load.
+
+ODrive supports the use of its GPIO pins to connect to external brake drive electronics.
+
+When the ODrive engages the drive electronics, the brake will be disabled. When the drive enters a fault or idle state, the brake will be re-engaged.
+
+---
+
+## Mechanical Brake Configuration
+Each axis supports one mechanical brake. The following properties are accessible through `odrivetool`:
+
+Name |  Type | Default
+--- | -- | -- 
+gpio_num | int | 0
+enabled | boolean | false
+is_active_low | boolean | true
+pulldown | boolean | true
+
+### gpio_num
+The GPIO pin number, according to the silkscreen labels on ODrive. Set with these commands:
+```
+<odrv>.<axis>.mechanical_brake.config.gpio_num = <1, 2, 3, 4, 5, 6, 7, 8>
+```
+
+### enabled
+Enables/disables the operation of the mechanical brake.
+```
+<odrv>.<axis>.mechanical_brake.config.enabled = <True, False>
+```
+
+### is_active_low
+Most safety braking systems are active low, e.g. when the power is off, the brake is on. If the system uses brake drive electronics which use active high logic, flip this bit then reconsider the safety implications of your design...
+
+### pulldown
+If `true`, it enables the GPIO pulldown resistor.  If `false`, it enables the GPIO pullup resistor.
+
+### Example
+
+Let's say we're hacking away on an old ABB robotic arm. We've wired a 24V brake drive circuit triggered by GPIO5. When GPIO5 is driven, it will release the brakes on the axis we're moving.
+
+```
+<odrv>.<axis>.mechanical_brake.config.gpio_num = 5
+<odrv>.<axis>.mechanical_brake.config.enabled = True
+```
+
+Don't forget to save and reboot:
+```
+<odrv>.save_configuration()
+<odrv>.reboot()
+```
+
+### Testing The Mechanical Brakes
+Depending on your system this could be a dangerous experiment. Ensure that you have taken all necessary precautions to confirm if the wrong brake were inadvertently released it would not lead to injury or damage to equipment.
+
+```
+<odrv>.<axis>.mechanical_brake.release()
+```
+Note: If a brake is enabled, it will be engaged/disengaged during the next state machine step. 
+
+After you're satisfied with the testing, you can re-enable the brake using the command
+
+```
+<odrv>.<axis>.mechanical_brake.engage()
+```

From ccf4b17f822007c1ad552851a674082e8a7f59e0 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Wed, 19 Aug 2020 00:15:19 -0700
Subject: [PATCH 03/12] Updated changelog

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31adaf34..50da5f3f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased Features
 Please add a note of your changes below this heading if you make a Pull Request.
+### Added
+* [Mechanical brake support](docs/mechanical-brakes.md)
 
 # Release Candidate
 ## [0.5.1] - Date TBD

From 87ea4a423fa50e9f5a564eab960211b3897366b1 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Wed, 19 Aug 2020 12:31:26 -0700
Subject: [PATCH 04/12] Updated mechanical brake support as per upstream
 refactor

---
 Firmware/Board/v3/board.cpp                |  3 +++
 Firmware/MotorControl/axis.hpp             |  1 +
 Firmware/MotorControl/mechanical_brake.cpp | 30 +++++-----------------
 Firmware/MotorControl/mechanical_brake.hpp | 18 ++++---------
 4 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/Firmware/Board/v3/board.cpp b/Firmware/Board/v3/board.cpp
index 5f883809..d2b0afbb 100644
--- a/Firmware/Board/v3/board.cpp
+++ b/Firmware/Board/v3/board.cpp
@@ -96,6 +96,7 @@ Encoder encoders[AXIS_COUNT] = {
 
 // TODO: this has no hardware dependency and should be allocated depending on config
 Endstop endstops[2 * AXIS_COUNT];
+MechanicalBrake mechanical_brakes[AXIS_COUNT];
 
 SensorlessEstimator sensorless_estimators[AXIS_COUNT];
 Controller controllers[AXIS_COUNT];
@@ -116,6 +117,7 @@ std::array<Axis, AXIS_COUNT> axes{{
         motors[0], // motor
         trap[0], // trap
         endstops[0], endstops[1], // min_endstop, max_endstop
+        mechanical_brakes[0], // mechanical brake
     },
     {
         1, // axis_num
@@ -135,6 +137,7 @@ std::array<Axis, AXIS_COUNT> axes{{
         motors[1], // motor
         trap[1], // trap
         endstops[2], endstops[3], // min_endstop, max_endstop
+        mechanical_brakes[1], // mechanical brake
     },
 }};
 
diff --git a/Firmware/MotorControl/axis.hpp b/Firmware/MotorControl/axis.hpp
index b1611b93..b3965b39 100644
--- a/Firmware/MotorControl/axis.hpp
+++ b/Firmware/MotorControl/axis.hpp
@@ -8,6 +8,7 @@ class Axis;
 #include "controller.hpp"
 #include "trapTraj.hpp"
 #include "endstop.hpp"
+#include "mechanical_brake.hpp"
 #include "low_level.h"
 #include "utils.hpp"
 #include "communication/interface_uart.h" // TODO: remove once uart_poll() is gone
diff --git a/Firmware/MotorControl/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
index 27707d2a..591efc62 100644
--- a/Firmware/MotorControl/mechanical_brake.cpp
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -1,41 +1,23 @@
 #include <odrive_main.h>
 
-MechanicalBrake::MechanicalBrake(MechanicalBrake::Config_t& config)
-    : config_(config) {
-    update_config();
-}
-
-bool MechanicalBrake::get_state() {
-    return mechanical_brake_state_;
-}
-
-void MechanicalBrake::update_config() {
+bool MechanicalBrake::apply_config() {
     set_enabled(config_.enabled);
+    return true;
 }
 
 void MechanicalBrake::engage() {
-    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
-    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
-    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_RESET : GPIO_PIN_SET );
+    get_gpio(config_.gpio_num).write(config_.is_active_low ? 0 : 1);
 }
 
 void MechanicalBrake::release() {
-    uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
-    GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
-    HAL_GPIO_WritePin(gpio_port, gpio_pin, config_.is_active_low ? GPIO_PIN_SET : GPIO_PIN_RESET );
+    get_gpio(config_.gpio_num).write(config_.is_active_low ? 1 : 0);
 }
 
 void MechanicalBrake::set_enabled(bool enable) {
     if (config_.gpio_num != 0) {
-        uint16_t gpio_pin = get_gpio_pin_by_pin(config_.gpio_num);
-        GPIO_TypeDef* gpio_port = get_gpio_port_by_pin(config_.gpio_num);
+        Stm32Gpio gpio = get_gpio(config_.gpio_num);
         if (enable) {
-            HAL_GPIO_DeInit(gpio_port, gpio_pin);
-            GPIO_InitTypeDef GPIO_InitStruct;
-            GPIO_InitStruct.Pin = gpio_pin;
-            GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP;
-            GPIO_InitStruct.Pull = config_.pulldown ? GPIO_PULLDOWN : GPIO_PULLUP;
-            HAL_GPIO_Init(gpio_port, &GPIO_InitStruct);
+            gpio.config(GPIO_MODE_OUTPUT_PP, config_.pulldown ? GPIO_PULLDOWN : GPIO_PULLUP);
         }
     }
 }
\ No newline at end of file
diff --git a/Firmware/MotorControl/mechanical_brake.hpp b/Firmware/MotorControl/mechanical_brake.hpp
index 9586f8b3..240e7ed4 100644
--- a/Firmware/MotorControl/mechanical_brake.hpp
+++ b/Firmware/MotorControl/mechanical_brake.hpp
@@ -11,26 +11,18 @@ class MechanicalBrake {
 
         // custom setters
         MechanicalBrake* parent = nullptr;
-        void set_gpio_num(uint16_t value) { gpio_num = value; parent->update_config(); }
-        void set_enabled(uint32_t value) { enabled = value; parent->update_config(); }
+        void set_gpio_num(uint16_t value) { gpio_num = value; parent->apply_config(); }
+        void set_enabled(uint32_t value) { enabled = value; parent->apply_config(); }
     };
 
-    explicit MechanicalBrake(MechanicalBrake::Config_t& config);
+    MechanicalBrake() {}
 
-    MechanicalBrake::Config_t& config_;
+    MechanicalBrake::Config_t config_;
     Axis* axis_ = nullptr;
 
-    void update_config();
+    bool apply_config();
     void set_enabled(bool enabled);
-
-    void update();
-    bool get_state();
     void release();
     void engage();
-
-    bool mechanical_brake_state_ = true;
-
-   private:
-    bool pin_state_ = false;
 };
 #endif // __MECHANICAL_BRAKE_HPP
\ No newline at end of file

From 89bf879ff97eae995aceae9effd263a9096cb927 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Thu, 27 Aug 2020 18:28:33 -0700
Subject: [PATCH 05/12] Added GPIO_MODE_MECH_BRAKE, removed pull-ups for
 outputs, updated docs, added check for GPIO_MODE_MECH_BRAKE to startup in
 main

---
 Firmware/MotorControl/main.cpp             |  8 ++++-
 Firmware/MotorControl/mechanical_brake.cpp | 13 +++++---
 Firmware/MotorControl/mechanical_brake.hpp |  5 +--
 Firmware/odrive-interface.yaml             |  9 ++++-
 docs/interfaces.md                         | 38 +++++++++++-----------
 docs/mechanical-brakes.md                  |  4 ---
 tools/odrive/enums.py                      |  2 ++
 7 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp
index c1af09f4..225c01b1 100644
--- a/Firmware/MotorControl/main.cpp
+++ b/Firmware/MotorControl/main.cpp
@@ -411,7 +411,8 @@ extern "C" int main(void) {
         GPIO_InitStruct.Pin = get_gpio(i).pin_mask_;
 
         // Set Alternate Function setting for this GPIO mode
-        if (mode == ODriveIntf::GPIO_MODE_DIGITAL || mode == ODriveIntf::GPIO_MODE_ANALOG_IN) {
+        if (mode == ODriveIntf::GPIO_MODE_DIGITAL || mode == ODriveIntf::GPIO_MODE_ANALOG_IN
+            || mode == ODriveIntf::GPIO_MODE_MECH_BRAKE) {
             GPIO_InitStruct.Alternate = 0;
         } else {
             auto it = std::find_if(
@@ -497,6 +498,11 @@ extern "C" int main(void) {
                 GPIO_InitStruct.Pull = GPIO_NOPULL;
                 GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW;
             } break;
+            case ODriveIntf::GPIO_MODE_MECH_BRAKE: {
+                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/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
index 591efc62..b70334ff 100644
--- a/Firmware/MotorControl/mechanical_brake.cpp
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -1,6 +1,7 @@
 #include <odrive_main.h>
 
 bool MechanicalBrake::apply_config() {
+    config_.parent = this;
     set_enabled(config_.enabled);
     return true;
 }
@@ -14,10 +15,12 @@ void MechanicalBrake::release() {
 }
 
 void MechanicalBrake::set_enabled(bool enable) {
-    if (config_.gpio_num != 0) {
-        Stm32Gpio gpio = get_gpio(config_.gpio_num);
-        if (enable) {
-            gpio.config(GPIO_MODE_OUTPUT_PP, config_.pulldown ? GPIO_PULLDOWN : GPIO_PULLUP);
-        }
+    Stm32Gpio gpio = get_gpio(config_.gpio_num);
+
+    // We need this flag to alert the system to the configuration on boot
+    odrv.config_.gpio_modes[config_.gpio_num] = ODriveIntf::GPIO_MODE_MECH_BRAKE;
+
+    if (enable) {
+        gpio.config(GPIO_MODE_OUTPUT_PP, GPIO_NOPULL, GPIO_SPEED_FREQ_LOW);   
     }
 }
\ No newline at end of file
diff --git a/Firmware/MotorControl/mechanical_brake.hpp b/Firmware/MotorControl/mechanical_brake.hpp
index 240e7ed4..ae8373fd 100644
--- a/Firmware/MotorControl/mechanical_brake.hpp
+++ b/Firmware/MotorControl/mechanical_brake.hpp
@@ -1,13 +1,14 @@
 #ifndef __MECHANICAL_BRAKE_HPP
 #define __MECHANICAL_BRAKE_HPP
 
-class MechanicalBrake {
+#include <autogen/interfaces.hpp>
+
+class MechanicalBrake : public ODriveIntf::MechanicalBrakeIntf  {
    public:
     struct Config_t {
         uint16_t gpio_num = 0;
         bool enabled = false;
         bool is_active_low = true;
-        bool pulldown = true;
 
         // custom setters
         MechanicalBrake* parent = nullptr;
diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml
index 26491c4d..39108079 100644
--- a/Firmware/odrive-interface.yaml
+++ b/Firmware/odrive-interface.yaml
@@ -925,7 +925,13 @@ interfaces:
           gpio_num: {type: uint16, c_setter: set_gpio_num}
           enabled: {type: bool, c_setter: set_enabled}
           is_active_low: bool
-          pulldown: bool
+    functions:
+      engage:
+        doc: |
+          This function engages the mechanical brake if one is present and enabled.
+      release:
+        doc: |
+          This function releases the mecahncal brake if one is present and enabled.
 
 valuetypes:
   ODrive.GpioMode:
@@ -950,6 +956,7 @@ valuetypes:
       Enc0: {doc: The pin is used by quadrature encoder 0.}
       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.}
 
   ODrive.Can.Protocol:
     values: {Simple: }
diff --git a/docs/interfaces.md b/docs/interfaces.md
index 93297657..090948f4 100644
--- a/docs/interfaces.md
+++ b/docs/interfaces.md
@@ -18,25 +18,25 @@ The ODrive can be controlled over various ports and protocols. If you're comfort
 
 ## Pinout
 
-| #  | Label         | `GPIO_MODE_DIGITAL`    | `GPIO_MODE_ANALOG_IN` | `GPIO_MODE_UART0` | `GPIO_MODE_PWM0` | `GPIO_MODE_CAN0` | `GPIO_MODE_I2C0` | `GPIO_MODE_ENC0` | `GPIO_MODE_ENC1` |
-|----|---------------|------------------------|-----------------------|-------------------|------------------|------------------|------------------|------------------|------------------|
-|  0 | _not a pin_   |                        |                       |                   |                  |                  |                  |                  |                  |
-|  1 | GPIO1 (+)     | general purpose        | analog input          | **UART0.TX**      | PWM0.0           |                  |                  |                  |                  |
-|  2 | GPIO2 (+)     | general purpose        | analog input          | **UART0.RX**      | PWM0.1           |                  |                  |                  |                  |
-|  3 | GPIO3         | general purpose        | **analog input**      |                   | PWM0.2           |                  |                  |                  |                  |
-|  4 | GPIO4         | general purpose        | **analog input**      |                   | PWM0.3           |                  |                  |                  |                  |
-|  5 | GPIO5         | general purpose        | **analog input** (*)  |                   |                  |                  |                  |                  |                  |
-|  6 | GPIO6 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |
-|  7 | GPIO7 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |
-|  8 | GPIO8 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |
-|  9 | M0.A          | general purpose        |                       |                   |                  |                  |                  | **ENC0.A**       |                  |
-| 10 | M0.B          | general purpose        |                       |                   |                  |                  |                  | **ENC0.B**       |                  |
-| 11 | M0.Z          | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |
-| 12 | M1.A          | general purpose        |                       |                   |                  |                  | I2C.SCL          |                  | **ENC1.A**       |
-| 13 | M1.B          | general purpose        |                       |                   |                  |                  | I2C.SDA          |                  | **ENC1.B**       |
-| 14 | M1.Z          | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |
-| 15 | _not exposed_ | general purpose        |                       |                   |                  | **CAN0.RX**      | I2C.SCL          |                  |                  |
-| 16 | _not exposed_ | general purpose        |                       |                   |                  | **CAN0.TX**      | I2C.SDA          |                  |                  |
+| #  | Label         | `GPIO_MODE_DIGITAL`    | `GPIO_MODE_ANALOG_IN` | `GPIO_MODE_UART0` | `GPIO_MODE_PWM0` | `GPIO_MODE_CAN0` | `GPIO_MODE_I2C0` | `GPIO_MODE_ENC0` | `GPIO_MODE_ENC1` | `GPIO_MODE_MECH_BRAKE` |
+|----|---------------|------------------------|-----------------------|-------------------|------------------|------------------|------------------|------------------|------------------|------------------------|
+|  0 | _not a pin_   |                        |                       |                   |                  |                  |                  |                  |                  |                        |
+|  1 | GPIO1 (+)     | general purpose        | analog input          | **UART0.TX**      | PWM0.0           |                  |                  |                  |                  | mechanical brake       |
+|  2 | GPIO2 (+)     | general purpose        | analog input          | **UART0.RX**      | PWM0.1           |                  |                  |                  |                  | mechanical brake       |
+|  3 | GPIO3         | general purpose        | **analog input**      |                   | PWM0.2           |                  |                  |                  |                  | mechanical brake       |
+|  4 | GPIO4         | general purpose        | **analog input**      |                   | PWM0.3           |                  |                  |                  |                  | mechanical brake       |
+|  5 | GPIO5         | general purpose        | **analog input** (*)  |                   |                  |                  |                  |                  |                  | mechanical brake       |
+|  6 | GPIO6 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  | mechanical brake       |
+|  7 | GPIO7 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  | mechanical brake       |
+|  8 | GPIO8 (*) (+) | **general purpose**    |                       |                   |                  |                  |                  |                  |                  | mechanical brake       |
+|  9 | M0.A          | general purpose        |                       |                   |                  |                  |                  | **ENC0.A**       |                  |                        |
+| 10 | M0.B          | general purpose        |                       |                   |                  |                  |                  | **ENC0.B**       |                  |                        |
+| 11 | M0.Z          | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |                        |
+| 12 | M1.A          | general purpose        |                       |                   |                  |                  | I2C.SCL          |                  | **ENC1.A**       |                        |
+| 13 | M1.B          | general purpose        |                       |                   |                  |                  | I2C.SDA          |                  | **ENC1.B**       |                        |
+| 14 | M1.Z          | **general purpose**    |                       |                   |                  |                  |                  |                  |                  |                        |
+| 15 | _not exposed_ | general purpose        |                       |                   |                  | **CAN0.RX**      | I2C.SCL          |                  |                  |                        |
+| 16 | _not exposed_ | general purpose        |                       |                   |                  | **CAN0.TX**      | I2C.SDA          |                  |                  |                        |
 
 
 (*) ODrive v3.5 and later <br>
diff --git a/docs/mechanical-brakes.md b/docs/mechanical-brakes.md
index dc2cdaf6..ae3ce821 100644
--- a/docs/mechanical-brakes.md
+++ b/docs/mechanical-brakes.md
@@ -16,7 +16,6 @@ Name |  Type | Default
 gpio_num | int | 0
 enabled | boolean | false
 is_active_low | boolean | true
-pulldown | boolean | true
 
 ### gpio_num
 The GPIO pin number, according to the silkscreen labels on ODrive. Set with these commands:
@@ -33,9 +32,6 @@ Enables/disables the operation of the mechanical brake.
 ### is_active_low
 Most safety braking systems are active low, e.g. when the power is off, the brake is on. If the system uses brake drive electronics which use active high logic, flip this bit then reconsider the safety implications of your design...
 
-### pulldown
-If `true`, it enables the GPIO pulldown resistor.  If `false`, it enables the GPIO pullup resistor.
-
 ### Example
 
 Let's say we're hacking away on an old ABB robotic arm. We've wired a 24V brake drive circuit triggered by GPIO5. When GPIO5 is driven, it will release the brakes on the axis we're moving.
diff --git a/tools/odrive/enums.py b/tools/odrive/enums.py
index 17409599..8ca03c21 100644
--- a/tools/odrive/enums.py
+++ b/tools/odrive/enums.py
@@ -16,6 +16,7 @@ GPIO_MODE_PWM0                           = 8
 GPIO_MODE_ENC0                           = 9
 GPIO_MODE_ENC1                           = 10
 GPIO_MODE_ENC2                           = 11
+GPIO_MODE_MECH_BRAKE                     = 12
 
 # ODrive.Can.Protocol
 PROTOCOL_SIMPLE                          = 0
@@ -45,6 +46,7 @@ ENCODER_MODE_SINCOS                      = 2
 ENCODER_MODE_SPI_ABS_CUI                 = 256
 ENCODER_MODE_SPI_ABS_AMS                 = 257
 ENCODER_MODE_SPI_ABS_AEAT                = 258
+ENCODER_MODE_SPI_ABS_RLS                 = 259
 
 # ODrive.Controller.ControlMode
 CONTROL_MODE_VOLTAGE_CONTROL             = 0

From 801bda211e36da93255166f5a2b9fc77a7c0ea52 Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Thu, 27 Aug 2020 22:02:25 -0700
Subject: [PATCH 06/12] Changed the GPIO configuration for the mechanical brake
 to only take effect at boot

---
 Firmware/MotorControl/mechanical_brake.cpp | 10 +++-------
 docs/mechanical-brakes.md                  |  2 ++
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/Firmware/MotorControl/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
index b70334ff..21a98881 100644
--- a/Firmware/MotorControl/mechanical_brake.cpp
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -14,13 +14,9 @@ void MechanicalBrake::release() {
     get_gpio(config_.gpio_num).write(config_.is_active_low ? 1 : 0);
 }
 
-void MechanicalBrake::set_enabled(bool enable) {
-    Stm32Gpio gpio = get_gpio(config_.gpio_num);
-
-    // We need this flag to alert the system to the configuration on boot
-    odrv.config_.gpio_modes[config_.gpio_num] = ODriveIntf::GPIO_MODE_MECH_BRAKE;
-
+void MechanicalBrake::set_enabled(bool enable) { 
     if (enable) {
-        gpio.config(GPIO_MODE_OUTPUT_PP, GPIO_NOPULL, GPIO_SPEED_FREQ_LOW);   
+        // We need this flag to alert the system to the configuration on boot
+	    odrv.config_.gpio_modes[config_.gpio_num] = ODriveIntf::GPIO_MODE_MECH_BRAKE;
     }
 }
\ No newline at end of file
diff --git a/docs/mechanical-brakes.md b/docs/mechanical-brakes.md
index ae3ce821..3dde1e18 100644
--- a/docs/mechanical-brakes.md
+++ b/docs/mechanical-brakes.md
@@ -22,12 +22,14 @@ The GPIO pin number, according to the silkscreen labels on ODrive. Set with thes
 ```
 <odrv>.<axis>.mechanical_brake.config.gpio_num = <1, 2, 3, 4, 5, 6, 7, 8>
 ```
+After GPIO pin number is changed, you'll need to run `<odrv>.save_configuration()` and `<odrv>.reboot()` for changes to take effect.
 
 ### enabled
 Enables/disables the operation of the mechanical brake.
 ```
 <odrv>.<axis>.mechanical_brake.config.enabled = <True, False>
 ```
+After mechanical brake is enabled or disabled, you'll need to run `<odrv>.save_configuration()` and `<odrv>.reboot()` for changes to take effect.
 
 ### is_active_low
 Most safety braking systems are active low, e.g. when the power is off, the brake is on. If the system uses brake drive electronics which use active high logic, flip this bit then reconsider the safety implications of your design...

From f691536b3c2b90b7cedc8a852e5e715a05380d52 Mon Sep 17 00:00:00 2001
From: Unknown <pfguenette@gmail.com>
Date: Wed, 2 Sep 2020 20:44:56 -0400
Subject: [PATCH 07/12] Fix DRV lock-up problem when using absolute encoders

---
 Firmware/MotorControl/axis.cpp    |  5 ++---
 Firmware/MotorControl/encoder.hpp |  3 ++-
 Firmware/MotorControl/main.cpp    | 19 ++++++++++++++++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp
index 18b2d96e..310a7a8a 100644
--- a/Firmware/MotorControl/axis.cpp
+++ b/Firmware/MotorControl/axis.cpp
@@ -85,10 +85,9 @@ static void step_cb_wrapper(void* ctx) {
 }
 
 
-// @brief Sets up all components of the axis,
-// such as gate driver and encoder hardware.
+// @brief Does Nothing
 void Axis::setup() {
-    motor_.setup();
+    // Does nothing - Motor and encoder setup called separately.
 }
 
 static void run_state_machine_loop_wrapper(void* ctx) {
diff --git a/Firmware/MotorControl/encoder.hpp b/Firmware/MotorControl/encoder.hpp
index 15be6576..d100d168 100644
--- a/Firmware/MotorControl/encoder.hpp
+++ b/Firmware/MotorControl/encoder.hpp
@@ -5,9 +5,10 @@
 #error "This file should not be included directly. Include odrive_main.h instead."
 #endif
 
+
 class Encoder : public ODriveIntf::EncoderIntf {
 public:
-    const uint32_t MODE_FLAG_ABS = 0x100;
+    static constexpr uint32_t MODE_FLAG_ABS = 0x100;
 
     struct Config_t {
         Mode mode = MODE_INCREMENTAL;
diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp
index 2809f5da..78bc9b37 100644
--- a/Firmware/MotorControl/main.cpp
+++ b/Firmware/MotorControl/main.cpp
@@ -262,15 +262,28 @@ int odrive_main(void) {
     // must happen after communication is initialized
     pwm_in_init();
 
-    // Setup hardware for all components
-    for (size_t i = 0; i < AXIS_COUNT; ++i) {
-        axes[i]->setup();
+    // Set up the CS pins for absolute encoders
+    for(auto& axis : axes){
+        if(axis->encoder_.config_.mode & Encoder::MODE_FLAG_ABS){
+            axis->encoder_.abs_spi_cs_pin_init();
+        }
     }
 
+    // Setup motors (DRV8301 SPI transactions here)
+    for(auto& axis : axes){
+        axis->motor_.setup();
+    }
+
+    // Setup encoders (Starts encoder SPI transactions)
     for(auto& axis : axes){
         axis->encoder_.setup();
     }
 
+    // Setup anything remaining in each axis
+    for(auto& axis : axes){
+        axis->setup();
+    }
+
     // Start PWM and enable adc interrupts/callbacks
     start_adc_pwm();
 

From 0882c71beb1ecef858f4e021fe31852503c0dada Mon Sep 17 00:00:00 2001
From: Shaun Meehan <shaun@logiclow.com>
Date: Thu, 3 Sep 2020 18:59:14 -0700
Subject: [PATCH 08/12] Removed apply_config and enable functions, removed
 automatic gpio swap, fixed readme and odrive-interface to match

---
 Firmware/MotorControl/main.cpp             |  1 -
 Firmware/MotorControl/mechanical_brake.cpp | 21 ++++++---------------
 Firmware/MotorControl/mechanical_brake.hpp |  6 +-----
 Firmware/odrive-interface.yaml             |  1 -
 docs/mechanical-brakes.md                  | 19 ++++++++-----------
 5 files changed, 15 insertions(+), 33 deletions(-)

diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp
index 09838764..65609497 100644
--- a/Firmware/MotorControl/main.cpp
+++ b/Firmware/MotorControl/main.cpp
@@ -103,7 +103,6 @@ static bool config_apply_all() {
                && axes[i].controller_.apply_config()
                && axes[i].min_endstop_.apply_config()
                && axes[i].max_endstop_.apply_config()
-               && axes[i].mechanical_brake_.apply_config()
                && motors[i].apply_config()
                && axes[i].apply_config();
     }
diff --git a/Firmware/MotorControl/mechanical_brake.cpp b/Firmware/MotorControl/mechanical_brake.cpp
index 21a98881..c9d3602f 100644
--- a/Firmware/MotorControl/mechanical_brake.cpp
+++ b/Firmware/MotorControl/mechanical_brake.cpp
@@ -1,22 +1,13 @@
 #include <odrive_main.h>
 
-bool MechanicalBrake::apply_config() {
-    config_.parent = this;
-    set_enabled(config_.enabled);
-    return true;
-}
-
 void MechanicalBrake::engage() {
-    get_gpio(config_.gpio_num).write(config_.is_active_low ? 0 : 1);
+	if (odrv.config_.gpio_modes[config_.gpio_num] == ODriveIntf::GPIO_MODE_MECH_BRAKE){
+		get_gpio(config_.gpio_num).write(config_.is_active_low ? 0 : 1);
+	}
 }
 
 void MechanicalBrake::release() {
-    get_gpio(config_.gpio_num).write(config_.is_active_low ? 1 : 0);
+	if (odrv.config_.gpio_modes[config_.gpio_num] == ODriveIntf::GPIO_MODE_MECH_BRAKE){
+		get_gpio(config_.gpio_num).write(config_.is_active_low ? 1 : 0);
+	}
 }
-
-void MechanicalBrake::set_enabled(bool enable) { 
-    if (enable) {
-        // We need this flag to alert the system to the configuration on boot
-	    odrv.config_.gpio_modes[config_.gpio_num] = ODriveIntf::GPIO_MODE_MECH_BRAKE;
-    }
-}
\ No newline at end of file
diff --git a/Firmware/MotorControl/mechanical_brake.hpp b/Firmware/MotorControl/mechanical_brake.hpp
index ae8373fd..26362ce5 100644
--- a/Firmware/MotorControl/mechanical_brake.hpp
+++ b/Firmware/MotorControl/mechanical_brake.hpp
@@ -7,13 +7,11 @@ class MechanicalBrake : public ODriveIntf::MechanicalBrakeIntf  {
    public:
     struct Config_t {
         uint16_t gpio_num = 0;
-        bool enabled = false;
         bool is_active_low = true;
 
         // custom setters
         MechanicalBrake* parent = nullptr;
-        void set_gpio_num(uint16_t value) { gpio_num = value; parent->apply_config(); }
-        void set_enabled(uint32_t value) { enabled = value; parent->apply_config(); }
+        void set_gpio_num(uint16_t value) { gpio_num = value; }
     };
 
     MechanicalBrake() {}
@@ -21,8 +19,6 @@ class MechanicalBrake : public ODriveIntf::MechanicalBrakeIntf  {
     MechanicalBrake::Config_t config_;
     Axis* axis_ = nullptr;
 
-    bool apply_config();
-    void set_enabled(bool enabled);
     void release();
     void engage();
 };
diff --git a/Firmware/odrive-interface.yaml b/Firmware/odrive-interface.yaml
index 8c527d04..3d31d252 100644
--- a/Firmware/odrive-interface.yaml
+++ b/Firmware/odrive-interface.yaml
@@ -924,7 +924,6 @@ interfaces:
         c_is_class: False
         attributes:
           gpio_num: {type: uint16, c_setter: set_gpio_num}
-          enabled: {type: bool, c_setter: set_enabled}
           is_active_low: bool
     functions:
       engage:
diff --git a/docs/mechanical-brakes.md b/docs/mechanical-brakes.md
index 3dde1e18..ab2164e1 100644
--- a/docs/mechanical-brakes.md
+++ b/docs/mechanical-brakes.md
@@ -14,7 +14,6 @@ Each axis supports one mechanical brake. The following properties are accessible
 Name |  Type | Default
 --- | -- | -- 
 gpio_num | int | 0
-enabled | boolean | false
 is_active_low | boolean | true
 
 ### gpio_num
@@ -24,26 +23,24 @@ The GPIO pin number, according to the silkscreen labels on ODrive. Set with thes
 ```
 After GPIO pin number is changed, you'll need to run `<odrv>.save_configuration()` and `<odrv>.reboot()` for changes to take effect.
 
-### enabled
-Enables/disables the operation of the mechanical brake.
-```
-<odrv>.<axis>.mechanical_brake.config.enabled = <True, False>
-```
-After mechanical brake is enabled or disabled, you'll need to run `<odrv>.save_configuration()` and `<odrv>.reboot()` for changes to take effect.
-
 ### is_active_low
 Most safety braking systems are active low, e.g. when the power is off, the brake is on. If the system uses brake drive electronics which use active high logic, flip this bit then reconsider the safety implications of your design...
 
+### Enabling
+The configuration of the mechanical brake will enable the brake functionality. There's no need to specifically 'enable' this feature.
+
+
 ### Example
 
 Let's say we're hacking away on an old ABB robotic arm. We've wired a 24V brake drive circuit triggered by GPIO5. When GPIO5 is driven, it will release the brakes on the axis we're moving.
 
+We need to notify the axis of the GPIO number we've attached our brake to, and configure the ODrive pin mode to `GPIO_MODE_MECH_BRAKE`:
 ```
 <odrv>.<axis>.mechanical_brake.config.gpio_num = 5
-<odrv>.<axis>.mechanical_brake.config.enabled = True
+<odrv>.config.gpio5_mode = GPIO_MODE_MECH_BRAKE
 ```
 
-Don't forget to save and reboot:
+Pin configurations only take effect after a save/reboot so don't forget to run:
 ```
 <odrv>.save_configuration()
 <odrv>.reboot()
@@ -55,7 +52,7 @@ Depending on your system this could be a dangerous experiment. Ensure that you h
 ```
 <odrv>.<axis>.mechanical_brake.release()
 ```
-Note: If a brake is enabled, it will be engaged/disengaged during the next state machine step. 
+Note: If a brake is configured, it will be automatically engaged/disengaged during the next state machine step.
 
 After you're satisfied with the testing, you can re-enable the brake using the command
 

From d2b469eca0adb53cd9945dffff589df303df58b5 Mon Sep 17 00:00:00 2001
From: Samuel Sadok <samuel.sadok@bluewin.ch>
Date: Wed, 9 Sep 2020 09:14:55 +0200
Subject: [PATCH 09/12] add top level HWIL test script

---
 tools/odrive/tests/run_all_tests.sh | 30 +++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100755 tools/odrive/tests/run_all_tests.sh

diff --git a/tools/odrive/tests/run_all_tests.sh b/tools/odrive/tests/run_all_tests.sh
new file mode 100755
index 00000000..ab04a99c
--- /dev/null
+++ b/tools/odrive/tests/run_all_tests.sh
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+declare -a tests=('analog_input_test.py'
+                  'calibration_test.py'
+                  'can_test.py'
+                  'closed_loop_test.py'
+                  'encoder_test.py'
+                  'fibre_test.py'
+                  'integration_test.py'
+                  'nvm_test.py'
+                  'pwm_input_test.py'
+                  'step_dir_test.py'
+                  'uart_ascii_test.py'
+                  )
+summary=""
+
+for test in "${tests[@]}"; do
+    (ipython3 "$test" -- --test-rig-yaml ../../test-rig-rpi.yaml || true) | tee /tmp/odrivetest.log
+    if grep "All tests passed!" /tmp/odrivetest.log; then
+        summary="$summary - $test: passed"$'\n'
+    else
+        summary="$summary - $test: failed"$'\n'
+    fi
+
+    echo "########################"
+    echo "Current status:"
+    echo -n "$summary"
+    echo "########################"
+done

From 4c5761c120c20e82b399d43ff2e58b548b9f4a01 Mon Sep 17 00:00:00 2001
From: Unknown <pfguenette@gmail.com>
Date: Thu, 10 Sep 2020 19:31:29 -0400
Subject: [PATCH 10/12] Reset Ibus in reset_current_control

---
 Firmware/MotorControl/motor.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Firmware/MotorControl/motor.cpp b/Firmware/MotorControl/motor.cpp
index c7e50128..f22c46c2 100644
--- a/Firmware/MotorControl/motor.cpp
+++ b/Firmware/MotorControl/motor.cpp
@@ -51,6 +51,7 @@ void Motor::reset_current_control() {
     current_control_.v_current_control_integral_d = 0.0f;
     current_control_.v_current_control_integral_q = 0.0f;
     current_control_.acim_rotor_flux = 0.0f;
+    current_control_.Ibus = 0.0f;
 }
 
 // @brief Tune the current controller based on phase resistance and inductance

From b952a5e7545f6d39f9a46cfcbadb7f3f99901077 Mon Sep 17 00:00:00 2001
From: PAJohnson <pajohns4@ncsu.edu>
Date: Thu, 10 Sep 2020 19:48:36 -0400
Subject: [PATCH 11/12] Fix for high commanded currents during lockin ramp.

---
 Firmware/MotorControl/axis.cpp                 | 8 ++++----
 Firmware/MotorControl/sensorless_estimator.cpp | 6 ++++--
 Firmware/MotorControl/sensorless_estimator.hpp | 3 ++-
 docs/commands.md                               | 8 ++++----
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp
index 6a89e3c3..16b98bbe 100644
--- a/Firmware/MotorControl/axis.cpp
+++ b/Firmware/MotorControl/axis.cpp
@@ -238,9 +238,9 @@ bool Axis::run_lockin_spin(const LockinConfig_t &lockin_config) {
     float x = 0.0f;
     run_control_loop([&]() {
         float phase = wrap_pm_pi(lockin_config.ramp_distance * x);
-        float I_mag = lockin_config.current * x;
+        float torque = lockin_config.current * motor_.config_.torque_constant * x;
         x += current_meas_period / lockin_config.ramp_time;
-        if (!motor_.update(I_mag, phase, 0.0f))
+        if (!motor_.update(torque, phase, 0.0f))
             return false;
         return x < 1.0f;
     });
@@ -269,7 +269,7 @@ bool Axis::run_lockin_spin(const LockinConfig_t &lockin_config) {
         distance += vel * current_meas_period;
         phase = wrap_pm_pi(phase + vel * current_meas_period);
 
-        if (!motor_.update(lockin_config.current, phase, vel))
+        if (!motor_.update(lockin_config.current * motor_.config_.torque_constant, phase, vel))
             return false;
         return !spin_done(true); //vel_override to go to next phase
     });
@@ -285,7 +285,7 @@ bool Axis::run_lockin_spin(const LockinConfig_t &lockin_config) {
             distance += vel * current_meas_period;
             phase = wrap_pm_pi(phase + vel * current_meas_period);
 
-            if (!motor_.update(lockin_config.current, phase, vel))
+            if (!motor_.update(lockin_config.current * motor_.config_.torque_constant, phase, vel))
                 return false;
             return !spin_done();
         });
diff --git a/Firmware/MotorControl/sensorless_estimator.cpp b/Firmware/MotorControl/sensorless_estimator.cpp
index d70c7d2d..878b6ee7 100644
--- a/Firmware/MotorControl/sensorless_estimator.cpp
+++ b/Firmware/MotorControl/sensorless_estimator.cpp
@@ -66,13 +66,15 @@ bool SensorlessEstimator::update() {
     }
 
     // predict PLL phase with velocity
-    pll_pos_ = wrap_pm_pi(pll_pos_ + current_meas_period * vel_estimate_);
+    pll_pos_ = wrap_pm_pi(pll_pos_ + current_meas_period * vel_estimate_erad_);
     // update PLL phase with observer permanent magnet phase
     phase_ = fast_atan2(eta[1], eta[0]);
     float delta_phase = wrap_pm_pi(phase_ - pll_pos_);
     pll_pos_ = wrap_pm_pi(pll_pos_ + current_meas_period * pll_kp * delta_phase);
     // update PLL velocity
-    vel_estimate_ += current_meas_period * pll_ki * delta_phase;
+    vel_estimate_erad_ += current_meas_period * pll_ki * delta_phase;
+    // convert to mechanical turns/s for controller usage.
+    vel_estimate_ = vel_estimate_erad_ / (std::max((float)axis_->motor_.config_.pole_pairs, 1.0f) * 2.0f * M_PI);
 
     vel_estimate_valid_ = true;
     return true;
diff --git a/Firmware/MotorControl/sensorless_estimator.hpp b/Firmware/MotorControl/sensorless_estimator.hpp
index 9f59b28a..a0a6ec3c 100644
--- a/Firmware/MotorControl/sensorless_estimator.hpp
+++ b/Firmware/MotorControl/sensorless_estimator.hpp
@@ -18,7 +18,8 @@ public:
     Error error_ = ERROR_NONE;
     float phase_ = 0.0f;                        // [rad]
     float pll_pos_ = 0.0f;                      // [rad]
-    float vel_estimate_ = 0.0f;                      // [rad/s]
+    float vel_estimate_ = 0.0f;                      // [turn/s]
+    float vel_estimate_erad_ = 0.0f;                 // [rad/s]
     bool vel_estimate_valid_ = false;
     // float pll_kp_ = 0.0f;                       // [rad/s / rad]
     // float pll_ki_ = 0.0f;                       // [(rad/s^2) / rad]
diff --git a/docs/commands.md b/docs/commands.md
index 1b41b30f..a621d578 100644
--- a/docs/commands.md
+++ b/docs/commands.md
@@ -82,15 +82,15 @@ All variables that are part of a `[...].config` object can be saved to non-volat
 ## Setting up sensorless
 The ODrive can run without encoder/hall feedback, but there is a minimum speed, usually around a few hunderd RPM.
 
-To give an example, suppose you have a motor with 7 pole pairs, and you want to spin it at 3000 RPM. Then you would set the `input_vel` to `3000 * 2*pi/60 * 7 = 2199 rad/s electrical`.
-
-Below are some suggested starting parameters that you can use. Note that you _must_ set the `pm_flux_linkage` correctly for sensorless mode to work.
+Below are some suggested starting parameters that you can use. Note that you _must_ set the `pm_flux_linkage` correctly for sensorless mode to work. Motor calibration and setup must also be completed before sensorless mode will work.
 
 ```
 odrv0.axis0.controller.config.vel_gain = 0.01
 odrv0.axis0.controller.config.vel_integrator_gain = 0.05
 odrv0.axis0.controller.config.control_mode = 2
-odrv0.axis0.controller.input_vel = 400
+odrv0.axis0.controller.input_vel = 10
+odrv0.axis0.controller.config.vel_limit = <a value greater than input_vel>
+odrv0.axis0.motor.config.current_lim = 2 * odrv0.axis0.config.sensorless_ramp.current
 odrv0.axis0.motor.config.direction = 1
 odrv0.axis0.sensorless_estimator.config.pm_flux_linkage = 5.51328895422 / (<pole pairs> * <motor kv>)
 ```

From b089573feaeaa3ab51e56f02d1cf895b353b85a4 Mon Sep 17 00:00:00 2001
From: PAJohnson <pajohns4@ncsu.edu>
Date: Wed, 16 Sep 2020 22:42:37 -0400
Subject: [PATCH 12/12] Fix for issue where using sensorless mode with input
 mode VEL_RAMP would cause the motor to try and reach a very high speed at the
 end of the sensorless ramp

---
 CHANGELOG.md                   | 3 +++
 Firmware/MotorControl/axis.cpp | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 12b17c06..3a6c0c76 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,9 @@ Please add a note of your changes below this heading if you make a Pull Request.
 * `axis.motor.thermal_current_lim` has been removed. Instead a new property is available `axis.motor.effective_current_lim` which contains the effective current limit including any thermal limits.
 * `axis.motor.get_inverter_temp()`, `axis.motor.inverter_temp_limit_lower` and `axis.motor.inverter_temp_limit_upper` have been moved to seperate fet thermistor object under `axis.fet_thermistor`. `get_inverter_temp()` function has been renamed to `temp` and is now a read-only property.
 * `axis.config.counts_per_step` is now `axis.config.turns_per_step`
+* Outputs of `axis.sensorless_estimator` are now in turns/s instead of electrical rad/s
+* Fixed bug of high current during lockin-ramp caused by `motor::update()` expecting a torque command instead of current
+* Fixed bug where commanded velocity was extremely high just after sensorless ramp when using `input_mode` INPUT_MODE_VEL_RAMP caused by `vel_setpoint` and `axis.config.sensorless_ramp.vel` being in different units
 
 
 # Releases
diff --git a/Firmware/MotorControl/axis.cpp b/Firmware/MotorControl/axis.cpp
index 16b98bbe..52150800 100644
--- a/Firmware/MotorControl/axis.cpp
+++ b/Firmware/MotorControl/axis.cpp
@@ -561,7 +561,7 @@ void Axis::run_state_machine_loop() {
                 if (status) {
                     // call to controller.reset() that happend when arming means that vel_setpoint
                     // is zeroed. So we make the setpoint the spinup target for smooth transition.
-                    controller_.vel_setpoint_ = config_.sensorless_ramp.vel;
+                    controller_.vel_setpoint_ = config_.sensorless_ramp.vel / (2.0f * M_PI * motor_.config_.pole_pairs);
                     status = run_sensorless_control_loop();
                 }
             } break;