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.

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

Roboclaw ESC driver #22215

Merged
merged 27 commits into from
Nov 28, 2023
Merged

Roboclaw ESC driver #22215

merged 27 commits into from
Nov 28, 2023

Conversation

PerFrivik
Copy link
Contributor

@PerFrivik PerFrivik commented Oct 12, 2023

Solved Problem

Refined the Roboclaw driver for the R1 Aion Robotics Rover with the RoboClaw 2x15A motor controller.
This update:

  • Corrects a previously hardcoded serial message size for clarity and functionality.
  • Fixes an issue with the driver not reconnecting after a robot power cycle.
  • Enables the driver to publish encoder data.
  • Provides more transparent error handling.

Solution

Switched Control Mode for Wheels:

Transitioned from duty cycle control to velocity control for the rover's wheels. This change enabled the driver to access and read the encoder data. It's noteworthy to mention that, for reasons yet to be determined, the duty cycle control did not support concurrent encoder data reading.

Introduction of RoboClawError Enum:

Implemented a RoboClawError enum class which provides defined messages for specific, predefined errors, enhancing the clarity of error feedback.

Clarified Hardcoded Message Size for Roboclaw Command:

To make the hardcoded message size more transparent and understandable, the following definition was established:

#define CMD_READ_STATUS_MESSAGE_SIZE 6

Publishing Encoder Data:

The system has been updated to publish encoder data that includes:

  • Speed
  • Position
  • Timestamp

Direct Subscription to Control Topics:

The current implementation bypasses the control allocation and directly subscribes to two specific control topics:

  • vehicle_thrust_setpoint
  • vehicle_torque_setpoint

The motor control gets converted within the code. This approach and control allocation skip is a temporary measure.

Rapid Startup Connection Failure

The initialization sequence of the Roboclaw driver has been adjusted. Now, if the initial connection attempt is unsuccessful after a power cycle, the driver will automatically retry the connection, ensuring consistent and reliable connectivity.

Changelog Entry

For release notes:

New driver: Support Roboclaw ESC serial communication e.g. for Aion Robotics R1 Rover

Context

https://youtu.be/qvCw9TRRALQ

@PerFrivik PerFrivik requested a review from MaEtUgR October 12, 2023 14:43
@PerFrivik PerFrivik self-assigned this Oct 12, 2023
@PerFrivik
Copy link
Contributor Author

I just noticed that I have some unwanted changes in RoverPositionControl.cpp that are not supposed to be there. Will fix.

msg/CMakeLists.txt Outdated Show resolved Hide resolved
@MaEtUgR MaEtUgR changed the title Roboclaw upstream Roboclaw ESC driver Oct 31, 2023
@junwoo091400 junwoo091400 added the Drivers 🔧 Sensors, Actuators, etc label Nov 3, 2023
@github-actions github-actions bot added the v3.0 label Nov 10, 2023
@mcsauder
Copy link
Contributor

Any way to figure out how many users of this driver there are? Maybe merge it and find out how many problems are left when they start using it? I know that is dangerous. ;)

@MaEtUgR
Copy link
Member

MaEtUgR commented Nov 13, 2023

Any way to figure out how many users of this driver there are?

Based on the fact it was broken before there can't be many users but we have the Aion R1 and it proves quite useful for rover testing. So we reworked the driver.

Maybe merge it and find out how many problems are left

Sure, we can still contribute fixes as we go. right now I'm reviewing and probably after a final test we can move on.

@MaEtUgR MaEtUgR force-pushed the roboclaw-upstream branch 3 times, most recently from 5bb2683 to d146eb1 Compare November 14, 2023 15:50
@MaEtUgR MaEtUgR marked this pull request as ready for review November 14, 2023 16:05
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thanks @PerFrivik for the tests and corrections! Now the directions make sense 👍
Can you remove the newlines and too specific parameters from the airframe again just keeping the ROBOCLAW_ ones?
And one last thing to check: Does the wheel already start turning with a command of 1 out of 127?

@PerFrivik
Copy link
Contributor Author

Thanks @PerFrivik for the tests and corrections! Now the directions make sense 👍 Can you remove the newlines and too specific parameters from the airframe again just keeping the ROBOCLAW_ ones? And one last thing to check: Does the wheel already start turning with a command of 1 out of 127?

Hey @MaEtUgR Thank you for reviewing it! I hope the changes are okay now. I checked and the wheel starts turning at values between 5-7/127.

@PerFrivik
Copy link
Contributor Author

Rebased on main

dagar
dagar previously approved these changes Nov 23, 2023
@dagar
Copy link
Member

dagar commented Nov 23, 2023

Minor CI failure here.

https://github.com/PX4/PX4-Autopilot/actions/runs/6929156822/job/18846330621?pr=22215

Otherwise I'm happy to bring this in to enable more testing.

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this. Let's iterate on the R1 config while testing the revised differential rover functionality.

@MaEtUgR MaEtUgR merged commit befbc19 into main Nov 28, 2023
89 of 90 checks passed
@MaEtUgR MaEtUgR deleted the roboclaw-upstream branch November 28, 2023 15:30
@PerFrivik PerFrivik mentioned this pull request Dec 15, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Drivers 🔧 Sensors, Actuators, etc
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants