Skip to content

AP_DDS: Local Velocity Publisher#23541

Merged
tridge merged 2 commits intoArduPilot:masterfrom
pedro-fuoco:dds-local-velocity-publisher
Apr 27, 2023
Merged

AP_DDS: Local Velocity Publisher#23541
tridge merged 2 commits intoArduPilot:masterfrom
pedro-fuoco:dds-local-velocity-publisher

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 force-pushed the dds-local-velocity-publisher branch from 7f7fb11 to 9492b85 Compare April 18, 2023 21:20
@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
Contributor

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

@pedro-fuoco pedro-fuoco force-pushed the dds-local-velocity-publisher branch from 9492b85 to 9a915a6 Compare April 20, 2023 16:18
@khancyr khancyr requested a review from Ryanf55 April 21, 2023 15:15
@khancyr khancyr added the ROS label Apr 21, 2023
@Ryanf55
Copy link
Contributor

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
Contributor

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 pedro-fuoco force-pushed the dds-local-velocity-publisher branch from 9a915a6 to d6ec51e Compare April 25, 2023 17:05
@pedro-fuoco pedro-fuoco force-pushed the dds-local-velocity-publisher branch from d6ec51e to 25fc39e Compare April 25, 2023 17:07
@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

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