Skip to content

Type safety for CartesianInterpolator#1325

Merged
henningkayser merged 1 commit intomoveit:mainfrom
wyattrees:pr-cartesian_interp_type_safety
Jun 16, 2022
Merged

Type safety for CartesianInterpolator#1325
henningkayser merged 1 commit intomoveit:mainfrom
wyattrees:pr-cartesian_interp_type_safety

Conversation

@wyattrees
Copy link
Copy Markdown
Contributor

@wyattrees wyattrees commented Jun 7, 2022 •
edited
Loading

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
tylerjw
tylerjw approved these changes Jun 7, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@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
Copy Markdown

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 approved these changes Jun 16, 2022 •
edited
Loading
View reviewed changes
@henningkayser henningkayser merged commit 4e97446 into moveit:main Jun 16, 2022
mergify bot pushed a commit that referenced this pull request Jun 16, 2022
@wyattrees @mergify
Type safety for CartesianInterpolator (#1325)
a2ed7f3 
(cherry picked from commit 4e97446)
@mergify mergify bot mentioned this pull request Jun 16, 2022
Merged
mergify bot pushed a commit that referenced this pull request Jun 16, 2022
@wyattrees @mergify
Type safety for CartesianInterpolator (#1325)
c663b08 
(cherry picked from commit 4e97446)
@mergify mergify bot mentioned this pull request Jun 16, 2022
Merged
henningkayser pushed a commit that referenced this pull request Jun 16, 2022
@mergify @wyattrees
Type safety for CartesianInterpolator (#1325) (#1375)
1725620 
(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
@mergify @wyattrees
Type safety for CartesianInterpolator (#1325) (#1374)
048ef98 
(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
@wyattrees @peterdavidfagan
Type safety for CartesianInterpolator (moveit#1325)
6b278bd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tylerjw tylerjw tylerjw approved these changes

@henningkayser henningkayser henningkayser approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wyattrees @tylerjw @henningkayser