Skip to content

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