Add documentation about adding new SIH airframes

[mbjd]

This took quite a bit of asking around and reverse engineering. Hopefully this will save the next person a bunch of time.

@ Reviewers, please read carefully for any inaccurate/misleading info. If unsure but there is someone else who knows this better, please request their review too.

I am not 100% certain specifically about:

• whether SENS_EN_* parameters are needed (or just recommended/usual practice)
• whether this is the right place to include things like EKF2_GPS_DELAY which is also not strictly necessary
• whether the naming template for SIH on FC is strictly necessary (and appropriate to document here), or just convention.

Thanks for any clarification.

[hamishwillee]

Thanks. I'm on holiday. Will be back and a week ish and review then.

[hamishwillee]

@mbjd Thanks for this - a very welcome addition.

whether the naming template for SIH on FC is strictly necessary (and appropriate to document here), or just convention.

That will be a convention. Fine as documented.

whether this is the right place to include things like EKF2_GPS_DELAY which is also not strictly necessary

I think it probably is the right place.

whether SENS_EN_* parameters are needed (or just recommended/usual practice)

I don't know the answer to this. It is possible that the SIH on FC implementation allows you to turn on and off the core simulated sensors using the same params as the real sensors, but it might not.

@MaEtUgR @romain-chiap Could I please get your comments on this?

[romain-chiap]

That looks good to me. I think for SIH on the FC (plugged with USB cable) we also need param set CBRK_USB_CHK 894281

Regarding SENS_EN_* I just checked in the code and they are used to start the sensor drivers as simulation. So yes, they are neeeded.

[mbjd]

@hamishwillee, thanks for the suggestions, commited most of them.

@hamishwillee @romain-chiap @MaEtUgR @RomanBapst Regarding the SENS_EN* parameters: If SYS_HITL=2, the sensor drivers are started regardless of parameters. So should we drop them here for simplicity?

Regarding the other comment about param set-default SENS_EN_ARSPDSIM 1 I think it would make sense to recommend that for airplanes and VTOL, what are your opinions?

[hamishwillee]

@mbjd You're welcome.

@hamishwillee @romain-chiap @MaEtUgR @RomanBapst Regarding the SENS_EN* parameters: If SYS_HITL=2, the sensor drivers are started regardless of parameters. So should we drop them here for simplicity?

IFF they are enabled in SIH by default for all cases we should drop them here AND in the airframe files (in the airframe files we should note in a comment that these sensors are started by default). This is captured in https://github.com/PX4/PX4-user_guide/pull/3538/files#r1926033004 and https://github.com/PX4/PX4-user_guide/pull/3538/files#r1926033512

I say IFF because it isn't clear why we only had these explicitly enabled for the SIH as SITL case? (It might be that they are enabled only by default for SIH on FC). If it is only enabled for one case then my suggestions are wrong, and we should either add SENS_EN_ARSPDSIM OR update SITL to also start these by default. I lean towards the latter, but there might be a reason why the FC/SITL cases are handled differently.

Regarding the other comment about param set-default SENS_EN_ARSPDSIM 1 I think it would make sense to recommend that for airplanes and VTOL, what are your opinions?

Since this is started by default (and just not used for QUAD) then I don't think we need to mention it other than as I have done in https://github.com/PX4/PX4-user_guide/pull/3538/files#r1926033004

[mbjd]

So, just chatted with @MaEtUgR and the situation is like this:

If running SIH-on-FC, the sensor simulations are started automatically if SYS_HITL=2, and the parameters SENS_EN* are not needed. This happens here.

If running SIH as SITL, then the sensore are only started if the corresponding SENS_EN* are set -- that happens here.

Therefore, I will keep the SENS_EN* parameters in the SIH-as-SITL section where they are needed. But I am adding a comment along the lines of this to make it clear that for SIH on FC they are not needed.

[hamishwillee]

Hi @mbjd

That's helpful, in particular the discussion with Matthias you provided above #3538 (comment)

I have subedited a bit to put the information where I think it should go. For example, to me the information that not all frame types can be simulated is quite important, so I moved that in a warning up the top. I also thought it worth mentioning how the simulated sensor stuff works when the param is first mentioned. Lastly, I thought the heading needs to be at the same level as dynamic models.

Anyway, I think this is good to go, is there anything you think we are waiting on?

[github-actions]

No flaws found

[mbjd]

@hamishwillee thanks, looks great! I agree that the fact we're not able to simulate any airframe is important enough to put up front.

Tagged @sfuhrer for a last glance over it. If he likes it we're good to go from my side.

[sfuhrer]

That's extremely helpful and looks all correct to me, thanks for the initiative @mbjd !

[romain-chiap]

Thanks everyone, it's always good to have up to date documentation!

[hamishwillee]

When @romain-chiap invented SIH on PX4 I had no idea how much traction it would end up getting. Thanks so much @mbjd for making it even more useful. Rover next, anyone, anyone :-)