Commander: start pulling arming related parts into separate folder

[MaEtUgR]

Describe problem solved by this pull request
Goal: The user knows in advance if he'll be able to arm.

Describe your solution
First step: Pull things that relate to arming logic into the folder Arming and make sure to have a clear interface for all checks that happen before arming can take place

Describe possible alternatives
Hack my way through commander trying to run important bits and pieces in a random and unmaintainable way.

Test data / coverage
Only draft yet but I'm constantly testing every single step by using the existing unit-sitl tests and my own sitl testing. Hopefully I try to get to the point where efficient gtest unit testing is possible.

Additional Context
#7055

[jlecoeur]

Goal: The user knows in advance if he'll be able to arm.

That is a revolution

[ndepal]

Related? #13025 #12989

[MaEtUgR]

@ndepal Definitely related. Thanks for referencing. I can rebase either way no matter which one goes first.

[dagar]

Glad to see some movement here!

One thing I'd like to get sorted out is the line between preflight, prearm, and checks that can run continuously.

I'm wondering if we can get to the point where nearly everything results in status flags and have the arming machine act on that data rather than actually executing the checks.

The other missing piece is fitting this into mavlink.

[dagar]

Perhaps not what you want to tackle here, but I'm wondering if all this sensors code should be owned by the sensors module. Then a higher level sensors status would be published for commander to act on.

[MaEtUgR]

@dagar Thanks for looking over it and your input! I try to share as many thoughts as possible. I want to first get it all in a less interdependent and hence more maintainable adaptable state.

I'd like to get sorted out is the line between preflight, prearm

My intentions after talking to a bunch of people about this: Preflight checks are the ones that determine if you can fly or not (user-focused), the pre arm checks (I left the name for now but it might not make sense anymore) are just one kind of preflight check since they prevent arming and hence flying.

and checks that can run continuously.
I'm wondering if we can get to the point where nearly everything results in status flags and have the arming machine act on that data rather than actually executing the checks.

Exactly the intention and why I started refactoring this. The goal is that there is no check that was not done before giving an arm command/input, no surprises to the user anymore. The arming state machine itself just knows if there's green light.

The other missing piece is fitting this into mavlink.

I agree, first I have to leave the strings but have them all optional (to not spam), then I plan to fill the flags at the beginning of https://mavlink.io/en/messages/common.html#SYS_STATUS as in-between solution and eventually we'll have the events interface taking care of delivering the status to the UI.

Perhaps not what you want to tackle here, but I'm wondering if all this sensors code should be owned by the sensors module. Then a higher level sensors status would be published for commander to act on.

I thought exactly the same let's go in steps.

My currently intended steps:

• pull all checks together into PreFlightCheck into as small of an include and symbol scope as possible, dependencies get clear
• make sure checks are callable without output and with one single call covering all desired checks
• refactor code of checks, I saw they add up to >1200 lines of code which is almost not possible without duplication and there are better places for certain checks e.g. where the data of interest is produced or consumed

This PR should only do the first point otherwise it gets too big of a change.

[MaEtUgR]

@dagar @bresch I rebased on master with the only relatively trivial conflict of these lines:
549fb0d#diff-a2bcb238e25d8a97ab2aee48f54dbc48R497-R513

It would be nice to get this in soon. I can rebase other conflicting prs easily.

[dannyfpv]

Tested on Pixhawk4 v5 f-450 & Pixhawk v4pro f-450
Preflight fail: heading estimate not stable, unable to arm.

change to master firmware no issues

[MaEtUgR]

Preflight fail: heading estimate not stable, unable to arm.

This was a bug introduced with #13036 and fixed in #13261 and #13267. I was unlucky to have master at the exact time it was broken as my base. I'll rebase and this will fix the problem.

[MaEtUgR]

Thanks for the reviews and comments also from @bkueng.
I rebased, removed the funtional commit since like @dagar mentioned they don't really help by themselfes yet and addressed all the comments about includes with respect to folders in the last commit.

[MaEtUgR]

I'm assuming CI problems are now unrelated. It's ready for review check.

[dagar]

Looks good to me. Can you rebase?

[MaEtUgR]

Rebased
The conflict was just these four space characters: a475d71#diff-eb94b84d1c6e5b1530f8ad834cef9b22R3761

[MaEtUgR]

Fixed style using rebase in https://github.com/PX4/Firmware/compare/856bca21538ee02c66fde813d666d3eebac6e75d..fbaee7624805c46309318d20fa2815ad34dcdc1b

[MaEtUgR]

Upon request of @dagar I did a quick test on a real vehicle with a pixhawk 4 mini. I can normally arm and fly when all checks pass. When the vehicle is upside down I can't arm because of the failing roll check.