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

Backport #1748: Adds a tool for environment data visualization and custom environmental sensors #1798

Merged
merged 11 commits into from
Nov 21, 2022

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Nov 16, 2022

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

🎉 New feature

Closes #

Summary

This PR adds a tool to visualize Scalar Environmental Data. It also adds custom sensors. It is a backport of #1748

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

Test it

Checklist

  • 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)
  • 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.

…stom environmental sensors

This PR adds a tool to visualize Scalar Environmental Data. It also adds custom sensors.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 added 🌱 garden Ignition Garden MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv labels Nov 16, 2022
@arjo129 arjo129 requested a review from hidmic November 16, 2022 01:28
@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #1798 (fae0438) into gz-sim7 (2d19f93) will increase coverage by 0.20%.
The diff coverage is 81.08%.

@@             Coverage Diff             @@
##           gz-sim7    #1798      +/-   ##
===========================================
+ Coverage    64.26%   64.47%   +0.20%     
===========================================
  Files          340      341       +1     
  Lines        26962    27187     +225     
===========================================
+ Hits         17328    17529     +201     
- Misses        9634     9658      +24     
Impacted Files Coverage Δ
include/gz/sim/Util.hh 100.00% <ø> (ø)
include/gz/sim/components/Environment.hh 100.00% <ø> (ø)
src/Util.cc 91.94% <70.58%> (-1.15%) ⬇️
.../systems/environment_preload/EnvironmentPreload.cc 69.69% <75.00%> (-1.22%) ⬇️
...nmental_sensor_system/EnvironmentalSensorSystem.cc 82.75% <82.75%> (ø)
src/components/Environment.cc 100.00% <100.00%> (ø)
src/SdfEntityCreator.cc 86.44% <0.00%> (+0.88%) ⬆️
src/systems/acoustic_comms/AcousticComms.cc 98.13% <0.00%> (+3.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM, just like #1748. Jenkins CI seems broken though.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Copy link
Contributor Author

arjo129 commented Nov 17, 2022

https://build.osrfoundation.org/job/ignition_gazebo-ci-pr_any-ubuntu_auto-amd64/9909/ failing because of warnings triggered by code generated by Qt's moc. I don't think theres much I can do. Also I'm trying to understand the ABI checker I think the issue is we merged #1616, before merging this PR in. I don't know if we released #1616, but if we have then we are ☠️ . I made some changes to the environmental data component so that we could use it.

@hidmic
Copy link
Contributor

hidmic commented Nov 17, 2022

@arjo129 hmm, I think we can still solve those issues (or they will come back to hunt us in the future).

On Windows, it looks like we have visibility issues.

error LNK2001: unresolved external symbol "class std::optional<class gz::math::v7::Vector3<double> > __cdecl gz::sim::v7::getGridFieldCoordinates(class gz::sim::v7::EntityComponentManager const &,class gz::math::v7::Vector3<double> const &,class std::shared_ptr<struct gz::sim::v7::components::EnvironmentalData> const &)" (?getGridFieldCoordinates@v7@sim@gz@@YA?AV?$optional@V?$Vector3@N@v7@math@gz@@@std@@AEBVEntityComponentManager@123@AEBV?$Vector3@N@1math@3@AEBV?$shared_ptr@UEnvironmentalData@components@v7@sim@gz@@@5@@Z) [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\gz-sim7\src\gui\plugins\environment_visualization\EnvironmentVisualization.vcxproj]

Namely, getGridFieldCoordinates is missing a GZ_SIM_VISIBLE attribute.

On Ubuntu, I suspect that it is the use of the Q_PROPERTY(... MEMBER ...) form that is triggering bad code generation within the MOC. It is the first instance of this form I see in the codebase.

And while the ABI checker output is pretty much useless, I suspect it takes issue with the changes to the EnvironmentalData struct, none of which are ABI compatible even if API compatible.

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>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129
Copy link
Contributor Author

arjo129 commented Nov 21, 2022

Test failures are now unrelated. This does break ABI of the nightly, but since we have yet to release it should not be a problem. Going to merge.

@arjo129 arjo129 merged commit 4fa1f26 into gz-sim7 Nov 21, 2022
@arjo129 arjo129 deleted the arjo/backport/environmental_data_viz branch November 21, 2022 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants