Make master process explicitly 'required' for parent launch#1228
Make master process explicitly 'required' for parent launch#1228dirk-thomas merged 1 commit intoros:lunar-develfrom
Conversation
|
Maintainers, any thoughts on this change? This seems like sensible default behaviour, but if it's not I'm happy to put it behind a command line arg. |
|
I am travelling for the next couple weeks so it might be a while before I can take a look at this. |
|
I can't think of any scenario where this wouldn't be desirable behaviour— I guess with the one caveat that the master isn't actually required once the system is up and running if there are no new topics coming up after initial setup. But a system in that state is pretty minimally usable, since you won't be able to inspect much of anything. That said, I'm curious what's crashing your rosmaster. Do you have core dumps from that that are shareable, or a MWE? In parallel to this change, it would be great to figure out the underlying cause of the crash in what should be a relatively simple, stable component. In any case, 👍 from me. |
|
Sadly the errors were transient, so we don't have an MWE. Hopefully with the patched behaviour, it'll be easier to spot a downed roscore and collect artifacts for reproduction. |
|
Hello maintainers, any further thoughts on this patch? |
|
Ping for review |
|
Thank you the patch. It sounds reasonable to me. |
In the existing implementation, if
rosmasterdies, the parent launch will continue running unaffected.This is particularly unpleasant when the launch is a standalone
roscore. Adding the required argument here causes any roslaunch process manager that brings uprosmasterto also come down ifrosmasterdies unexpectedly.