Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes three things Hangprinter needs #680

Merged
merged 5 commits into from
May 31, 2022
Merged

Fixes three things Hangprinter needs #680

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae262e4
Sets steps_ in CANSimple::set_input_pos_callback
tobbelobb Mar 21, 2022
d272fbe
Puts fmodf_pos back into the steps_ assignment
tobbelobb Mar 29, 2022
58a6567
Stands still upon switching to position mode via can
tobbelobb Apr 5, 2022
c5017f4
Making circular_setpoints optional in step/dir mode
tobbelobb May 4, 2022
fa144cf
Makes behavior consistent across interfaces
tobbelobb May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Please add a note of your changes below this heading if you make a Pull Request.
* Ensure endstops update before being checked for errors, to prevent [#625](https://github.com/odriverobotics/ODrive/issues/625)
* Reset trajectory_done_ during homing to ensure a new trajectory is actually computed [#634](https://github.com/odriverobotics/ODrive/issues/634)
* Use `input_xxx` as a DC offset in tuning mode
* Syncs `steps_` with input pos when set explicitly via CAN.

# Releases
## [0.5.4] - 2021-10-12
Expand Down
4 changes: 1 addition & 3 deletions Firmware/MotorControl/axis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,7 @@ bool Axis::start_closed_loop_control() {
return false;
} else {
controller_.pos_setpoint_ = *pos_init;
controller_.input_pos_ = *pos_init;
float range = controller_.config_.circular_setpoint_range;
steps_ = (int64_t)(fmodf_pos(*pos_init, range) / range * controller_.config_.steps_per_circular_range);
controller_.set_input_pos_and_steps(*pos_init);
}
}
controller_.input_pos_updated();
Expand Down
23 changes: 19 additions & 4 deletions Firmware/MotorControl/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ bool Controller::anticogging_calibration(float pos_estimate, float vel_estimate)
}
}

void Controller::set_input_pos_and_steps(float const pos) {
input_pos_ = pos;
if (config_.circular_setpoints) {
float const range = config_.circular_setpoint_range;
axis_->steps_ = (int64_t)(fmodf_pos(pos, range) / range * config_.steps_per_circular_range);
} else {
axis_->steps_ = (int64_t)(pos * config_.steps_per_circular_range);
}
}


void Controller::update_filter_gains() {
float bandwidth = std::min(config_.input_filter_bandwidth, 0.25f * current_meas_hz);
input_filter_ki_ = 2.0f * bandwidth; // basic conversion to discrete time
Expand All @@ -109,11 +120,15 @@ bool Controller::update() {
std::optional<float> anticogging_vel_estimate = axis_->encoder_.vel_estimate_.present();

if (axis_->step_dir_active_) {
if (!pos_wrap.has_value()) {
set_error(ERROR_INVALID_CIRCULAR_RANGE);
return false;
if (config_.circular_setpoints) {
if (!pos_wrap.has_value()) {
set_error(ERROR_INVALID_CIRCULAR_RANGE);
return false;
}
input_pos_ = (float)(axis_->steps_ % config_.steps_per_circular_range) * (*pos_wrap / (float)(config_.steps_per_circular_range));
} else {
input_pos_ = (float)(axis_->steps_) / (float)(config_.steps_per_circular_range);
}
input_pos_ = (float)(axis_->steps_ % config_.steps_per_circular_range) * (*pos_wrap / (float)(config_.steps_per_circular_range));
}

if (config_.anticogging.calib_anticogging) {
Expand Down
2 changes: 2 additions & 0 deletions Firmware/MotorControl/controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class Controller : public ODriveIntf::ControllerIntf {
void start_anticogging_calibration();
bool anticogging_calibration(float pos_estimate, float vel_estimate);

void set_input_pos_and_steps(float pos);

void update_filter_gains();
bool update();

Expand Down
17 changes: 15 additions & 2 deletions Firmware/communication/can/can_simple.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ bool CANSimple::get_encoder_count_callback(const Axis& axis) {
}

void CANSimple::set_input_pos_callback(Axis& axis, const can_Message_t& msg) {
axis.controller_.input_pos_ = can_getSignal<float>(msg, 0, 32, true);
axis.controller_.set_input_pos_and_steps(can_getSignal<float>(msg, 0, 32, true));
axis.controller_.input_vel_ = can_getSignal<int16_t>(msg, 32, 16, true, 0.001f, 0);
axis.controller_.input_torque_ = can_getSignal<int16_t>(msg, 48, 16, true, 0.001f, 0);
axis.controller_.input_pos_updated();
Expand All @@ -269,7 +269,20 @@ void CANSimple::set_input_torque_callback(Axis& axis, const can_Message_t& msg)
}

void CANSimple::set_controller_modes_callback(Axis& axis, const can_Message_t& msg) {
axis.controller_.config_.control_mode = static_cast<Controller::ControlMode>(can_getSignal<int32_t>(msg, 0, 32, true));

Controller::ControlMode const mode = static_cast<Controller::ControlMode>(can_getSignal<int32_t>(msg, 0, 32, true));
if (mode == Controller::CONTROL_MODE_POSITION_CONTROL) {
float estimate = 0.0f;
if (axis.controller_.config_.circular_setpoints) {
estimate = axis.encoder_.pos_circular_.any().value_or(0.0f);
} else {
estimate = axis.encoder_.pos_estimate_.any().value_or(0.0f);
}

axis.controller_.set_input_pos_and_steps(estimate);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reaction on writing the control mode is appropriate, but we need the behavior to be consistent when set from other sources than CAN. My suggestion is:

  1. Merge to a reusable function this snippet with the "// To avoid any transient on startup" section in axis.cpp start_closed_loop_control. You could call it set_setpoint_to_estimate.
  2. Add a c_setter tag to the control_mode endpoint in odrive-interface.yaml, so this can get executed when we write it on USB or UART.
  3. Add an entry in the changelog about this reaction on control mode change.


axis.controller_.config_.control_mode = static_cast<Controller::ControlMode>(mode);
axis.controller_.config_.input_mode = static_cast<Controller::InputMode>(can_getSignal<int32_t>(msg, 32, 32, true));
}

Expand Down