ros_comm
ros_comm copied to clipboard
Return the exit code a required process through roslaunch
Return the exit code to the caller of a required process at least for a regular parent launch process, child and remote processes need looking at.
May solve https://github.com/ros/ros_comm/issues/919
Made some cleanup and existing tests pass.
A new test that launches a node with required=true then forces it to exit non-zero, then inspects roslaunch exit code is the next step. Any pointers to an existing roslaunch test that would be a good template for this would be very welcome, otherwise I'll figure it out eventually.
I'll hold on to the remaining extra prints until I'm done with that, a few I think it makes sense to keep.
Just tested this patch ported to melodic. Works for me!
I did notice that the roslaunch xml node has to be marked as required="true"
, which is logical. Just wanted to point that out, as it threw me for a loop for a second.
1612214530.892673: Code shutdown requested: 37
================================================================================REQUIRED process [host/exit_code_node-1] has died!
process has died [pid 24194, exit code 37, cmd /root/src/core/nodes/exit_code_node.py __name:=exit_code_node __log:=/root/.ros/log/906b1440-649f-11eb-8a83-78d00426c726/-exit_code_node-1.log].
log file: /root/.ros/log/906b1440-649f-11eb-8a83-78d00426c726/-exit_code_node-1*.log
Initiating shutdown!
================================================================================
Going to exit due to code 37
[host/exit_code_node-1] killing on exit
shutting down processing monitor...
... shutting down processing monitor complete
roslaunch runner done
ending server mode 37
exiting from main() 37
root@remote:~# echo $?
37
would love to see this in melodic. Is there something I can do to help get this merged?
Added a unit test, can run manually with
catkin run_tests
rostest test_roslaunch roslaunch.test
Can the requires-changes
label be removed if there are no more changes needed?