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

[ros2] Add default ign_gazebo version #145

Closed

Conversation

caioaamaral
Copy link

@caioaamaral caioaamaral commented Mar 26, 2021

🦟 Bug fix

Fixes #133

Summary

This change uses as the default version the dependency resolved by the package manifest (which uses ignition-gazebo3 for the foxy distro). This is accomplished by appending the flag --force-version (only when it's not already set) to the ign_args

Checklist

Note to maintainers: Remember to use Squash-Merge

@caioaamaral
Copy link
Author

caioaamaral commented Mar 26, 2021

I added to the checklist another pr which this one depends on (gazebosim/gz-tools#44)

Is a unit test desired? (maybe we could test it by using specific configurations with the ci?)

@caioaamaral
Copy link
Author

caioaamaral commented Mar 26, 2021

@chapulina I've opened the PR to the wrong branch, could you please move it to the ros2 branch please? 🤦

edit: fixed

@caioaamaral caioaamaral changed the base branch from melodic to ros2 March 26, 2021 22:08
return ign_args


def __get_ign_gazebo_default_version(): # this will fail if the package build was performed with '--merge-install'
Copy link
Author

@caioaamaral caioaamaral Mar 26, 2021

Choose a reason for hiding this comment

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

To the reviewer:

At least locally I couldn't find the package when my workspace was build with --merge-install flag, apparently the overlay does not work with such configuration (also noticed that the package manifest was not installed).

@caioaamaral caioaamaral changed the title Fix/default ign version [ros2] Add default ign_gazebo version Mar 26, 2021
@chapulina chapulina added bug Something isn't working needs upstream release Blocked by a release of an upstream library ROS 2 ROS 2 labels Mar 27, 2021
@caioaamaral caioaamaral force-pushed the fix/default-ign-version branch 2 times, most recently from dc2cba0 to dc639c8 Compare March 29, 2021 00:39
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

do you mind to fix the conflicts with the main branch ?

@caioaamaral
Copy link
Author

do you mind to fix the conflicts with the main branch ?

Done

Signed-off-by: Caio Amaral <caioaamaral@gmail.com>

Minor Fix
@caioaamaral
Copy link
Author

caioaamaral commented Apr 13, 2021

Friendly ping.

I think this one is good to go now that gazebosim/gz-tools#44 is merged

@chapulina
Copy link
Contributor

Thanks, @caioaamaral , we'll take a look soon. Note that we'll need an ign-tools release, and also upload the released version to packages.ros.org, before this can be merged.

@chapulina chapulina changed the base branch from ros2 to foxy April 27, 2021 01:20
@chapulina
Copy link
Contributor

I'm getting an error on launch, I tried compiling with --merge-install and without. Do you know what's happening?

  File "/home/chapulina/dev_focal/ws_foxy/install/ros_ign_gazebo/share/ros_ign_gazebo/launch/ign_gazebo.launch.py", line 39, in generate_lau
nch_description
    OpaqueFunction(__get_ign_args)
TypeError: __init__() takes 1 positional argument but 2 were given

@chapulina chapulina mentioned this pull request Apr 27, 2021
7 tasks
@mjcarroll mjcarroll changed the base branch from foxy to ros2 June 10, 2021 18:11
@chapulina
Copy link
Contributor

@caioaamaral , are you still working on this PR?

@caioaamaral
Copy link
Author

@caioaamaral , are you still working on this PR?

Sorry for the delay. I'll have a look by the end of this week

@chapulina
Copy link
Contributor

Hi @caioaamaral , I'm closing this due to inactivity, but feel free to reopen if you start working on it again. Thanks!

@chapulina chapulina closed this Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs upstream release Blocked by a release of an upstream library ROS 2 ROS 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Launch file may start wrong ign-gazebo version
3 participants