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

Migration to Humble #77

Merged
merged 16 commits into from
Sep 13, 2022
Merged

Migration to Humble #77

merged 16 commits into from
Sep 13, 2022

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented Aug 15, 2022

Fix #76

🛑 Package renaming 🛑

  • rmf_building_sim_gazebo_plugins -> rmf_building_sim_gz_classic_plugins
  • rmf_building_sim_ignition_plugins -> rmf_building_sim_gz_plugins
  • rmf_robot_sim_gazebo_plugins -> rmf_robot_sim_gz_classic_plugins
  • rmf_robot_sim_ignition_plugins -> rmf_robot_sim_gz_plugins

This PR also switches to unique node names required for gazebo_ros nodes in humble. It effectively makes main compatible with humble and breaks behavior with galactic builds. A separate galactic-devel branch has been created which will support galactic builds.

humble_plugins.mp4

This should be merged in after all other changes that are compatible with galactic are merged into main and then subsequently merging main into galactic-devel

Merge checklist:

Caveat

The changes here should still compile with galactic but the behaviour might be different since the node name string passed in will be processed as the namespace as seen here. I haven't tested the runtime behavior on galactic but I'd expect the node to now be namespaced and hence also the topics published and subscribed to. If that is the case, then this would break demo behavior and we should consider creating a galactic tag of this repo before merging into main.

Signed-off-by: Yadunund yadunund@gmail.com

Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
@Yadunund Yadunund requested a review from codebot August 15, 2022 10:35
Signed-off-by: Yadunund <yadunund@gmail.com>
@youliangtan
Copy link
Member

Thanks for the fix. I was facing this issue when running with humble too.

I just tested this on galactic. however, this is giving me an error printout of:

[Processing: rmf_building_sim_gazebo_plugins, rmf_robot_sim_gazebo_plugins]                                                            
--- stderr: rmf_robot_sim_gazebo_plugins                                                                                                   
/home/youliang/openrmf_ws/src/rmf/rmf_simulation/rmf_robot_sim_gazebo_plugins/src/readonly.cpp: In member function ‘virtual void ReadonlyPlugin::Load(gazebo::physics::ModelPtr, sdf::v9::ElementPtr)’:
/home/youliang/openrmf_ws/src/rmf/rmf_simulation/rmf_robot_sim_gazebo_plugins/src/readonly.cpp:63:62: error: no matching function for call to ‘gazebo_ros::Node::Get(sdf::v9::ElementPtr&, const string&)’
   63 |   _readonly_common->init(gazebo_ros::Node::Get(sdf, node_name));
      |                                                              ^
In file included from /home/youliang/openrmf_ws/src/rmf/rmf_simulation/rmf_robot_sim_gazebo_plugins/src/readonly.cpp:21:
/opt/ros/galactic/include/gazebo_ros/node.hpp:53:20: note: candidate: ‘static gazebo_ros::Node::SharedPtr gazebo_ros::Node::Get()’
   53 |   static SharedPtr Get();
      |                    ^~~
/opt/ros/galactic/include/gazebo_ros/node.hpp:53:20: note:   candidate expects 0 arguments, 2 provided
/opt/ros/galactic/include/gazebo_ros/node.hpp:87:20: note: candidate: ‘static gazebo_ros::Node::SharedPtr gazebo_ros::Node::Get(sdf::v9::ElementPtr)’
   87 |   static SharedPtr Get(sdf::ElementPtr _sdf);
      |                    ^~~
/opt/ros/galactic/include/gazebo_ros/node.hpp:87:20: note:   candidate expects 1 argument, 2 provided
/home/youliang/openrmf_ws/src/rmf/rmf_simulation/rmf_robot_sim_gazebo_plugins/src/TeleportDispenser.cpp: In member function ‘virtual void rmf_robot_sim_gazebo_plugins::TeleportDispenserPlugin::Load(gazebo::physics::ModelPtr, sdf::v9::ElementPtr)’:
/home/youliang/openrmf_ws/src/rmf/rmf_simulation/rmf_robot_sim_gazebo_plugins/src/TeleportDispenser.cpp:201:42: error: no matching function for call to ‘gazebo_ros::Node::Get(sdf::v9::ElementPtr&, const string&)’
  201 |     gazebo_ros::Node::Get(_sdf, node_name));

From here, ros_gazebo's galactic branch consists of the overload function. Yet this isn't working.

After checking further the ros_gazebo header via cat /opt/ros/galactic/include/gazebo_ros/node.hpp i noticed that api doesnt have the function:

SharedPtr Get(
    sdf::ElementPtr _sdf,
    const std::string & _defaultNamespace);

This missing api can be validated by checking the branch new rmf docker img with galactic tag.

docker run -it ghcr.io/open-rmf/rmf_deployment_template/rmf-simulation:galactic
cat /opt/ros/galactic/include/gazebo_ros/node.hpp

In this case, I believe the apt bin package for gazebo_ros isnt up to date?

@Yadunund
Copy link
Member Author

Yadunund commented Aug 17, 2022

Thanks for trying this out @youliangtan! I guess the changes in galactic have not been released yet.

I suspect even when it does get released, namespacing the nodes would lead to other problems. I just checked and there is already a galactic branch for this repo. So I'm thinking if the best option we have is to first merge the latest main into galactic. And then merge this into main while acknowledging that main will no longer build with galactic.

Edit: galactic branch is for cutting releases. Have created galactic-devel to merge non-breaking changes into from main.

luca-della-vedova and others added 7 commits August 25, 2022 18:37
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
@Yadunund
Copy link
Member Author

Dispensers now work 🎉

@Yadunund Yadunund changed the title Use unique names for plugins (humble fix) Migration to Humble Aug 25, 2022
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
vcs-repo-file-url: |
https://raw.githubusercontent.com/open-rmf/rmf/main/rmf.repos
https://raw.githubusercontent.com/open-rmf/rmf/humble-image/rmf.repos
Copy link
Member

Choose a reason for hiding this comment

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

Reminder to self, revert this

Copy link
Member Author

Choose a reason for hiding this comment

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

Its one of the checkboxes in the PR description.

Signed-off-by: Yadunund <yadunund@gmail.com>
Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

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

Alright let's do it! Will make a followup PR to change the github action back to main once the main RMF repo is updated.

@luca-della-vedova luca-della-vedova enabled auto-merge (squash) September 8, 2022 05:13
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gz-classic plugins crash when running with Humble
4 participants