[JTC] Return INVALID_GOAL error code for rejected trajectories (#700)#2237
Open
AdvityaCode wants to merge 1 commit intoros-controls:masterfrom
Open
[JTC] Return INVALID_GOAL error code for rejected trajectories (#700)#2237AdvityaCode wants to merge 1 commit intoros-controls:masterfrom
AdvityaCode wants to merge 1 commit intoros-controls:masterfrom
Conversation
christophfroehlich
requested changes
Mar 28, 2026
Member
christophfroehlich
left a comment
There was a problem hiding this comment.
Thank you for picking this.
We will have to discuss this in the next PMC meeting if the accept-and-immediately-reject pattern is something everyone is happy with.
I propose that we use tl::expected (until std::expected is available) instead of returning an empty string.
| bool validate_trajectory_msg(const trajectory_msgs::msg::JointTrajectory & trajectory) const; | ||
| /// Validate a trajectory message. Returns an empty string if valid, or a | ||
| /// human-readable error description if the trajectory should be rejected. | ||
| std::string validate_trajectory_msg( |
Member
There was a problem hiding this comment.
what do you think of using tl-expected here? Similar to the validators in https://github.com/PickNikRobotics/generate_parameter_library?tab=readme-ov-file#custom-validator-functions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the Joint Trajectory Controller rejects an invalid trajectory (empty points, wrong joint names, non-monotonic timestamps, etc.), the action client receives an opaque REJECT response with no structured feedback. The only signal available to the client is a log line in the controller's terminal — which is not a viable error-handling strategy for automated systems.
This is a limitation of action protocol (tracked upstream at ros2/rclc#271): GoalResponse::REJECT provides no mechanism to set error_code or error_string on the result.
Closes #700.
Proposed Solution
This is my suggested approach — I'm open to feedback and happy to revise the implementation. It is just what I thought would be the best approach.
The workaround discussed in #699 is to always accept goals at the action protocol level, then immediately abort with a populated error_code and error_string if validation fails. I followed that direction here, but I wasn't sure on some of the design choices so please let me know if any of these should be done differently:
- Whether the controller is active (PRIMARY_STATE_INACTIVE → abort with INVALID_GOAL)
- Whether the trajectory message is valid (all existing checks → abort with INVALID_GOAL + descriptive error_string)
I wasn't certain whether changing the return type of validate_trajectory_msg to std::string was the right call, or whether an out-parameter or a separate struct would be preferred. Happy to change this if there's a style the project prefers.
Tests