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

Added remapping resolution for action names (backport #1170) #1220

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

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Mar 25, 2025

Currently remapping of action names is not possible see ros2/ros2#1312 .
I have now stumbled over this multiple times and I think it is a useful feature, so i took a shot at implementing the remapping of action names. Please let me know if you have any comments or if I overlooked something.
Fixes ros2/ros2#1312.


This is an automatic backport of pull request #1170 done by Mergify.

* Added remapping resolution for action names

Signed-off-by: Justus Braun <[email protected]>

* Fix cpplint/uncrustify

Signed-off-by: Justus Braun <[email protected]>

* Simplified returned error codes in case name resolution failes.

Signed-off-by: Justus Braun <[email protected]>

* Renamed action_name field to remapped_action_name.

Signed-off-by: Justus Braun <[email protected]>

* Removed unnecessary resolved_action_name stack variable

Signed-off-by: Justus Braun <[email protected]>

* Added tests for action name remapping.

Signed-off-by: Justus Braun <[email protected]>

* Add tests for action name remapping using local arguments

Signed-off-by: Justus Braun <[email protected]>

---------

Signed-off-by: Justus Braun <[email protected]>
(cherry picked from commit 3ea07c7)
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

I think this looks good to backport.

  • This is bug fix to resolve action names into remapped names if specified.
  • Only impl structure is changed, that means we do not break user space.

@fujitatomoya
Copy link
Collaborator

Pulls: #1220
Gist: https://gist.githubusercontent.com/fujitatomoya/535d6c2cbecc1480b5f9cec280f5487e/raw/3581f6c56d5709ab80263c82a86684930e115d5f/ros2.repos
BUILD args: --packages-above-and-dependencies rcl_action
TEST args: --packages-above rcl_action
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15460

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants