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

Component inspector: refactor Pose3d C++ code into a separate class #1400

Merged
merged 3 commits into from
Mar 23, 2022

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Mar 19, 2022

🎉 New feature

Part of

Summary

While working on the system inspector, I thought it would be good to adopt some of the refactoring done for the model editor in #1231 to keep the code organized. That work is happening on the chapulina/6/system_inspector branch.

This PR ports the approach used on ComponentModelEditor:

  • All the logic relative to a given display type is moved to its own class / file.
  • I ported Pose3d to the new format as an example. It's a comprehensive example because:
    • It registers a callback to update the view (C++ ➡️ QML)
    • It has a C++ Q_INVOKABLE function which is called from QML (QML ➡️ C++)
  • Some differences from ComponentModelEditor
    • Renamed RegisterComponentCreator to AddUpdateViewCb because I think this reflects better what the function is used for
    • Removed the Entity parameter from the callback because it can be attained from the inspector
    • I had to expose the WorldName and TransportNode so Pose3d could use them (this isn't used by the model editor because it doesn't send commands through transport directly)
    • One display type can handle multiple component types (i.e. inspector::Pose3d works with components::Pose, components::WorldPose and components::WorldPoseCmd)

Test it

Check that all affected components are behaving as always:

  1. ign gazebo rolling_shapes.sdf
  2. See the model poses update as they roll
  3. Edit the poses and see the shapes teleported accordingly
  4. See how link poses are correct, and read-only
  5. ign gazebo sensors.sdf
  6. See the WorldPose component for the sensor entities

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

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

@chapulina chapulina added GUI Gazebo's graphical interface (not pure Ignition GUI) OOBE 📦✨ Out-of-box experience labels Mar 19, 2022
@chapulina chapulina requested a review from nkoenig March 19, 2022 02:58
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Mar 19, 2022
@chapulina chapulina mentioned this pull request Mar 19, 2022
9 tasks
@codecov
Copy link

codecov bot commented Mar 19, 2022

Codecov Report

Merging #1400 (e25fcfe) into ign-gazebo3 (64755e5) will increase coverage by 0.04%.
The diff coverage is 28.57%.

@@               Coverage Diff               @@
##           ign-gazebo3    #1400      +/-   ##
===============================================
+ Coverage        77.82%   77.86%   +0.04%     
===============================================
  Files              248      250       +2     
  Lines            14342    14352      +10     
===============================================
+ Hits             11161    11175      +14     
+ Misses            3181     3177       -4     
Impacted Files Coverage Δ
.../plugins/component_inspector/ComponentInspector.hh 28.57% <ø> (ø)
src/gui/plugins/component_inspector/Pose3d.hh 0.00% <0.00%> (ø)
.../plugins/component_inspector/ComponentInspector.cc 8.31% <36.36%> (+1.57%) ⬆️
src/gui/plugins/component_inspector/Pose3d.cc 43.47% <43.47%> (ø)

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 64755e5...e25fcfe. Read the comment docs.

@chapulina chapulina mentioned this pull request Mar 22, 2022
8 tasks
@nkoenig
Copy link
Contributor

nkoenig commented Mar 22, 2022

I'm assuming you left out the pose3d edit toggling based on the play/pause state in order to reduce the diff, correct?

@chapulina
Copy link
Contributor Author

I'm assuming you left out the pose3d edit toggling based on the play/pause state in order to reduce the diff, correct?

Oh no, that was because I'm not backporting the model editing functionality. I'm only backporting the APIs that allow us to split up the C++ code.

So the pose editing here should continue working the same way as it already does on Citadel, which is while either playing or paused, using a service call.

@chapulina chapulina merged commit 6feba08 into ign-gazebo3 Mar 23, 2022
@chapulina chapulina deleted the chapulina/3/inspector_refactoring branch March 23, 2022 17:08
chapulina added a commit that referenced this pull request Apr 13, 2022
* GzSceneManager: Prevent crash 💥 when inserted from menu (#1371)

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: ahcorde <[email protected]>

Co-authored-by: ahcorde <[email protected]>

* Prepare version 6.7.0 (#1373)

* Prepare version 6.7.0

Signed-off-by: Jose Luis Rivero <[email protected]>

* Populate GUI plugins that are empty (#1375)

Signed-off-by: Louise Poubel <[email protected]>

* Fix visualization python tutorial (#1377)

Signed-off-by: ahcorde <[email protected]>

* Add xyz and rpy offset to published odometry pose (#1341)

* Added xyz and rpy offset to published pose

Signed-off-by: Aditya <[email protected]>

* Added headless rendering tutorial (#1386)

* Added headless rendering tutorial

Signed-off-by: Nate Koenig <[email protected]>

* Update tutorials/headless_rendering.md

Co-authored-by: Louise Poubel <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>

* Update tutorials/headless_rendering.md

Co-authored-by: Louise Poubel <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>

* Mention ogre2

Signed-off-by: Nate Koenig <[email protected]>

* Added note about software rendering

Signed-off-by: Nate Koenig <[email protected]>

* add 'on linux systems'

Signed-off-by: Nate Koenig <[email protected]>

* Add xyz and rpy offset to published odometry pose (#1341)

* Added xyz and rpy offset to published pose

Signed-off-by: Aditya <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Aditya Pande <[email protected]>

* Allow to turn on/off lights (#1343)

Signed-off-by: ahcorde <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>

* Add gazebo Entity id to rendering sensor's user data (#1381)

Adds the gazebo::Entity id to a rendering::Sensor's UserData object. The main use case is so that we can get back the corresponding gazebo Entity in the rendering thread when processing sensors.

Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Don't mark entities with a ComponentState::NoChange component as modified (#1391)

Signed-off-by: Ashton Larkin <[email protected]>

* Disable ModelCommandAPI_TEST.RgbdCameraSensor on macOS (#1397)

Disabling test since it's very flaky.

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Disable PeerTracker.PeerTrackerStale on macOS (#1398)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Toggle Light visuals (#1387)

Signed-off-by: ahcorde <[email protected]>

* Prevent hanging when world has only non-world plugins (#1383)

Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Component inspector: refactor Pose3d C++ code into a separate class (#1400)

Signed-off-by: Louise Poubel <[email protected]>

* add initial_position param to joint controller system (#1406)

Similar to the <initial_position> parameter in the JointTrajectoryController system, this PR adds an <initial_position> parameter to the JointPositionController to let users specifies the initial target pos for a joint.

Signed-off-by: Ian Chen <[email protected]>

* Fix JointStatePublisher topic name for nested models (#1405)

The joint-state-publisher system currently assumes it's always attached to the top level model and hence generates an incorrect topic name for nested models. This PR updates it to generate the correct topic name.

Signed-off-by: Ian Chen <[email protected]>

* Added user command to set multiple entities (#1394)

Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>

* Disable tests that are expected to fail on Windows (#1408)

Signed-off-by: Louise Poubel <[email protected]>

* Fortress: Install Ogre 2.2, simplify docker (#1395)

Signed-off-by: Louise Poubel <[email protected]>

* SceneBroadcaster: only send changed state information for change events (#1392)

Signed-off-by: Ashton Larkin <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>

* Add wheel slip user command (#1241)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>

* Distortion camera integration test (#1374)

Signed-off-by: William Lew <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>

* Add the Model Photo Shoot system, port of Modelpropshop plugin from Gazebo classic (#1331)

This system can be used to generate thumbnails of models. In comparison to the Modelpropshop plugin in Gazebo classic, this adds the ability to generate thumbnails after randomizing the joint positions of the model.

Signed-off-by: Marco A. Gutierrez <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>

* Referring to Fuel assets within a heightmap (#1419)

Signed-off-by: Louise Poubel <[email protected]>

* Disable sensors in sensors system when battery is drained (#1385)

Added a new parameter <disable_on_drained_battery> to the sensors system. If set to true, sensors will be disabled if the model is out of battery.

It listens to the battery state of charge from the battery plugin, and if the charge reaches zero, all child sensors and nested sensors are set to be inactive.

Signed-off-by: Ian Chen <[email protected]>

* 🏁🎈 5.4.0 (#1420)

Signed-off-by: Louise Poubel <[email protected]>

* ServerConfig accepts an sdf::Root DOM object (#1333)

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Michael Carroll <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Preparing 6.8.0 release (#1425)

* Prepareing 6.8.0 release

Signed-off-by: Jose Luis Rivero <[email protected]>

* Update changelog after merging forward port

Signed-off-by: Jose Luis Rivero <[email protected]>

* Add Gaussian noise to Odometry Publisher (#1393)

* Added gaussian noise and odometry with covariance publisher

Signed-off-by: Aditya <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>

* Fix deprecation warnings for ModelPhotoShoot (#1437)

Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: ahcorde <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Aditya Pande <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Ashton Larkin <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Ivan Santiago Paunovic <[email protected]>
Co-authored-by: William Lew <[email protected]>
Co-authored-by: Marco A. Gutiérrez <[email protected]>
Co-authored-by: Michael Carroll <[email protected]>
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel GUI Gazebo's graphical interface (not pure Ignition GUI) OOBE 📦✨ Out-of-box experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants