search

PX4 / flight_review

web application for flight log analysis & review
https://logs.px4.io/
BSD 3-Clause "New" or "Revised" License
195 stars 191 forks source link

Add plot for third instance of actuator_outputs. #204

Closed RicardoM17 closed 3 years ago

RicardoM17 commented 3 years ago

Currently when flying a vehicle that has an AUX mixer + also has UAVCAN ESCs the motors for the plots will not be plotted as they fall into actuator_outputs.02. This fixes that. It also removes actuator_outputs.00 when this one is empty (which happens in the aforementioned case).

Current:

image

Fixed:

image

This is with a log from PX4 ~V1.10. Right now it's not really easy to reproduce this as there's an issue on PX4 with logging and actuator_outputs.02. Issue: https://github.com/PX4/PX4-Autopilot/issues/16414

Still to be done, The way that the plot titles are done is not right for these edge cases. If there's only UAVCAN the main outputs will go on AUX I believe, But this could be done in a separate PR.

Signed-off-by: Ricardo Marques marques.ricardo17@gmail.com

RicardoM17 commented 3 years ago

@bkueng FYI

RicardoM17 commented 3 years ago

Thanks, this looks good. Can you combine the 3 plots into single loop?

      actuator_output_plots = [(0, "Main"), (1, "AUX"), (2, "EXTRA")]
      for topic_instance, plot_name in actuator_output_plots:
             ...

Thanks for the review @bkueng . It's a good call. I added it now, let me know if it's ok or if further changes are needed.

RicardoM17 commented 3 years ago

Thanks for the review @bkueng . It took me a few tries but it should be good to go now.

I don't know if there isn't a formatter I can use? Unfortunately the one I have configured on my IDE was formatting many other parts of the document, which made this process a bit more time consuming than usual.