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 inherited model topic name. #1689

Merged
merged 2 commits into from
Sep 22, 2022
Merged

Enable inherited model topic name. #1689

merged 2 commits into from
Sep 22, 2022

Conversation

bperseghetti
Copy link
Member

@bperseghetti bperseghetti commented Sep 4, 2022

🦟 Bug fix

Fixes #1688

Summary

Allows for inheriting model name for robotNamespace when SDF element is not set and provides a debug message showing the topics it subscribes to.

image

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.

@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 4, 2022
@jgoppert
Copy link
Contributor

jgoppert commented Sep 6, 2022

It would be great to see this merged. It will streamline our workflow with the PX4 autopilot.

@bperseghetti
Copy link
Member Author

Example of the updated MulticopterMotorModel in use:
px4sim0

@bperseghetti
Copy link
Member Author

More examples of it using the resource spawner when multiple vehicles are in use and the name dynamically changes:
px4sim1

@bperseghetti
Copy link
Member Author

@chapulina @nkoenig @mjcarroll If this PR was to get approved what might be the earliest timeline to seeing it in a fortress update/release?

We are hoping to use this in several student competitions in less than a month. The only way we can migrate away from classic gazebo for multi-vehicle workflow (generation through both resource spawner and autopilot generation) requires this very minor change (I can not think of a way where it would interfere with any existing usage of the plugin).

If you would like to try playing with it and the PX4 Autopilot:

  1. Clone PX4:
    git clone -b pr-gazebo-motor-update [email protected]:PX4/PX4-Autopilot.git --recursive

  2. Install PX4 build toolchain (assuming on 22.04):

cd PX4-Autopilot/Tools/setup
./ubuntu.sh
  1. Build and start simulation from PX4 binary:
#Depending on your gazebo build may require: export IGN_CONFIG_PATH=/usr/local/share/ignition/
cd <your path>/PX4-Autopilot
make px4_sitl_ign
PX4_SYS_AUTOSTART=4001 PX4_GZ_MODEL=x500-Depth ./build/px4_sitl_ign/bin/px4
commander arm -f #spins motors after system says ready for takeoff
  1. In another terminal start a new PX4 instance using same model to spawn but different pose offset in x:
#Depending on your gazebo build may require: export IGN_CONFIG_PATH=/usr/share/ignition/
PX4_SYS_AUTOSTART=4001 PX4_GZ_MODEL_POSE="2" PX4_GZ_MODEL=x500-Depth ./build/px4_sitl_ign/bin/px4 -i 1
commander arm -f #spins motors after system says ready for takeoff
  1. Open resource spawner in gazebo gui, click and drag the x500-Depth to where you want it. In another new terminal connect PX4 to the dynamically spawned model by passing it's entity name in simulation to PX4_GZ_MODEL_NAME (assuming it should just be x500-Depth if following this guide):
#Depending on your gazebo build may require: export IGN_CONFIG_PATH=/usr/share/ignition/
PX4_SYS_AUTOSTART=4001 PX4_GZ_MODEL_NAME=x500-Depth ./build/px4_sitl_ign/bin/px4 -i 2
commander arm -f #spins motors after system says ready for takeoff
  1. In another terminal start a new PX4 instance using model x500 to spawn but full pose and yawed 90 degrees:
#Depending on your gazebo build may require: export IGN_CONFIG_PATH=/usr/share/ignition/
PX4_SYS_AUTOSTART=4001 PX4_GZ_MODEL_POSE="2,2,0,0,0,1.57" PX4_GZ_MODEL=x500 ./build/px4_sitl_ign/bin/px4 -i 3
commander arm -f #spins motors after system says ready for takeoff

All of these used the same base model (x500-Base) that included the updated MulticopterMotorModel with robotNamespace removed: https://app.gazebosim.org/RudisLaboratories/fuel/models/x500-Base

This allows for light and unique model generation to rapidly test different sensors for CI and general simulation purposes as seen in the other "include style" models:

<include merge='true'>
  <uri>https://fuel.gazebosim.org/1.0/RudisLaboratories/models/x500-Base</uri>
</include>

@mjcarroll
Copy link
Contributor

If this PR was to get approved what might be the earliest timeline to seeing it in a fortress update/release?

Basically as soon as we can get it merged, I can do a release for you to get it in binaries. I will start reviewing now.

@bperseghetti
Copy link
Member Author

If this PR was to get approved what might be the earliest timeline to seeing it in a fortress update/release?

Basically as soon as we can get it merged, I can do a release for you to get it in binaries. I will start reviewing now.

Awesome, we are really excited to use gazebo, will this change also work it's way into garden?

@mjcarroll
Copy link
Contributor

Awesome, we are really excited to use gazebo, will this change also work it's way into garden?

Garden is currently frozen (apart from critical bug fixes) as we do some QA through the end of the month. After that, features from fortress will be forward ported before the next minor release.

@mjcarroll mjcarroll self-assigned this Sep 7, 2022
@bperseghetti
Copy link
Member Author

@nkoenig just tested your suggestion, works great. Just resigned it for DCO and pushed.

bperseghetti and others added 2 commits September 8, 2022 12:34
Signed-off-by: Benjamin Perseghetti <[email protected]>
Update to remove leading model name.

Co-authored-by: Nate Koenig <[email protected]>
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
Copy link
Member Author

Rebased on current ign-gazebo6 to hopefully fix the triggeredcamera test issues.

@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #1689 (d711e1b) into ign-gazebo6 (69e7757) will increase coverage by 0.03%.
The diff coverage is 82.19%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1689      +/-   ##
===============================================
+ Coverage        64.69%   64.73%   +0.03%     
===============================================
  Files              321      321              
  Lines            26062    26124      +62     
===============================================
+ Hits             16862    16912      +50     
- Misses            9200     9212      +12     
Impacted Files Coverage Δ
.../systems/triggered_publisher/TriggeredPublisher.hh 100.00% <ø> (ø)
...s/multicopter_motor_model/MulticopterMotorModel.cc 75.72% <33.33%> (-0.22%) ⬇️
.../systems/triggered_publisher/TriggeredPublisher.cc 83.28% <84.28%> (-0.38%) ⬇️

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

@bperseghetti bperseghetti requested review from nkoenig and removed request for mjcarroll September 8, 2022 20:55
@bperseghetti
Copy link
Member Author

@mjcarroll sorry, I did not mean to remove you as a reviewer, I meant to just also add @nkoenig but seemed to remove you when I did.

@bperseghetti
Copy link
Member Author

@nkoenig @mjcarroll is there anything else desired to be changed in this PR?

@mjcarroll mjcarroll merged commit 6b48eea into gazebosim:ign-gazebo6 Sep 22, 2022
@bperseghetti
Copy link
Member Author

If this PR was to get approved what might be the earliest timeline to seeing it in a fortress update/release?

Basically as soon as we can get it merged, I can do a release for you to get it in binaries. I will start reviewing now.

@mjcarroll @nkoenig is there a timing on when 6.13 will get released?

nkoenig added a commit that referenced this pull request Nov 2, 2022
* Add topic parameter to thrust plugin (#1681)

* Add topic parameter.

Signed-off-by: Carlos Agüero <[email protected]>

* 🎈 6.12.0: bumped minor and updated changelog (#1682)

* bumped minor and updated changelog

Signed-off-by: Dharini Dutia <[email protected]>

* fixed changelog as per feedback and updated migration guide

Signed-off-by: Dharini Dutia <[email protected]>

Signed-off-by: Dharini Dutia <[email protected]>

* Fix reference link in ackermann steering (#1683)

Signed-off-by: Kenji Brameld <[email protected]>

* Fix installation instructions on Ubuntu 22.04 (#1686)

Signed-off-by: Silvio Traversaro <[email protected]>

* Add a service to trigger functionality (#1611)

* initial commit to allow plugin to call a service

Signed-off-by: Liam Han <[email protected]>

* adding tutorial and modifying the world sdf

Signed-off-by: Liam Han <[email protected]>

* added test for single input and single service output

Signed-off-by: Liam Han <[email protected]>

* added test for single input and multiple service output

Signed-off-by: Liam Han <[email protected]>

* added test for invalid matching service name => timeout

Signed-off-by: Liam Han <[email protected]>

* modified variables the camelCase

Signed-off-by: Liam Han <[email protected]>

* fixed typo, indentation, grammar, lines that exceeded 80 char

Signed-off-by: Liam Han <[email protected]>

* fixing ubuntu bionic ci issue

Signed-off-by: Liam Han <[email protected]>

* silly syntax mistake on expect_eq

Signed-off-by: Liam Han <[email protected]>

* added three more test cases that addesses incorrect response type, incorrect request type and false result

Signed-off-by: Liam Han <[email protected]>

* WIP: major restructuring and currently working. Requires more cleanup and test

Signed-off-by: Liam Han <[email protected]>

* WIP: fixed preprocessor define bug

Signed-off-by: Liam Han <[email protected]>

* WIP: working but extremely convoluted

Signed-off-by: Liam Han <[email protected]>

* WIP major modification but a lot of errors and tests failed

Signed-off-by: Liam Han <[email protected]>

* stable version: had to revert back to previous work. all tests passed

Signed-off-by: Liam Han <[email protected]>

* modified to use blocking Request method as well as reduce a service worker thread to just one thread with the publisher. all tests passed

Signed-off-by: Liam Han <[email protected]>

* stable version: had to revert back to previous work. all tests passed

Signed-off-by: Liam Han <[email protected]>

* successfully reverted and tested

Signed-off-by: Liam Han <[email protected]>

* fixing PR suggestions

Signed-off-by: Liam Han <[email protected]>

* changed string with 'serv' to 'srv' and included <mutex> to the header

Signed-off-by: Liam Han <[email protected]>

* fixed indentation and removed rep.set_data since it's unused on the client service

Signed-off-by: Liam Han <[email protected]>

* getting rid of the id

Signed-off-by: Liam Han <[email protected]>

* fixed race condition resulting seldom test failure

Signed-off-by: Liam Han <[email protected]>

* changed from triggerSrv to serviceCount. This compensates for the two threads running at different rate

Signed-off-by: Liam Han <[email protected]>

* braces indentation

Signed-off-by: Mabel Zhang <[email protected]>

* addressing gnu c compiler (gcc) warnings

Signed-off-by: Liam Han <[email protected]>

Signed-off-by: Liam Han <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Co-authored-by: Mabel Zhang <[email protected]>

* Fix loading render engine plugins in GUI  (#1694)

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

* Enable inherited model topic name. (#1689)

Allows for inheriting model name for robotNamespace when SDF element is not set and provides a debug message showing the topics it subscribes to.

Signed-off-by: Benjamin Perseghetti <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>

* Add ResourceSpawner example file (#1701)

Add an example file for the ResourceSpawner plugin. I'm using this to link from https://github.com/gazebosim/docs/blob/master/garden/Model_insertion_fuel.md. To improve gazebosim/garden-tutorial-party#1991.

Signed-off-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Update triggered_publisher.sdf (#1737)

found a silly typo that was pushed back in PR (#1611)

* Adds sky cubemap URI to the sky.proto's header (#1739)

* Adds sky cubemap URI to the sky.proto's header

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

* require sdf 12.6

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

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

* Return absolute path when finding a resource (#1741)

* Adds sky cubemap URI to the sky.proto's header

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

* Return absolute path when finding a resource

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

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

* Restore Add System GUI plugin (#1685)

* cherry pick aef3020

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

* Adding thrust coefficient calculation (#1652)

* adding thrust coefficient calculation

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

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.cc

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* Update src/systems/thruster/Thruster.hh

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>

* thrust coefficient test and behavior updates

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

* making float comparision more robust

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

* fix float comparision and lint

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

Signed-off-by: Marco A. Gutierrez <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>

* Enable/Disable individual hydrodynamic components. (#1692)

This commit enables and disables individual components of the hydrodynamics. This is often useful for debugging odd behaviours of a hydrodynamic model.

* Fortress: Removed warnings (#1754)

* Fortress: Removed warnings

* Removed unused speedlimit file (#1761)

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

* Enable use of ign gazebo -s on Windows (take two) (#1764)

* Enable use of ign gazebo -s on Windows

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

* Update src/CMakeLists.txt

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

* Fix cmdmodel6.rb and cmdgazebo6.rb contining the same code

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

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

* Script and tutorial for generating procedural datasets with Blender (#1412)

Signed-off-by: Andrej Orsula <[email protected]>

* Fix scene_broadcaster_system test (#1766)

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

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

* Lint

Signed-off-by: Michael Carroll <[email protected]>

* Clean build and update test

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

Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: Dharini Dutia <[email protected]>
Signed-off-by: Kenji Brameld <[email protected]>
Signed-off-by: Silvio Traversaro <[email protected]>
Signed-off-by: Liam Han <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Benjamin Perseghetti <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Marco A. Gutierrez <[email protected]>
Signed-off-by: ahcorde <[email protected]>
Signed-off-by: Silvio <[email protected]>
Signed-off-by: Andrej Orsula <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Co-authored-by: Carlos Agüero <[email protected]>
Co-authored-by: Dharini Dutia <[email protected]>
Co-authored-by: Kenji Brameld <[email protected]>
Co-authored-by: Silvio Traversaro <[email protected]>
Co-authored-by: Liam Han <[email protected]>
Co-authored-by: Mabel Zhang <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Benjamin Perseghetti <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Marco A. Gutierrez <[email protected]>
Co-authored-by: Arjo Chakravarty <[email protected]>
Co-authored-by: Andrej Orsula <[email protected]>
Co-authored-by: Michael Carroll <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

MulticopterMotorModel does not inherit model name when robotNamespace is not set.
4 participants