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

AP_DDS: Local Velocity Publisher #23541

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

pedro-fuoco
Copy link
Contributor

@pedro-fuoco pedro-fuoco commented Apr 18, 2023

Resolves #23279
Here is the PR working in its current state in SITL:
Screenshot from 2023-04-18 18-21-45
It is currently following:

// In ROS REP 103, it follows this convention
// X - Forward
// Y - Left
// Z - Up
// https://www.ros.org/reps/rep-0103.html#axis-orientation

For linear velocity, the data is earth-fixed but starts alligned with the robot body (x-forward)
For angular velocity, the gyro data is body-fixed.

@pedro-fuoco pedro-fuoco marked this pull request as ready for review April 18, 2023 21:25
@pedro-fuoco
Copy link
Contributor Author

For linear velocity, the data is earth-fixed but starts alligned with the robot body (x-forward)
For angular velocity, the gyro data is body-fixed.

Should we keep it that way ? Or do everything acording to NED ? This is easy to implement but the AP community needs to agree on how to do it

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Apr 20, 2023

For linear velocity, the data is earth-fixed but starts alligned with the robot body (x-forward)
For angular velocity, the gyro data is body-fixed.

Should we keep it that way ? Or do everything acording to NED ? This is easy to implement but the AP community needs to agree on how to do it

As before, we want to follow ROS rep's as best as possible. In REP-147, it says, even for aerial vehicles, to follow REP-105.
https://ros.org/reps/rep-0147.html#coordinate-frames

That said, it doesn't clarify on NED vs ENU. For now, let's stick with ENU, since that's largely a convention in the ROS community, and this is a ROS interface.

I think it's totally reasonable, later, to add an alternative subset of topics in NED convention, if it's needed.

@pedro-fuoco
Copy link
Contributor Author

pedro-fuoco commented Apr 20, 2023

REP-103 solves the dispute between ENU vs NED:

For short-range Cartesian representations of geographic locations, use the east north up [5] (ENU) convention
For outdoor systems where it is desirable to work under the north east down [6] (NED) convention, define an appropriately transformed secondary frame with the "_ned" suffix

ENU should be default and we can add NED later, as you proposed. I will change the frame to ENU and commit it as a fixup

@khancyr khancyr requested a review from Ryanf55 April 21, 2023 15:15
@khancyr khancyr added the ROS label Apr 21, 2023
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Apr 21, 2023

We spent some time testing, just deciding what coordinate system the orientation should follow on the vehicle. Likely, we will match what NAV2 is using, but I had issues bringing the examples up in NAV2.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Apr 25, 2023

Please rebase this on master to fix conflicts with the pose topic. Then, I can test it. Thanks!

@pedro-fuoco
Copy link
Contributor Author

pedro-fuoco commented Apr 25, 2023

Rebased and updated the axis orientation to follow ENU and what was agreed on #23480. Code comments were updated accordingly.
@Ryanf55 I've tested it in SITL with a controller and both the linear and angular velocities seem to check out! Feel free to check it

@tridge tridge merged commit 8194310 into ArduPilot:master Apr 27, 2023
82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AP_DDS: Add support for geometry_msgs/TwistStamped velocity publishing from AP
4 participants