Virtual pitot refactor + other pitot improvements

[breadoven]

Simplifies virtual pitot by removing current driver style implementation and instead runs it as a simple function. The benefit is simplified logic and removal of essentially unnecessary and repetitive code. It also avoids the illogical conversion from airspeed to pressure than back to airspeed which makes little sense when virtual pitot provides airspeed directly from GPS and wind estimates.

PR also includes changes to the pitot protothread to simplify implementation of Fake pitot and Simulator pitot. Simulator airspeed is moved and handled directly within pitotUpdate. Fake pitot pressure conversions are removed and only Fake pitot airspeed used instead. This involves moving ptYield() to the end of the thread rather than have it in the middle. It's not clear why it was placed in the middle originally other than possibly to avoid time delta issues for filtering on the first iteration. Moving it doesn't seem to affect how the pitot protothread works when tested in HITL. However, the real test will be with pitot hardware which hasn't been tested (don't have one). Pressure based airspeed calculations are skipped if Fake airspeeds are used.

Virtual airspeed is based on the Virtual pitot driver logic using -> wind estimated airspeed falling back to -> GPS 3D speed if no wind estimate and finally -> fixedWingReferenceAirspeed if no GPS available. Not sure about using fixedWingReferenceAirspeed, it may be better to only use this for a Virtual pitot and otherwise use 0.

HITL testing shows the Virtual pitot works as expected as do HITL pitot functions, i.e. pitot simulation and pitot failure. Still needs fully testing properly with actual pitot hardware.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[github-actions]

Test firmware build ready — commit 2e0ff0a

Download firmware for PR #11668

244 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

[breadoven]

@error414 Have you tested this with a real pitot by any chance ? Would be useful to double check this works with actual hardware before merging.

[error414]

@breadoven I'm on vacation now, I will return next week, I will be able to test it from 13.8. If you would like I will add this PR to my test FW.

[breadoven]

@error414 It would be useful if you could test this. The Virtual pitot stuff works fine in HITL and I tested as far as I could using the Fake pitot but the only real way of double checking actual hardware pitot is with hardware ... that I don't have.

[sensei-hacker]

Thanks for this refactor — the direction is nice (dropping the pressure round-trip for virtual pitot makes a lot of sense, and moving ptYield() to the end looks like it actually fixes a staleness issue where validation was checking last iteration's airspeed rather than the current one). A few things I wasn't sure about while reading through:

It looks like pitotIsHealthy() might stop working correctly for real sensors, because getVirtualAirspeedEstimate() now sets pitot.lastSeenHealthyMs = millis() unconditionally at the top. That field otherwise only gets touched inside the real dev.get()/dev.start() success path in pitotThread, and pitotIsHealthy() reads it as a hardware comms-timeout check. But getVirtualAirspeedEstimate() is also called from isPitotReadingPlausible() on every armed+GPS-fixed iteration (for the plausibility check against a real sensor), so it looks like it'd keep re-arming the timeout clock regardless of whether the physical sensor is still responding. Since pid.c gates coordinated-turn airspeed on pitotIsHealthy(), could this end up masking a real hardware comms failure? Would it make sense to scope that lastSeenHealthyMs stamp to just the PITOT_VIRTUAL branch in pitotUpdate(), rather than inside the shared GPS-estimate helper?

Separately, it looks like Fake/Simulator airspeed might read 0 for the first ~4 seconds after boot? The Fake/Simulator override (pitot.airSpeed = fakePitotGetAirspeed() / simulatorData.airSpeed, with usePressureCalculation = false) now runs before the pitotIsCalibrationComplete() check, and the else branch (calibration not yet complete) unconditionally does pitot.airSpeed = 0.0f without checking usePressureCalculation — so during the ~4s calibration window (pitotStartCalibration() still runs for PITOT_FAKE/simulator-backed hardware, just not PITOT_VIRTUAL), it looks like that zero would clobber the fake/sim value set moments earlier. Could that guard be added to the else branch too?

One more small thing: virtual pitot detection now requires STATE(AIRPLANE) in addition to feature(FEATURE_GPS) (previously GPS-only). Was that intentional? If so, might be worth calling out explicitly in the PR description — as written it'd silently disable virtual pitot for anyone running it on a rover/boat config after upgrading.

[breadoven]

It looks like pitotIsHealthy() might stop working correctly for real sensors, because getVirtualAirspeedEstimate() now sets pitot.lastSeenHealthyMs = millis() unconditionally at the top. ...

Good spot ! You're correct. I'll move pitot.lastSeenHealthyMs = millis() to pitotUpdate().

Separately, it looks like Fake/Simulator airspeed might read 0 for the first ~4 seconds after boot? The Fake/Simulator override (pitot.airSpeed = fakePitotGetAirspeed() / simulatorData.airSpeed, with usePressureCalculation = false) now runs before the pitotIsCalibrationComplete() check, and the else branch (calibration not yet complete) unconditionally does pitot.airSpeed = 0.0f without checking usePressureCalculation — so during the ~4s calibration window (pitotStartCalibration() still runs for PITOT_FAKE/simulator-backed hardware, just not PITOT_VIRTUAL), it looks like that zero would clobber the fake/sim value set moments earlier. Could that guard be added to the else branch too?

I need to check this again because testing in HITL with virtual pitot stops the simulator pitot options working. Could have sworn this was working before but looking at the code now there's no reason why it would given the proto thread isn't used with virtual pitot. Fix needed. As for the zero issue during calibration I wouldn't think it really matters, it's what you'd expect during calibration surely.

One more small thing: virtual pitot detection now requires STATE(AIRPLANE) in addition to feature(FEATURE_GPS) (previously GPS-only). Was that intentional? If so, might be worth calling out explicitly in the PR description — as written it'd silently disable virtual pitot for anyone running it on a rover/boat config after upgrading.

STATE(AIRPLANE) was only added since virtual pitot is really only intended for use with fixed wing so why not explicitly limit it to that. It won't realistically work for other platforms because wind estimator doesn't work for them. In that case all you get from the virtual pitot is GPS 3D speed which is available by itself anyway.