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] Add time-out for trajectory interfaces #609

Merged
merged 19 commits into from
Sep 14, 2023
Merged

[JTC] Add time-out for trajectory interfaces #609

merged 19 commits into from
Sep 14, 2023

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented May 12, 2023 •
edited
Loading

This adds an optional time-out parameter for the trajectory interfaces (topic or action).

If using velocity as command interface, and the last velocity point is not zero -> the robot would move forever without stopping if no new trajectory is received.

@christophfroehlich christophfroehlich mentioned this pull request May 12, 2023
Closed
@christophfroehlich
Copy link
Contributor Author

Just wondering now, should the timeout be counted from the reception of the message, or from the time of the end of the trajectory (i.e., before_last_point==false)?

@bmagyar
Copy link
Member

bmagyar commented May 22, 2023

Good question... It's intuitive to do it from receiving the message but since this particular message has explicit timing in it I think we should only allow timeouts defined relative to the trajectory

@destogl
Copy link
Member

destogl commented May 31, 2023

We should define it from the end for the trajectory (time of the last point)

@christophfroehlich christophfroehlich marked this pull request as draft June 4, 2023 21:44
@mergify
Copy link
Contributor

mergify bot commented Jun 20, 2023

This pull request is in conflict. Could you fix it @christophfroehlich?

@bmagyar
Copy link
Member

bmagyar commented Jun 20, 2023

@christophfroehlich this PR shouldn't really target the jtc-features branch no?

@christophfroehlich
Copy link
Contributor Author

@christophfroehlich this PR shouldn't really target the jtc-features branch no?

It is somehow independent, but why not?

@mergify
Copy link
Contributor

mergify bot commented Jun 26, 2023

This pull request is in conflict. Could you fix it @christophfroehlich?

@christophfroehlich christophfroehlich marked this pull request as ready for review June 27, 2023 07:45
@mergify
Copy link
Contributor

mergify bot commented Jul 3, 2023

This pull request is in conflict. Could you fix it @christophfroehlich?

@bmagyar bmagyar added the hold Holding off merging this PR until some condition label Jul 17, 2023
@bmagyar
Copy link
Member

bmagyar commented Jul 17, 2023

Putting this on hold. It's not essential for the things on the jtc-features branch and we should prioritize those changes landing first.
This can come after directly onto the master branch.

@christophfroehlich christophfroehlich changed the base branch from jtc-features to master July 17, 2023 19:22
@christophfroehlich
Copy link
Contributor Author

CI fails because of flaky JTC tests, not related to this PR

@christophfroehlich christophfroehlich mentioned this pull request Aug 24, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2023

This pull request is in conflict. Could you fix it @christophfroehlich?

@bmagyar bmagyar removed the hold Holding off merging this PR until some condition label Sep 12, 2023
bmagyar
bmagyar approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Thank you, I'll apply the comments to fix the test compilation (due to the updated test API)

joint_trajectory_controller/doc/parameters.rst Outdated Show resolved Hide resolved
joint_trajectory_controller/src/joint_trajectory_controller.cpp
Comment on lines +236 to +237
traj_msg_external_point_ptr_.reset();
traj_msg_external_point_ptr_.initRT(set_hold_position());
Copy link
Member

Choose a reason for hiding this comment

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

do we always have to call both? I'd imagine re-setting the value resets things properly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a consequence of #168, I didn't know how to perform this because there is no set-method. But probably the reset() is not necessary before.

joint_trajectory_controller/test/test_trajectory_controller.cpp Outdated Show resolved Hide resolved
joint_trajectory_controller/test/test_trajectory_controller.cpp Outdated Show resolved Hide resolved
joint_trajectory_controller/test/test_trajectory_controller.cpp Outdated Show resolved Hide resolved
joint_trajectory_controller/test/test_trajectory_controller.cpp Outdated Show resolved Hide resolved
@bmagyar bmagyar merged commit 0f08731 into ros-controls:master Sep 14, 2023
@christophfroehlich christophfroehlich deleted the jtc_timeout branch September 14, 2023 17:00
@fmauch fmauch mentioned this pull request Sep 18, 2023
Closed
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Oct 21, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (ros-controls#609)
e72da08
@christophfroehlich christophfroehlich mentioned this pull request Oct 21, 2023
Merged
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Oct 21, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
e2fa699
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 11, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
0c79858
@christophfroehlich christophfroehlich mentioned this pull request Nov 14, 2023
Merged
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 15, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
645b65c
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 17, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
c412cf5
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 27, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
35f7a9d
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 29, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
389e6fe
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Nov 30, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
9837cd2
christophfroehlich added a commit to christophfroehlich/ros2_controllers that referenced this pull request Dec 5, 2023
@christophfroehlich
[JTC] Add time-out for trajectory interfaces (backport ros-controls#609)
4b9e25b
bmagyar pushed a commit that referenced this pull request Dec 6, 2023
@christophfroehlich @bmagyar
[JTC] Add time-out for trajectory interfaces (backport #609)
e59a007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saikishor saikishor saikishor approved these changes

@bmagyar bmagyar bmagyar approved these changes

@arne48 arne48 Awaiting requested review from arne48

@destogl destogl Awaiting requested review from destogl

@jackcenter jackcenter Awaiting requested review from jackcenter

@peterdavidfagan peterdavidfagan Awaiting requested review from peterdavidfagan

@sgmurray sgmurray Awaiting requested review from sgmurray

@VX792 VX792 Awaiting requested review from VX792

@DasRoteSkelett DasRoteSkelett Awaiting requested review from DasRoteSkelett

@erickisos erickisos Awaiting requested review from erickisos

@malapatiravi malapatiravi Awaiting requested review from malapatiravi

@mcbed mcbed Awaiting requested review from mcbed

@qiayuanl qiayuanl Awaiting requested review from qiayuanl

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@christophfroehlich @bmagyar @destogl @codecov-commenter @saikishor