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

Type safety for CartesianInterpolator #1325

Merged

Conversation

wyattrees
Copy link
Member

@wyattrees wyattrees commented Jun 7, 2022

Description

Overloads of CartesianInterpolator::computeCartesianPath may return a distance or a percentage value, while the return type is always double. This PR introduces type safety for these return values.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@wyattrees wyattrees added backport-galactic Mergify label that triggers a PR backport to Galactic backport-humble Mergify label that triggers a PR backport to Humble labels Jun 7, 2022
Copy link
Collaborator

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

This is a nice change. It is really helpful when your compiler can help you detect logic errors.

@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #1325 (97c5a1e) into main (99c73e6) will decrease coverage by 0.04%.
The diff coverage is 42.31%.

@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
- Coverage   61.41%   61.38%   -0.03%     
==========================================
  Files         274      274              
  Lines       24963    24979      +16     
==========================================
+ Hits        15329    15331       +2     
- Misses       9634     9648      +14     
Impacted Files Coverage Δ
...nclude/moveit/robot_state/cartesian_interpolator.h 57.90% <41.67%> (-27.81%) ⬇️
...it_core/robot_state/src/cartesian_interpolator.cpp 39.42% <42.86%> (-0.43%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.29% <0.00%> (-0.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99c73e6...97c5a1e. Read the comment docs.

@henningkayser henningkayser merged commit 4e97446 into ros-planning:main Jun 16, 2022
mergify bot pushed a commit that referenced this pull request Jun 16, 2022
mergify bot pushed a commit that referenced this pull request Jun 16, 2022
henningkayser pushed a commit that referenced this pull request Jun 16, 2022
(cherry picked from commit 4e97446)

Co-authored-by: Wyatt Rees <wyatt.rees@gmail.com>
henningkayser pushed a commit that referenced this pull request Jun 16, 2022
(cherry picked from commit 4e97446)

Co-authored-by: Wyatt Rees <wyatt.rees@gmail.com>
peterdavidfagan pushed a commit to peterdavidfagan/moveit2 that referenced this pull request Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-galactic Mergify label that triggers a PR backport to Galactic backport-humble Mergify label that triggers a PR backport to Humble
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants