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

Enable specific entities on buoyancy plugin #1067

Merged
merged 9 commits into from
Sep 30, 2021

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

This is technically a feature, but it fixes a usability bug that affects the Fortress demo.

Summary

Adds the ability of whitelisting just a few models in simulation which will be affected by buoyancy.

Also reduced console spam by printing a warning only once.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina requested a review from arjo129 September 28, 2021 16:58
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 28, 2021
@chapulina chapulina added beta Targeting beta release of upcoming collection bug Something isn't working labels Sep 28, 2021
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Copy link
Contributor

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Tested this. It seems to work for me. There are some style fixes required before merging though.

@chapulina chapulina changed the title Add whitelist to buoyancy plugin Enable specific entities on buoyancy plugin Sep 29, 2021
examples/worlds/graded_buoyancy.sdf Outdated Show resolved Hide resolved
test/worlds/graded_buoyancy.sdf Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #1067 (43d8e86) into main (a452a01) will increase coverage by 0.00%.
The diff coverage is 80.00%.

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

@@           Coverage Diff           @@
##             main    #1067   +/-   ##
=======================================
  Coverage   64.06%   64.06%           
=======================================
  Files         255      255           
  Lines       19963    19986   +23     
=======================================
+ Hits        12790    12805   +15     
- Misses       7173     7181    +8     
Impacted Files Coverage Δ
src/systems/buoyancy/Buoyancy.hh 100.00% <ø> (ø)
src/systems/buoyancy/Buoyancy.cc 83.01% <80.00%> (+0.47%) ⬆️
src/SimulationRunner.cc 92.91% <0.00%> (-1.02%) ⬇️
src/Util.cc 93.53% <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 a452a01...5b9bc46. Read the comment docs.

Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

I've addressed all comments, merging!

@chapulina chapulina merged commit d6fbc26 into main Sep 30, 2021
@chapulina chapulina deleted the chapulina/6/buoyancy_whitelist branch September 30, 2021 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection bug Something isn't working 🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants