PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.17k stars 13.36k forks source link

MC and FW rate controllers write integrators into one topic #17174

Open le-chat opened 3 years ago

le-chat commented 3 years ago

MC and FW rate controllers write integrators into one topic rate_ctrl_status: https://github.com/PX4/PX4-Autopilot/blob/f6eae085978d7a11483156b5d4e461c875b46729/src/modules/fw_att_control/FixedwingAttitudeControl.cpp#L622 https://github.com/PX4/PX4-Autopilot/blob/f6eae085978d7a11483156b5d4e461c875b46729/src/modules/fw_att_control/FixedwingAttitudeControl.hpp#L117 https://github.com/PX4/PX4-Autopilot/blob/f6eae085978d7a11483156b5d4e461c875b46729/src/modules/mc_rate_control/MulticopterRateControl.cpp#L228 https://github.com/PX4/PX4-Autopilot/blob/f6eae085978d7a11483156b5d4e461c875b46729/src/modules/mc_rate_control/MulticopterRateControl.hpp#L107

However, for VTOLs both controllers work simultaneously. The result is that usually MC integrator wins.

Here in the two logs pitch rate error integral is not zero (that is it is from MC -- on v1.11.3 FW integrators disabled in MC mode even for tailsitters): https://review.px4.io/plot_app?log=209cb3c3-3929-468f-83ef-7f2a9f86e2ed https://review.px4.io/plot_app?log=bdb47a46-d68d-4f06-bf3d-fbb17d260f93

Possible ideal solution:

bresch commented 3 years ago

You could do something like done here: https://github.com/PX4/PX4-Autopilot/blob/06cd3eded52297ee2277163a445efed0571ac481/src/modules/fw_att_control/FixedwingAttitudeControl.cpp#L46

_controller_status_pub(vtol ? ORB_ID(rate_ctrl_status_1) : ORB_ID(rate_ctrl_status_0))

And change the multirotor publication to rate_ctrl_status_0 And define rate_ctrl_status_0 an rate_ctrl_status_1 in the message definition, like in https://github.com/PX4/PX4-Autopilot/blob/06cd3eded52297ee2277163a445efed0571ac481/msg/actuator_controls.msg#L26

dagar commented 3 years ago

You could also simply publish multiple instances by using uORB::PublicationMulti<> instead. Then in each we could add an enum to identify the source.

le-chat commented 3 years ago

@dagar They are already uORB::PublicationMulti. However, in logs we see only MC controller's data. Can added enum change this?

dagar commented 3 years ago

@dagar They are already uORB::PublicationMulti. However, in logs we see only MC controller's data. Can added enum change this?

Great, then the next step is to add some identifying information to each msg (the enum).

It looks like logger is already configured to log 2 instances. https://github.com/PX4/PX4-Autopilot/blob/5f4802a239321b9f3d1845f4e7b4262030752039/src/modules/logger/logged_topics.cpp#L130

le-chat commented 3 years ago

Ah, yes, in master rate_ctrl_status logged with add_topic_multi, it is the stable v1.11.3 where it's logged with just add_topic. How do resulting logs look in Flight Review, interestingly. Can't browse public logs with recent master and VTOL. Probably, the simplest way to check is to build a v1.11.3 with this patch and fly with it.