mit-aera / FlightGoggles

A framework for photorealistic hardware-in-the-loop agile flight simulation using Unity3D and ROS. Developed by MIT AERA group.
http://flightgoggles.mit.edu
Other
402 stars 99 forks source link

reporter.py not terminating on collision? #127

Closed lukked closed 5 years ago

lukked commented 5 years ago

I noticed that collisions cause FG to reset the drone to initial position but all nodes remain active. for example roslaunch flightgoggles scorer.launch level:=final gate_locations:=0

does not terminate on collision, but if I remember correctly, collisions should cause reporter.py to shut down and, consequently, roslaunch should be shut down as well.

Any thoughts on this? Is there any parameter that must be set?

Winter-Guerra commented 5 years ago

We have not seen this problem before. Does this happen every time? If so, could you please send us a rosbag and perhaps a dump of your Ros params?

lukked commented 5 years ago

It happens every time, no matter if I do rostopic pub -1 /uav/collision std_msgs/Empty or a real collision occurs.

This is displayed at start: [ERROR] [1553959759.306791, 0.073958]: Challenge name could not be read

Gate positions do visually seem to change.

I can't prepare a rosbag right now but here are the dumps for roslaunch flightgoggles scorer.launch level:=final gate_locations:=0 and roslaunch flightgoggles scorer.launch level:=final gate_locations:=1 dump0.yaml.txt dump1.yaml.txt

varunmurali1 commented 5 years ago

Those parameter dumps do not appear to have all the parameters. For instance I do not see a /uav/challenge_name. I also think that you possibly have an older version of these files from alphapilot. Could you please check if the following lines in your scorer.launch are in the right namespace and present in your scorer.launch?

<group ns="/uav"> 
    <rosparam command="load" file="$(find flightgoggles)/config/challenges/gate_locations_$(arg gate_locations).yaml"/>
    <rosparam command="load" file="$(find flightgoggles)/config/challenges/challenge_final.yaml"/>  
  </group>

If they are in the right namespace could you please post your scorer.launch?

lukked commented 5 years ago

Regarding challenge name: I don't know what's happening. There was this error message that I posted above.

The files below are from a collision with the ceiling right above the initial position at: roslaunch flightgoggles scorer.launch level:=final gate_locations:=0 (I drove to collision manually using teleop)

The bagfile should cover data shortly before and after collision. Image related topics are not included.

scorer.launch.txt subset.bag.zip dump_.yaml.txt

varunmurali1 commented 5 years ago

I am not entirely sure how your params do not contain /uav/challenge_name when other parameters appear to be loaded. Perhaps, could you please check that the challenge.yaml is the same as the one provided by alphapilot and contains the challenge_name.

Looking at this line, the expected behavior is that if the challenge name is not defined the node signals shutdown. If this isn't the case, i.e. reporter does not shutdown at this error perhaps the shutdown signal is not being handled. In any case, the missing parameter would stop the reporter from running and logging any events.

lukked commented 5 years ago

That's it. In a hurry I obviously I overlooked that they changed challenge_final.yaml. Thanks a lot.

BTW, without the "challenge name" line in the yaml, reporter.py is constantly running before and after collision and roslaunch doesn't shut down. I don't know what reporter.py it is doing, but the process is there.

lukked commented 5 years ago

Hi @Winter-Guerra, the problem with reporter.py not terminating when challenge_name is missing is due to calling signal_shutdown() from outside the while loop. In addition, this causes the node to ignore subsequent calls to signal_shutdown() in the callbacks (like on collision). Same applies if the timeout parameter is missing. So the the name and timeout checks should be moved inside the while loop. I can make a pull request if you like.