feat: can reconnect

[uhobeike]

Hello,
I’ve been playing around with the BotWheel Explorer I purchased.

There’s something I noticed—when the emergency stop switch is pressed, the CAN connection gets cut off.
As a result, even if I send speed commands to the motors, there’s no response, which I found a bit frustrating. So I modified the code so that the motors will still respond to speed commands even after the CAN connection is reestablished.

You can choose whether or not to use this feature through a parameter setting.
<param name="can_reconnect">true</param>

I think this feature is quite useful.
I’ve confirmed that it works with the BotWheel Explorer!
If you find any issues, please let me know.

[samuelsadok]

The notion of "reconnect" is somewhat unfitting here:

• ODrive's "CANSimple" protocol isn't connection-based in the strict sense. You can observe RX messages to infer presence of the device (the watchdog on the ODrive side does this), however checking the result of send() does not give the correct information. The fact that it worked in your experiments is probably because the interface entered BUS-OFF state because it was the only remaining interface on the bus, but if you add more devices or use different interface settings, this check would fail.
• The reaction in on_can_reconnect() is not that it reconnects (as can_reconnect=true would suggest), but rather it re-enables CLOSED_LOOP_CONTROL.

So think what you're actually looking for is an "auto-retry / auto-reenable" feature that periodically checks the current axis state, and if it mismatches the desired axis state, it resends the state request. This will work with any type of error, not just CAN interruptions.

I can see two approaches:

• Exposing the axis state/error flags to the higher level controller, so that the user's ROS node can decide what to do in which situation.
• Inside the ROS control interface. You could do this on a timer, on write() or upon receiving a heartbeat. However in all cases beware of race conditions: if you send a state request at the same time as the ODrive sends a heartbeat, you might think the previous request didn't go through even though it actually did. If the first state request results in an error, you might miss the error status by sending a second request too quickly.

[samuelsadok]

The content of this if block is identical to part of ODriveHardwareInterface::perform_command_mode_switch() as far as I can tell. It should therefore be refactored into its own function(s).

All things considered, I think two separate functions make sense:

void ODriveHardwareInterface::request_closed_loop_control() {
    // send Set_Controller_Mode_msg_t, Clear_Errors_msg_t and Set_Axis_State_msg_t
}

void ODriveHardwareInterface::request_idle() {
    // send single Set_Axis_State_msg_t message
}

[samuelsadok]

The if-condition needs to be revised:

• !(axis.axis_state_ == 8) means "re-enable whenever the state is something other than CLOSED_LOOP_CONTROL". I prefer the more conservative check axis.axis_state_ == 0 which means "re-enable only when the state is IDLE". So during other states (calibration), the ROS plugin doesn't interfere.
• The error check as it is !(axis.axis_error_ == 0) is somewhat redundant with the state check. However I think it would be nice to introduce a configurable filter such that the ROS interface only re-enables if the error is signed off by the developer.
• axis.procedure_result_ needs to be ignored, it is only reported as ProcedureResult.SUCCESS (0) due to an oversight in the firmware. It will be ProcedureResult.BUSY (1) in future firmware versions (but we can't rely on that here either because it would break backwards compatibility)
• axis.trajectory_done_flag_ needs to be ignored, it is undefined for input modes other than InputMode.TRAP_TRAJ. (It happens to default to true if TRAP_TRAJ has not been used since last reboot)
• If we allow auto-enabling like that, we should also allow auto-disabling.

So in the end it becomes something like:

bool should_be_enabled = // same expression as any_enabled
if (should_be_enabled && axis.axis_state_ == 0 && (axis.axis_error_ & user_ignored_errors_) == 0) {
    request_closed_loop_control();
} else if (!should_be_enabled && axis.axis_state_ == 8) {
    request_idle();
}

[CLAassistant]

All committers have signed the CLA.

[uhobeike]

@samuelsadok
Thank you for your kind remarks and suggestions.
They are very helpful.

I will try the code you suggested during December.