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

[JTC] Backport recent changes #801

Merged
merged 22 commits into from
Dec 6, 2023

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Oct 21, 2023

This PR includes backports of PRs for JTC (with fixed merge conflicts) in the correct order.
Besides updates to the tests and docs, this includes

We decided backporting #558 (including its fixes) during a recent WG meeting. The others are optional but should not break anything (but avoid conflicts for future backports).

This PR should not be squashed maybe, but ff-merged on top of humble?! I could also make separate backport PRs, but this will take some time as they are depending on each other and are bugfixing earlier commits.

@padhupradheep
Copy link

padhupradheep commented Nov 2, 2023

Hey Folks,

Would be really helpful if we get this backport.. Does this need a review or something that I might probably help with?

Need this one #558 specifically..

@christophfroehlich
Copy link
Contributor Author

Hey Folks,

Would be really helpful if we get this backport.. Does this need a review or something that I might probably help with?

Need this one #558 specifically..

Reviews always help :) Feel free to clone the branch, test it with your setup, and leave your approval here!

@padhupradheep
Copy link

tested out #558 already.. I'm satisfied with the behavior. Will try to test the others too when I get some time.

Thanks @christophfroehlich

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

❗ No coverage uploaded for pull request base (humble@53ec31b). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 493990d differs from pull request most recent head 41304df. Consider uploading reports for the commit 41304df to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             humble     #801   +/-   ##
=========================================
  Coverage          ?   44.79%           
=========================================
  Files             ?       40           
  Lines             ?     3710           
  Branches          ?     1766           
=========================================
  Hits              ?     1662           
  Misses            ?      836           
  Partials          ?     1212           
Flag Coverage Δ
unittests 44.79% <0.00%> (?)

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

@bmagyar
Copy link
Member

bmagyar commented Nov 29, 2023

This is big... Put it out of draft mode as if we leave it there it will never be merged

christophfroehlich and others added 22 commits December 5, 2023 22:51
…#613)

* Add new test to ensure that controller goes into position holding when tolerances are violated
* Hold position if goal_time is exceeded with topic interface
* Fix hold on time-violation
* Simplify initialization of states
* Rename read_state_from_hardware method
* Don't set state_desired in on_activate
---------

Co-authored-by: Dr. Denis <[email protected]>
* Fix dynamic reconfigure of tolerances

* Fix static cast syntax
* Fix floating point comparison in JTC

* Fix format

---------

Co-authored-by: Christoph Fröhlich <[email protected]>
@bmagyar
Copy link
Member

bmagyar commented Dec 6, 2023

Heads up @fmauch @RobertWilbrandt

Just pushed a release to Humble without these changes. This will be included in the next release. Good news is that we do not intend to make any substantial changes to the JTC in Humble in the future.

@bmagyar bmagyar merged commit df59eec into ros-controls:humble Dec 6, 2023
11 of 13 checks passed
@christophfroehlich christophfroehlich deleted the jtc/backport branch December 6, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants