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

Added const to moveit_core/collision_detection per issue 879 #1416

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

bgill92
Copy link
Contributor

@bgill92 bgill92 commented Jul 6, 2022

Description

Const'd function parameters and locally scoped variables in the collision_detection portion of of moveit_core. This PR pertains to Const as much as possible #879

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

Copy link
Contributor

@vatanaksoytezer vatanaksoytezer left a comment

Choose a reason for hiding this comment

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

Just a few more places to put consts, otherwise lgtm! Thank you @bgill92!

moveit_core/collision_detection/src/collision_matrix.cpp Outdated Show resolved Hide resolved
moveit_core/collision_detection/src/collision_matrix.cpp Outdated Show resolved Hide resolved
@bgill92 bgill92 force-pushed the 879_adding_const_to_collision_checking branch from c7965fc to b539d75 Compare July 8, 2022 13:19
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #1416 (c7965fc) into main (47c52f2) will increase coverage by 10.71%.
The diff coverage is 51.95%.

❗ Current head c7965fc differs from pull request most recent head 9e42921. Consider uploading reports for the commit 9e42921 to get more accurate results

@@             Coverage Diff             @@
##             main    #1416       +/-   ##
===========================================
+ Coverage   50.85%   61.55%   +10.71%     
===========================================
  Files         381      274      -107     
  Lines       31735    24982     -6753     
===========================================
- Hits        16135    15375      -760     
+ Misses      15600     9607     -5993     
Impacted Files Coverage Δ
...llision_detection/src/collision_octomap_filter.cpp 0.00% <0.00%> (ø)
...t_core/collision_detection/src/collision_tools.cpp 0.00% <0.00%> (ø)
..._core/collision_detection/src/collision_matrix.cpp 37.15% <50.00%> (ø)
...eit_core/collision_detection/src/collision_env.cpp 72.58% <88.24%> (ø)
moveit_core/collision_detection/src/world.cpp 88.11% <93.34%> (ø)
...collision_detection/src/collision_plugin_cache.cpp 82.36% <100.00%> (ø)
moveit_ros/planning/rdf_loader/src/rdf_loader.cpp 53.47% <0.00%> (-11.88%) ⬇️
...nclude/moveit_setup_framework/data/srdf_config.hpp
...obot_state_rviz_plugin/src/robot_state_display.cpp
...moveit_setup_app_plugins/src/perception_config.cpp
... and 104 more

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 18ffec0...9e42921. Read the comment docs.

@bgill92 bgill92 force-pushed the 879_adding_const_to_collision_checking branch from b539d75 to a2e0e56 Compare July 12, 2022 13:09
@tylerjw tylerjw force-pushed the 879_adding_const_to_collision_checking branch from a2e0e56 to 9e42921 Compare July 12, 2022 19:17
@tylerjw
Copy link
Member

tylerjw commented Jul 12, 2022

This one is next for merging, rebased.

@tylerjw tylerjw merged commit 2de48a7 into moveit:main Jul 12, 2022
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants