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.

Sign up for GitHub

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

Jump to bottom

JTC trajectory end time validation fix #1090

Merged
merged 7 commits into from
May 22, 2024
Merged

JTC trajectory end time validation fix #1090

merged 7 commits into from
May 22, 2024

Conversation

henrygerardmoore
Copy link
Contributor

The time_from_start member for a JointTrajectoryPoint is monotonically increasing, not a delta time, so I think we can just use the last one in the vector; summing them up seems incorrect. I also moved the points empty check earlier because .back().time_from_start would seg fault with an empty points vector.

@christophfroehlich
Copy link
Contributor

Could you please check the docs if this is clearly documented how to fill the trajectory field? Here is a comment, but maybe we should add this in a more prominent place.

@henrygerardmoore
Copy link
Contributor Author

Could you please check the docs if this is clearly documented how to fill the trajectory field? Here is a comment, but maybe we should add this in a more prominent place.

@christophfroehlich
That page seems clear to me, the plots in there are very helpful. Perhaps it should be in the main JTC page, but where it is seems fine IMO.

christophfroehlich
christophfroehlich requested changes Apr 4, 2024
View reviewed changes
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM. Just one request: Could you please add checks for the end time to invalid_message test?

christophfroehlich
christophfroehlich approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@christophfroehlich christophfroehlich added backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron. labels Apr 15, 2024
@henrygerardmoore
Copy link
Contributor Author

@christophfroehlich is there anything I can do to help get this across the finish line?

@christophfroehlich
Copy link
Contributor

No, we have to wait for an approval of @destogl or @bmagyar

@christophfroehlich christophfroehlich linked an issue May 22, 2024 that may be closed by this pull request
JTC computes wrong trajectory_end_time during validate_trajectory_msg #1138
Closed
@christophfroehlich christophfroehlich mentioned this pull request May 22, 2024
Closed
@bmagyar bmagyar merged commit b51e4c2 into ros-controls:master May 22, 2024
5 of 18 checks passed
mergify bot pushed a commit that referenced this pull request May 22, 2024
@henrygerardmoore @mergify
JTC trajectory end time validation fix (#1090)
bd25d5f 
(cherry picked from commit b51e4c2)
@mergify mergify bot mentioned this pull request May 22, 2024
Merged
mergify bot pushed a commit that referenced this pull request May 22, 2024
@henrygerardmoore @mergify
JTC trajectory end time validation fix (#1090)
0a2bd6c 
(cherry picked from commit b51e4c2)
@mergify mergify bot mentioned this pull request May 22, 2024
Merged
@henrygerardmoore henrygerardmoore deleted the jtc_validation_fix branch May 22, 2024 17:25
christophfroehlich pushed a commit that referenced this pull request May 22, 2024
@henrygerardmoore @christophfroehlich
JTC trajectory end time validation fix (#1090)
cb0e239 
(cherry picked from commit b51e4c2)
christophfroehlich pushed a commit that referenced this pull request May 22, 2024
@henrygerardmoore @christophfroehlich
JTC trajectory end time validation fix (#1090)
fe85da4 
(cherry picked from commit b51e4c2)
christophfroehlich pushed a commit that referenced this pull request May 22, 2024
@mergify @henrygerardmoore
JTC trajectory end time validation fix (#1090) (#1140)
11e8ad7 
(cherry picked from commit b51e4c2)

Co-authored-by: Henry Moore <[email protected]>
christophfroehlich pushed a commit that referenced this pull request May 22, 2024
@mergify @henrygerardmoore
JTC trajectory end time validation fix (#1090) (#1141)
cb40e0b 
(cherry picked from commit b51e4c2)

Co-authored-by: Henry Moore <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophfroehlich christophfroehlich christophfroehlich approved these changes

@bmagyar bmagyar bmagyar approved these changes

@bijoua29 bijoua29 bijoua29 approved these changes

@arne48 arne48 Awaiting requested review from arne48

@destogl destogl Awaiting requested review from destogl

@jaron-l jaron-l Awaiting requested review from jaron-l

@livanov93 livanov93 Awaiting requested review from livanov93

Assignees
No one assigned
Labels
backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JTC computes wrong trajectory_end_time during validate_trajectory_msg
4 participants
@henrygerardmoore @christophfroehlich @bmagyar @bijoua29