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

Add the ability for ros actions nodes to get access to the action result even on failure #79

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

Conversation

JayHerpin
Copy link

The current implementation assumes that if the Ros Action is aborted or otherwise fails that the result returned from the action server is not meaningful. However,there are some cases where that is not true. Consider nav2/action/NavgiateThroughPoses which has an error code in the result. That error code is basically only applicable when the action fails!

The proposed implementation maintains API compatibility. If the user cares about the result, they can implement the newly added virtual method.

Note that result is nullopt on any code paths where a result is not available.

@b-adkins
Copy link

b-adkins commented Jan 4, 2025

IMO, this is the downside of PImpl. If a needed callback is missing, it's really hard to add it via child class, because everything is extra private.

(Or maybe it's an upside? Because it forces you to patch the library. Which you can then submit as a PR!)

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