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

adding IMU data in AP_DDS library for high frequency raw imu data transmission #26187

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

Astik-2002
Copy link
Contributor

@Astik-2002 Astik-2002 commented Feb 11, 2024

This code enables imu data transmission between ros2 nodes and AP via DDS. @Ryanf55 please review this and suggest changes.

Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. Looks like you followed the pattern well and populated the covariance in accordance with the docs.

libraries/AP_DDS/dds_xrce_profile.xml Show resolved Hide resolved
libraries/AP_DDS/AP_DDS_Client.cpp Outdated Show resolved Hide resolved
libraries/AC_CustomControl/AC_CustomControl.cpp Outdated Show resolved Hide resolved
@tridge
Copy link
Contributor

tridge commented Feb 19, 2024

the commit list needs needs a big cleanup

tridge
tridge previously requested changes Feb 19, 2024
libraries/AP_DDS/AP_DDS_Client.cpp Outdated Show resolved Hide resolved
libraries/AP_DDS/AP_DDS_Client.cpp Outdated Show resolved Hide resolved
libraries/AP_DDS/AP_DDS_Client.cpp Show resolved Hide resolved
libraries/AP_DDS/Idl/sensor_msgs/msg/Imu.idl Show resolved Hide resolved
@tridge
Copy link
Contributor

tridge commented Feb 20, 2024

note that this will get the 1st IMU, if you want the active IMU then you need to call AP_AHRS::_get_primary_gyro_index(), otherwise if you get a lane change then you will get noisy data

libraries/AP_DDS/dds_xrce_profile.xml Outdated Show resolved Hide resolved
Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

Getting close! Just a few fixes and it should be good to go.

Here's the data it captured on my system.

---
header:
  stamp:
    sec: 7
    nanosec: 632779000
  frame_id: base_link_ned
orientation:
  x: 0.9851087927818298
  y: 6.668890273431316e-05
  z: -1.8647700926521793e-05
  w: -0.17193196713924408
orientation_covariance:
- -1.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
angular_velocity:
  x: 5.1383547543082386e-05
  y: 0.0001419413456460461
  z: 5.413950566435233e-05
angular_velocity_covariance:
- -1.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
linear_acceleration:
  x: -0.0013561300002038479
  y: -0.001497147954069078
  z: -9.81856632232666
linear_acceleration_covariance:
- -1.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
- 0.0
---

libraries/AP_DDS/AP_DDS_Client.cpp Show resolved Hide resolved
libraries/AP_DDS/AP_DDS_Client.cpp Show resolved Hide resolved
libraries/AP_DDS/dds_xrce_profile.xml Outdated Show resolved Hide resolved
* Using NED frame
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Feb 26, 2024

Looks great.

I cleaned up your commits with
git rebase upstream/master
Then, I squashed them together.
And finally, I edited the commit message to use the AP_DDS prefix.

@Ryanf55 Ryanf55 requested a review from tridge February 26, 2024 17:32
@tridge tridge dismissed their stale review March 2, 2024 20:04

Ryan approved

@tridge tridge merged commit 24de88f into ArduPilot:master Mar 2, 2024
92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants