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

Compatibility for Docker on macOS #848

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

kalle264
Copy link

Description
Hey,
I want to use Webots with ROS running in Docker on macOS and not in the VM since the rest of my project runs in docker anyway.
So I made changes so that you can provide a host ip address in the launch file for WebotsLauncher and WebotsController. If you don't, they will just use the get_host_ip function like before.

webots = WebotsLauncher(
        world=PathJoinSubstitution([package_dir, 'worlds', world]),
        host_ip="host.docker.internal",
        ros2_supervisor=False
)
epuck_driver = WebotsController(
        robot_name='e-puck',
        ip_address="host.docker.internal",
        parameters=[
            {'robot_description': robot_description_path,
             'use_sim_time': use_sim_time,
             'set_robot_state_publisher': True},
            ros2_control_params
        ],
        remappings=mappings,
        respawn=True
    )

Affected Packages
List of affected packages:

  • webots_ros2_driver

Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Thank you for this very nice contribution!

I believe we should also update the documentation on how to use Webots ROS 2 on macOS with the new option you provide. Can you please make a PR with an updated version of this file and link it with this PR?

Co-authored-by: Olivier Michel <[email protected]>
@kalle264
Copy link
Author

Yes no problem, but probably not until next week.

@jyjblrd
Copy link

jyjblrd commented Oct 17, 2023

This is a nice PR. I also just ran into this issue while using parallels instead of UTM, so it would be nice to get this merged.

The current way get_host_ip "finds" the host ip address seems pretty fragile to me, im not even sure that code should be there. Perhaps just forcing the user to put in the hardcoded value would reduce confusion.

@kalle264
Copy link
Author

Hey @jyjblrd, unfortunately we switched over to Gazebo and my free time is currently also pretty full. You can contribute to the documentation if you want.

@jyjblrd
Copy link

jyjblrd commented Oct 18, 2023

Yeah no worries, I can finish it up in the next few days

@kalle264
Copy link
Author

Cool, thank you. Then I don't have to feel guilty every time I open Github :D

@brian7989
Copy link

This is very helpful. When do you expect this PR to be merged?

@jyjblrd
Copy link

jyjblrd commented Feb 12, 2024

@omichel still looking for this PR to be merged? sorry for airing this thread for so long, I've been busy but I've been using this fork for a while and I think it is very worthwhile to get on the main branch, and I have some time now

@jyjblrd
Copy link

jyjblrd commented Feb 12, 2024

#889 this PR fixes the problem for docker, but I am running a Parallels VM on my mac, which still requires the ip address to be put in manually. Happy to merge this PR in with the work done by #889

@omichel
Copy link
Member

omichel commented Feb 13, 2024

Hi, yes, please go ahead and let's merge this PR when it is fully tested and properly documented.

@krzysztofzz1
Copy link

When will it be merged to fix?

@omichel
Copy link
Member

omichel commented May 16, 2024

When will it be merged to fix?

Whenever someone fixes the failing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants