ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
452 stars 288 forks source link

👩‍🌾 [Proposal] No warning for missing ROS 1 #246

Closed chapulina closed 4 years ago

chapulina commented 4 years ago

This warning keeps coming up on ROS 2's CI, but if it's not something that needs to be fixed, I'd propose to turn into an informational message instead of a warning (which prompts action).

I'm not sure if this has been discussed in the past, and I can see an argument for keeping it as it is. I was just wondering what people thought.

dirk-thomas commented 4 years ago

I don't think this should be merged. The warning is there for a reason: if a user tries to build this package and ROS 1 can't be found there needs to be a warning.

Instead the CI builds showing this warning should be updated to not build ros1_bridge - or setup a ROS 1 environment if that is desired (which is actually the goal for the referenced job).

chapulina commented 4 years ago

Got it, makes sense.

It sounds like this package can't be used without ROS1. Then in this case, shouldn't this be an error instead?

dirk-thomas commented 4 years ago

Since the repo is part of the ros2.repos file which we recommend in the ROS 2 build-from-source instructions I don't think a FATAL_ERROR which will essentially fail the build is an option.