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

Fix SJTC from immediately returning success (#697) #710

Closed
wants to merge 1 commit into from

Conversation

panagelak
Copy link

I applied the fix (for humble) which was done at the ros2_controllers repo ros-controls/ros2_controllers#565 at the commit 2f546066827cf90020d6040c180ae564407b9bae

This pr refers to your newly opened issue #697

I hope it is merged : ) cheers

@@ -262,6 +267,7 @@ controller_interface::return_type ScaledJointTrajectoryController::update(const
result->set__error_code(FollowJTrajAction::Result::GOAL_TOLERANCE_VIOLATED);
active_goal->setAborted(result);
rt_active_goal_.writeFromNonRT(RealtimeGoalHandlePtr());

Choose a reason for hiding this comment

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

Would we need to also set traj_point_active_ptr_ = nullptr; at this line, since we reset the goal pointer here?

Copy link
Author

Choose a reason for hiding this comment

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

yes i thought about that too, but i did not see it in the original too when return error code goal_tolerance_violated.
this could be an error on the original or not needed or potentially breaking if inserted.
Furthermore it set's the nullptr on_cleanup and on_reset methods which i think are inhereted into the SJTC

@fmauch
Copy link
Collaborator

fmauch commented Jun 7, 2023

Thank you for providing this. Me or @RobertWilbrandt will have to look into this more deeply.

@fmauch
Copy link
Collaborator

fmauch commented Sep 21, 2023

I think this should be solved by #811

@fmauch
Copy link
Collaborator

fmauch commented Sep 21, 2023

Thank you @schornakj for bringing that up. Since we've updated the SJTC with the newest version from upstream's JTC this fix is included now (At the time of writing pending for Humble). I'm going to close this therefore.

@fmauch fmauch closed this Sep 21, 2023
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.

3 participants