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

Adds a message that allows loading environments via a topic #320

Merged
merged 14 commits into from
Aug 4, 2023

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Dec 20, 2022

🦟 Bug fix

Fixes #

Summary

Required by gazebosim/gz-sim#1842

Signed-off-by: Arjo Chakravarty arjo@openrobotics.org

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

include/gz/msgs/Utility.hh Outdated Show resolved Hide resolved
proto/gz/msgs/data_load_options.proto Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #320 (5d3797f) into gz-msgs9 (c7e0323) will increase coverage by 0.15%.
The diff coverage is 100.00%.

❗ Current head 5d3797f differs from pull request most recent head 97531a0. Consider uploading reports for the commit 97531a0 to get more accurate results

@@             Coverage Diff              @@
##           gz-msgs9     #320      +/-   ##
============================================
+ Coverage     95.41%   95.57%   +0.15%     
============================================
  Files            10       10              
  Lines          1026     1062      +36     
============================================
+ Hits            979     1015      +36     
  Misses           47       47              
Files Changed Coverage Δ
src/Utility.cc 99.02% <100.00%> (+0.05%) ⬆️

@@ -212,6 +220,14 @@ namespace gz
msgs::SphericalCoordinates Convert(
const math::SphericalCoordinates &_coord);

/// \brief Convert a msgs::SphericalCoordinatesType to an
/// gz::math::SphericalCoordinates::CoordinateTpye
/// \param[in] _coord The spherical coordinates to convert
Copy link
Collaborator

Choose a reason for hiding this comment

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

The spherical coordinate type ...

include/gz/msgs/Utility.hh Outdated Show resolved Hide resolved
proto/gz/msgs/data_load_options.proto Outdated Show resolved Hide resolved
src/Utility.cc Show resolved Hide resolved
src/Utility.cc Outdated
case math::SphericalCoordinates::CoordinateType::LOCAL:
return msgs::SphericalCoordinatesType::LOCAL;
case math::SphericalCoordinates::CoordinateType::LOCAL2:
return msgs::SphericalCoordinatesType::LOCAL2;
Copy link
Collaborator

@caguero caguero Dec 20, 2022

Choose a reason for hiding this comment

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

This block returns a warning as we don't return anything if none of the previous conditions are met.

Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

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

Tiny comment, otherwise looks good to me.

proto/gz/msgs/data_load_options.proto Outdated Show resolved Hide resolved
Required by gazebosim/gz-sim#1842

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 force-pushed the arjo/feat/environment_data_messages branch from a89aee2 to cc4ce08 Compare January 25, 2023 03:17
@arjo129
Copy link
Contributor Author

arjo129 commented Jan 25, 2023

I suggest holding off on merging this till gazebosim/gz-sim#1842 is complete.

proto/gz/msgs/data_load_options.proto Outdated Show resolved Hide resolved
arjo129 and others added 2 commits March 1, 2023 09:18
@arjo129 arjo129 requested a review from ahcorde April 17, 2023 02:34
@arjo129 arjo129 enabled auto-merge April 18, 2023 06:59
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

There are some open comments from Carlos here, if you could address them.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
@arjo129 arjo129 requested a review from mjcarroll May 10, 2023 02:38
@arjo129
Copy link
Contributor Author

arjo129 commented May 17, 2023

@mjcarroll I've addressed the issues. It's waiting on a ✔️ from you.

@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
@arjo129 arjo129 merged commit 7fc5805 into gz-msgs9 Aug 4, 2023
8 checks passed
@arjo129 arjo129 deleted the arjo/feat/environment_data_messages branch August 4, 2023 21:54
@azeey
Copy link
Contributor

azeey commented Aug 25, 2023

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.

This was merged with "Merge commit". Let's remember to use "Squash-Merge" next time.

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 🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants