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

Merge ign-gazebo3 ➡️ ign-gazebo6 #2013

Merged
merged 7 commits into from
Jun 16, 2023
Merged

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Jun 13, 2023

➡️ Forward port

Port ign-gazebo3 ➡️ ign-gazebo6

Branch comparison: ign-gazebo6...ign-gazebo3

Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)

azeey and others added 6 commits May 2, 2023 23:14
…usly (gazebosim#1962)

The main reason that the Resource Spawner took a long time to load is that it tried to fetch the list of all available models on Fuel instead of just the selected owner. And it did that while blocking the Qt thread, so the user was unable to interact with the GUI while the model list was being fetched. The approach I've taken is to only fetch the list of models for the default owner ("openrobotics") and allow users to add/remove any owner they want. The fetching is also done in a separate thread so as to not block the GUI.
---------

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
* Include gz/msgs.hh instead of ignition
* Install components.hh to gz/sim

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
…ts on a topic (gazebosim#1994)

* Working on marker topic

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

* Fixed build

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

* Fixed test

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

* Cleanup

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

* codecheck

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

---------

Signed-off-by: Nate Koenig <natekoenig@gmail.com>
…in a `<world>` (gazebosim#1998)

Signed-off-by: Henrique-BO <henrique.barrosoliveira@usp.br>
@azeey azeey self-assigned this Jun 13, 2023
@osrf-triage osrf-triage added this to Inbox in Core development Jun 13, 2023
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jun 13, 2023
@mjcarroll
Copy link
Contributor

Does this require an upstream release?

 [ 39%] Building CXX object src/gui/plugins/resource_spawner/CMakeFiles/ResourceSpawner.dir/ResourceSpawner.cc.o
  /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc: In lambda function:
  /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc:728:67: error: no matching function for call to 'ignition::fuel_tools::FuelClient::Models(ignition::fuel_tools::ModelIdentifier&, bool)'
             for (auto iter = worker.fuelClient.Models(modelId, false);
                                                                     ^
  In file included from /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.hh:27,
                   from /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc:18:
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:113:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ServerConfig&)'
         public: ModelIter Models(const ServerConfig &_server);
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:113:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:124:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ServerConfig&) const'
         public: ModelIter Models(const ServerConfig &_server) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:124:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:157:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ModelIdentifier&)'
         public: ModelIter Models(const ModelIdentifier &_id);
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:157:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:166:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ModelIdentifier&) const'
         public: ModelIter Models(const ModelIdentifier &_id) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:166:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:172:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::CollectionIdentifier&) const'
         public: ModelIter Models(const CollectionIdentifier &_id) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:172:25: note:   candidate expects 1 argument, 2 provided

@azeey
Copy link
Contributor Author

azeey commented Jun 13, 2023

Does this require an upstream release?

 [ 39%] Building CXX object src/gui/plugins/resource_spawner/CMakeFiles/ResourceSpawner.dir/ResourceSpawner.cc.o
  /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc: In lambda function:
  /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc:728:67: error: no matching function for call to 'ignition::fuel_tools::FuelClient::Models(ignition::fuel_tools::ModelIdentifier&, bool)'
             for (auto iter = worker.fuelClient.Models(modelId, false);
                                                                     ^
  In file included from /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.hh:27,
                   from /github/workspace/src/gui/plugins/resource_spawner/ResourceSpawner.cc:18:
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:113:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ServerConfig&)'
         public: ModelIter Models(const ServerConfig &_server);
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:113:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:124:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ServerConfig&) const'
         public: ModelIter Models(const ServerConfig &_server) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:124:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:157:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ModelIdentifier&)'
         public: ModelIter Models(const ModelIdentifier &_id);
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:157:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:166:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::ModelIdentifier&) const'
         public: ModelIter Models(const ModelIdentifier &_id) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:166:25: note:   candidate expects 1 argument, 2 provided
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:172:25: note: candidate: 'ignition::fuel_tools::ModelIter ignition::fuel_tools::FuelClient::Models(const ignition::fuel_tools::CollectionIdentifier&) const'
         public: ModelIter Models(const CollectionIdentifier &_id) const;
                           ^~~~~~
  /usr/include/ignition/fuel_tools7/gz/fuel_tools/FuelClient.hh:172:25: note:   candidate expects 1 argument, 2 provided

It does. I'll make one shortly.

@azeey azeey mentioned this pull request Jun 13, 2023
7 tasks
@azeey
Copy link
Contributor Author

azeey commented Jun 14, 2023

@osrf-jenkins run tests please

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Merging #2013 (94801e5) into ign-gazebo6 (dfb7039) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

❗ Current head 94801e5 differs from pull request most recent head c911f69. Consider uploading reports for the commit c911f69 to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #2013      +/-   ##
===============================================
- Coverage        65.38%   65.37%   -0.02%     
===============================================
  Files              327      327              
  Lines            26993    27005      +12     
===============================================
+ Hits             17649    17654       +5     
- Misses            9344     9351       +7     
Impacted Files Coverage Δ
src/gz.cc 64.13% <0.00%> (-0.71%) ⬇️
src/rendering/MarkerManager.cc 13.69% <66.66%> (+0.66%) ⬆️
src/Server.cc 90.19% <100.00%> (+0.26%) ⬆️

... and 2 files with indirect coverage changes

@nkoenig
Copy link
Contributor

nkoenig commented Jun 14, 2023

Is the windows failure part of the homebrew proto upgrade issue?

@azeey
Copy link
Contributor Author

azeey commented Jun 14, 2023

Is the windows failure part of the homebrew proto upgrade issue?

No, it's a test failure from a test that was added recently (in #1998). That PR targetted ign-gazebo3 which doesn't have windows CI.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Copy link
Contributor Author

azeey commented Jun 14, 2023

I've updated the test in Server_TEST.cc
from

  EXPECT_FALSE(*server.Running(0));

to

  EXPECT_FALSE(server.Running(0).has_value());

because server.Running(0) will be returning a nullopt since there is no world at index 0 (the SDF doesn't contain one) and dereferencing a nullopt is undefined behavior.

I've also updated the branch to contain the latest PR added to ign-gazebo6.

Copy link
Contributor

@nkoenig nkoenig left a comment

Choose a reason for hiding this comment

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

This looks fine. I'll leave CI determination up to you.

Core development automation moved this from Inbox to In review Jun 16, 2023
@azeey
Copy link
Contributor Author

azeey commented Jun 16, 2023

The test failure on Ubuntu seems to also occur on the stable branch (https://build.osrfoundation.org/view/ign-fortress/job/ignition_gazebo-ci-ign-gazebo6-focal-amd64/178/) . The windows warnings are also (https://build.osrfoundation.org/view/ign-fortress/job/ign_gazebo-ign-6-win/177) even though they seem to disappear in newer builds. I think that's probably due to the workspace not being cleared (gazebo-tooling/release-tools#724 (comment)). Merging!

@azeey azeey merged commit 4ce01ea into gazebosim:ign-gazebo6 Jun 16, 2023
6 of 9 checks passed
Core development automation moved this from In review to Done Jun 16, 2023
@azeey azeey deleted the 3_to_6 branch June 16, 2023 19:08
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.

None yet

5 participants