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 value for plugin path in ign_gazebo launch script #125

Merged
merged 1 commit into from
Dec 8, 2020
Merged

[ros2] Add default value for plugin path in ign_gazebo launch script #125

merged 1 commit into from
Dec 8, 2020

Conversation

AndrejOrsula
Copy link
Contributor

In #122, I have unfortunately introduced a regression in case IGN_GAZEBO_SYSTEM_PLUGIN_PATH was not previously set. My mistake... I forgot to test such use case prior to submitting that PR.

This PR should fix it by having a default value in case it is not set. The same logic is applied also for LD_LIBRARY_PATH because a similar issue could occur for non-Linux systems. I am not that familiar with Python, so I am unsure if there is a more standard way of doing this.

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
@chapulina chapulina added this to Inbox in Core development via automation Dec 8, 2020
@chapulina chapulina added the ROS 2 ROS 2 label Dec 8, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Oh thanks for catching that and for the fix! I'm also not a Python expert but this looks reasonable to me 👍

Core development automation moved this from Inbox to In review Dec 8, 2020
@chapulina chapulina merged commit c454ca5 into gazebosim:ros2 Dec 8, 2020
Core development automation moved this from In review to Done Dec 8, 2020
@AndrejOrsula AndrejOrsula changed the title (ROS 2) Add default value for plugin path in ign_gazebo launch script [ros2] Add default value for plugin path in ign_gazebo launch script Dec 8, 2020
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ROS 2 ROS 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants