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: battery topic to report all the available batteries #28291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tizianofiorenzani
Copy link

@tizianofiorenzani tizianofiorenzani commented Oct 2, 2024

Issue

#28288

What Changed

  • Battery topic renamed to /ap/battery
  • All the detected batteries are reported to the same topic with different frame_id from '0' to '<AP_BATT_MONITOR_MAX_INSTANCES>'

Test

colcon test --packages-select ardupilot_dds_tests \
--event-handlers=console_cohesion+ --pytest-args -k test_battery_msg_received

colcon test-result --all --verbose

@tizianofiorenzani
Copy link
Author

Ops, I left the iostream version, need to fix that or won't build for arm...

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/README.md Outdated Show resolved Hide resolved
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Oct 3, 2024

Squash once you incorporate the other feedback. LGTM, nice work.

@Ryanf55 Ryanf55 added the ROS label Oct 3, 2024
@tizianofiorenzani tizianofiorenzani force-pushed the wips/dds-battery-multiple-channels branch 2 times, most recently from 02d7946 to d1475d2 Compare October 3, 2024 14:59
@tizianofiorenzani tizianofiorenzani force-pushed the wips/dds-battery-multiple-channels branch from d1475d2 to 2255ceb Compare October 3, 2024 20:37
@tizianofiorenzani
Copy link
Author

@Ryanf55 I did one little change: the snprintf is now hal.util->snprintf and I changed the formatting to "%u" rather than casting to int.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Oct 4, 2024

I re-ran failed jobs. This should be able to be merged shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants