[JTC] Reject trajectories with nonzero terminal velocity by christophfroehlich · Pull Request #567 · ros-controls/ros2_controllers

christophfroehlich · 2023-04-08T20:51:47Z

As suggested by @bmagyar, the new trajectory should be even ignored if the last velocity point is nonzero.

This is valid for both interfaces (topic + actions).
I think there might be use-cases, where this is intentionally (maybe receiving topics from a MPC with moving horizon, where the end of the trajectory is not a steady state). Therefore, I added a parameter to be able to deactivate this behavior. To avoid breaking changes, I deactivated this by default now but give a deprecation warning if allow_nonzero_velocity_stop==true.

I also added a test for that, please review #603 first.

codecov-commenter · 2023-04-08T21:01:52Z

Codecov Report

Merging #567 (ff3096c) into master (e7f9962) will decrease coverage by 3.31%.
The diff coverage is 26.60%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #567      +/-   ##
==========================================
- Coverage   35.78%   32.48%   -3.31%     
==========================================
  Files         189        7     -182     
  Lines       17570      665   -16905     
  Branches    11592      357   -11235     
==========================================
- Hits         6287      216    -6071     
+ Misses        994      157     -837     
+ Partials    10289      292    -9997

Flag	Coverage Δ
unittests	`32.48% <26.60%> (-3.31%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ontroller/test/test_load_diff_drive_controller.cpp	`12.50% <0.00%> (ø)`
diff_drive_controller/src/odometry.cpp	`42.16% <11.11%> (ø)`
diff_drive_controller/src/speed_limiter.cpp	`46.55% <11.11%> (ø)`
...ive_controller/test/test_diff_drive_controller.cpp	`17.62% <12.08%> (ø)`
...troller/include/diff_drive_controller/odometry.hpp	`20.00% <20.00%> (ø)`
...iff_drive_controller/src/diff_drive_controller.cpp	`39.22% <35.50%> (ø)`
...de/diff_drive_controller/diff_drive_controller.hpp	`100.00% <100.00%> (ø)`

... and 189 files with indirect coverage changes

destogl · 2023-04-10T10:04:14Z

As suggested by @bmagyar, the new trajectory should be even ignored if the last velocity point is nonzero.

I don't agree with this at all. If we have velocity only input, we want to continue moving between trajectories. This is not so unusual scenario, e.g., when we have joystick input. I would rather like to see a timeout if we have velocity-only input to stop the input.

christophfroehlich · 2023-04-10T11:43:41Z

Ok, I did not think of using JTC with Joystick, but rejecting it always did not seem to make sense for me too.
So you mean we should even not hold the position automatically but only after some timeout reached?

destogl · 2023-04-11T17:57:57Z

So you mean we should even not hold the position automatically but only after some timeout reached?

Exactly, I think we want to avoid run away if only velocity is used.

christophfroehlich · 2023-04-11T18:22:39Z

now I'm confused :D maybe we should discuss this in the next WG meeting
I think there are different use-cases, and you have others in mind than I do..

BarisYazici · 2023-05-08T12:26:34Z

any update merging this PR?

christophfroehlich · 2023-05-08T13:19:34Z

We recently decided to merge several smaller changes (#567 #558 and some optional time-out for topic interface) for JTC into a feature branch, test them thoroughly together, and merge them then together into master.
I fear that this will take some more weeks to happen.

christophfroehlich · 2023-05-08T17:51:44Z

@BarisYazici you could help out by reviewing #558 and this PR. I think everyone can do this without special permissions (goto Files changed -> Review Changes on top right corner)

christophfroehlich · 2023-05-11T17:41:41Z

I've split this PR into two, the more important one is #608

destogl · 2023-06-02T16:02:47Z

I've split this PR into two, the more important one is #608

@christophfroehlich in which order should this be merged?

christophfroehlich · 2023-06-03T06:25:10Z

the order listed in #607 would be fine

mergify · 2023-06-20T20:39:24Z

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

mergify · 2023-06-26T16:37:37Z

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

joint_trajectory_controller/doc/userdoc.rst

christophfroehlich · 2023-07-02T18:47:01Z

The failing test is not related to this PR, but shows the flakiness of the JTC tests 😕

joint_trajectory_controller/src/joint_trajectory_controller_parameters.yaml

joint_trajectory_controller/doc/userdoc.rst

…s#567) * Reject receiving trajectory of last velocity point is non-zero * Update docs * Add tests * Change to parameterized test * Rename parameter * not true -> false --------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

* Reject receiving trajectory of last velocity point is non-zero * Update docs * Add tests * Change to parameterized test * Rename parameter * not true -> false --------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

* Reject receiving trajectory of last velocity point is non-zero * Update docs * Add tests * Change to parameterized test * Rename parameter * not true -> false --------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> (cherry picked from commit 99b67d7)

github-actions bot requested review from Serafadam, arne48, bmagyar, destogl, jackcenter, livanov93 and progtologist April 8, 2023 20:52

christophfroehlich changed the title ~~Hold the position after the goal was reached successfully~~ [JTC] Hold the position after the goal was reached successfully Apr 8, 2023

christophfroehlich changed the base branch from master to jtc-features May 8, 2023 17:47

This was referenced May 8, 2023

Renovate JTC action tests #603

Merged

[JTC] Fix tests and set_hold_position #607

Closed

christophfroehlich changed the title ~~[JTC] Hold the position after the goal was reached successfully~~ [JTC] Reject trajectories with nonzero terminal velocity May 11, 2023

christophfroehlich mentioned this pull request May 11, 2023

[JTC] Hold the position after the goal was reached successfully #608

Closed

2 tasks

bmagyar force-pushed the jtc-features branch from 391ad84 to 5a05e17 Compare June 26, 2023 18:09

christophfroehlich and others added 3 commits June 26, 2023 19:35

Reject receiving trajectory of last velocity point is non-zero

33c902a

Update docs

e38737f

Add tests

440d7d2

Change to parameterized test

cba9ce3

christophfroehlich force-pushed the jtc_hold_on_success branch from a78da5a to cba9ce3 Compare June 26, 2023 20:19

destogl reviewed Jun 27, 2023

View reviewed changes

joint_trajectory_controller/doc/userdoc.rst Outdated Show resolved Hide resolved

christophfroehlich and others added 2 commits June 27, 2023 15:43

Rename parameter

7dfaa51

Merge branch 'jtc-features' into jtc_hold_on_success

1b48ce7

saikishor approved these changes Jul 2, 2023

View reviewed changes

Merge branch 'jtc-features' into jtc_hold_on_success

eab5bdd

saikishor mentioned this pull request Jul 2, 2023

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

Merged

bmagyar reviewed Jul 3, 2023

View reviewed changes

joint_trajectory_controller/src/joint_trajectory_controller_parameters.yaml Outdated Show resolved Hide resolved

bmagyar reviewed Jul 3, 2023

View reviewed changes

joint_trajectory_controller/doc/userdoc.rst Outdated Show resolved Hide resolved

not true -> false

5b3f15f

bmagyar merged commit 811ae4d into ros-controls:jtc-features Jul 3, 2023

christophfroehlich mentioned this pull request Jul 4, 2023

[JTC] Explicitly set hold position #558

Merged

12 tasks

christophfroehlich mentioned this pull request Jul 17, 2023

[JTC] Re-enabling test, bugfixing and hardening. Adding a parameter to define when trajectories with non-zero velocity at the end are used. #705

Merged

christophfroehlich deleted the jtc_hold_on_success branch July 25, 2023 12:40

christophfroehlich mentioned this pull request Dec 10, 2023

[JTC] Invalidate empty trajectory messages #902

Merged

Conversation

christophfroehlich commented Apr 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Apr 8, 2023

Codecov Report

Uh oh!

destogl commented Apr 10, 2023

Uh oh!

christophfroehlich commented Apr 10, 2023

Uh oh!

destogl commented Apr 11, 2023

Uh oh!

christophfroehlich commented Apr 11, 2023

Uh oh!

BarisYazici commented May 8, 2023

Uh oh!

christophfroehlich commented May 8, 2023

Uh oh!

christophfroehlich commented May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christophfroehlich commented May 11, 2023

Uh oh!

destogl commented Jun 2, 2023

Uh oh!

christophfroehlich commented Jun 3, 2023

Uh oh!

mergify bot commented Jun 20, 2023

Uh oh!

mergify bot commented Jun 26, 2023

Uh oh!

Uh oh!

christophfroehlich commented Jul 2, 2023

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

christophfroehlich commented Apr 8, 2023 •

edited

Loading

christophfroehlich commented May 8, 2023 •

edited

Loading