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

add API for passing RNG to setToRandomPositionsNearBy #2799

Merged
merged 2 commits into from
Aug 27, 2021

Conversation

PeterMitrano
Copy link
Contributor

Description

add API for passing RNG to setToRandomPositionsNearBy

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

@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #2799 (b5f775e) into master (b170d62) will decrease coverage by 0.43%.
The diff coverage is 50.00%.

❗ Current head b5f775e differs from pull request most recent head cb4ca38. Consider uploading reports for the commit cb4ca38 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2799      +/-   ##
==========================================
- Coverage   60.87%   60.44%   -0.42%     
==========================================
  Files         366      366              
  Lines       31705    31677      -28     
==========================================
- Hits        19296    19144     -152     
- Misses      12409    12533     +124     
Impacted Files Coverage Δ
...bot_state/include/moveit/robot_state/robot_state.h 83.86% <ø> (-1.18%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 50.43% <50.00%> (-1.21%) ⬇️
moveit_core/robot_state/src/conversions.cpp 44.71% <0.00%> (-19.43%) ⬇️
.../ompl_interface/src/detail/constrained_sampler.cpp 43.25% <0.00%> (-16.21%) ⬇️
moveit_core/robot_state/src/attached_body.cpp 48.15% <0.00%> (-5.95%) ⬇️
moveit_core/planning_scene/src/planning_scene.cpp 58.77% <0.00%> (-4.37%) ⬇️
moveit_core/robot_model/src/planar_joint_model.cpp 75.68% <0.00%> (-0.90%) ⬇️
...istance_field/src/collision_env_distance_field.cpp 64.23% <0.00%> (-0.04%) ⬇️
... and 25 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 b170d62...cb4ca38. Read the comment docs.

@PeterMitrano
Copy link
Contributor Author

See #2802 for CI failure

@PeterMitrano PeterMitrano force-pushed the randomPositionsNearBy branch 2 times, most recently from 741e194 to b5f775e Compare August 13, 2021 15:21
Copy link
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 seems reasonable. Do you plan on updating the tests to make them more consistent by using a fixed seed and a specific RNG?

@tylerjw
Copy link
Member

tylerjw commented Aug 27, 2021

Please rebase so we can merge this. I tried updating it for you but the checkbox to let me push wasn't checked.

@felixvd
Copy link
Contributor

felixvd commented Aug 27, 2021

This seems reasonable. Do you plan on updating the tests to make them more consistent by using a fixed seed and a specific RNG?

Nice idea. Make a "good first issue" post out of it!

@PeterMitrano
Copy link
Contributor Author

Yea I can put adding a test on my TODO list but no guarantees, it would be good to make an issue for it then I can watch it

@tylerjw
Copy link
Member

tylerjw commented Aug 27, 2021

Yea I can put adding a test on my TODO list but no guarantees, it would be good to make an issue for it then I can watch it

#2851

@tylerjw tylerjw merged commit ee528ea into moveit:master Aug 27, 2021
Devesh-PatelUF added a commit to Devesh-PatelUF/moveit that referenced this pull request Jan 31, 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