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: revert change that removed namespace for TF subscriber topic #25384

Merged

Conversation

srmainwaring
Copy link
Contributor

@srmainwaring srmainwaring commented Oct 27, 2023

Without a namespace prefix the micro-ROS agent will attempt to send all /tf messages to ArduPilot DDS. This can quickly overwhelm the AP_DDS subscriber for large TF trees.

The preferred approach should be to use a topic_tools relay node to forward TF only when required, and prune the tree with a filter if appropriate.

Testing

Without this change the rover example

ros2 launch ardupilot_gz_bringup wildthumper_playpen.launch.py rviz:=true use_gz_tf:=true

results in continuous reporting of [RTPS_READER_HISTORY Error]

[micro_ros_agent-3] 2023-10-27 09:21:03.217 [RTPS_READER_HISTORY Error] Change payload size of '1764' bytes is larger than the history payload size of '1031' bytes and cannot be resized. -> Function can_change_be_added_nts
[micro_ros_agent-3] 2023-10-27 09:21:03.265 [RTPS_READER_HISTORY Error] Change payload size of '1764' bytes is larger than the history payload size of '1031' bytes and cannot be resized. -> Function can_change_be_added_nts
[micro_ros_agent-3] 2023-10-27 09:21:03.316 [RTPS_READER_HISTORY Error] Change payload size of '1764' bytes is larger than the history payload size of '1031' bytes and cannot be resized. -> Function can_change_be_added_nts

This is because the rover TF tree is large:

rover-tf

With the change the error is not reported.

@srmainwaring srmainwaring added ROS For-4.5 Planned for 4.5 release labels Oct 27, 2023
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
@srmainwaring srmainwaring force-pushed the prs/pr-dds-namespace-tf-sub-topic branch from e37cf7f to 12340ac Compare October 27, 2023 08:39
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.

I support this change. Yes, as @pedro-fuoco had originally found, it's annoying to deal with the remappings everywhere, but this is a necessary evil when working on a resource-constrained environment like ArduPilot where MicroXRCEDDS can't chunk payloads.

@srmainwaring
Copy link
Contributor Author

Also, we will need the prefix when we start dealing with more than one vehicle, and this keeps the topics consistent at least.

@peterbarker peterbarker merged commit 33f1221 into ArduPilot:master Oct 27, 2023
86 checks passed
@srmainwaring srmainwaring deleted the prs/pr-dds-namespace-tf-sub-topic branch October 28, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For-4.5 Planned for 4.5 release ROS
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants