gazebosim / gz-msgs

Messages for Gazebo robot simulation.

https://gazebosim.org

Apache License 2.0

24 stars 47 forks source link

Added cubemap_uri to sky #306

Closed nkoenig closed 1 year ago

nkoenig commented 2 years ago

🎉 New feature

Summary

Added a cubemap_uri string to the sky.proto message, which supports https://github.com/gazebosim/sdformat/pull/1037.

Checklist

[x] Signed all commits for DCO
[ ] Added tests
[ ] Added example and/or tutorial
[ ] Updated documentation (as needed)
[ ] Updated migration guide (as needed)
[ ] Consider updating Python bindings (if the library has them)
[x] codecheck passed (See contributing)
[x] All tests passed (See test coverage)
[x] 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

mjcarroll commented 2 years ago

Test failures look to be unrelated. Any chance you would want to address them here or is this time-critical?

mjcarroll commented 2 years ago

Actually, is this supposed to be targeting main or gz-msgs9

nkoenig commented 2 years ago

This is suppose to target main because msgs9 has already been released. I could squint my eyes and sneak this into gz-msgs9. It technically breaks API, but I doubt anyone has used gz-msgs9 in the last couple hours.

mabelzhang commented 2 years ago

Test failures look to be unrelated (bash completion flags). ~~This is time critical for end of the week.~~

mabelzhang commented 2 years ago

Wait, we're doing skybox for Fortress, which is still on msgs8 right?

nkoenig commented 2 years ago

This is not needed for the skybox work. The skybox work uses the header map in the sky message. This is here to replace that method in the future.

nkoenig commented 2 years ago

Waiting on gz-sim7 to main forward port.

codecov[bot] commented 1 year ago

Codecov Report

Merging #306 (236ccab) into main (5d00d2a) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 236ccab differs from pull request most recent head b69c696. Consider uploading reports for the commit b69c696 to get more accurate results

@@           Coverage Diff           @@
##             main     #306   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files           9        9           
  Lines        1073     1073           
=======================================
  Hits         1027     1027           
  Misses         46       46