airspeed_selector: fix inconsistent throttle units #26145
Conversation
4c4a4e4 to
ffedbda
Compare
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 8 byte (0 %)]px4_fmu-v6x [Total VM Diff: 8 byte (0 %)]Updated: 2026-01-06T14:59:35 |
cherrypicked from #26219. remove when rebasing on that.
ffedbda to
f9f037b
Compare
Fixes: - False positive airspeed failures due to first principle check. This was caused by mixing different throttle units in the check: Some with battery scaling, some without. As the battery depletes, this difference can already trigger the quite sensitive check. Now it uses purely TECS setpoints again, which are without battery scaling. - Slightly wrong synthetic airspeed as battery depletes. Previously, the synthetic airspeed used thrust from vehicle_thrust_setpoint_0, which already includes battery scaling. Battery scaling is intended to offset the effects of a depleted battery, so we command a higher throttle when the battery is empty but we do not go faster. The TECS throttle setpoint more closely reflects the physical response of the vehicle and so it is preferrable to use that for synthetic airspeed. Small simplifications without changing functionality: - update throttle filter only on TECS update. This removes the need to check for data recency inside of it. Functionality is kept - in any case if synthetic airspeed is demanded outside of fixed wing we give trim airspeed. For this we add an explicit if - remove current time argument from update_throttle_filter, is already class variable
75e9c33 to
44a03e8
Compare
| float32 airspeed_derivative_filtered # [m/s^2] Filtered indicated airspeed derivative | ||
| float32 throttle_filtered # [-] Filtered fixed-wing throttle | ||
| float32 pitch_filtered # [rad] Filtered pitch | ||
| float32 calibrated_airspeed_synth_m_s # [m/s] [@invalid NaN] Synthetic airspeed |
There was a problem hiding this comment.
I think Marco and me discussed this typo before, and didn't found it severe enough to require a message definition change (as it is a versioned message it would result in us needed in message translation). We instead said to do it once we anyway have another change in that message.
There was a problem hiding this comment.
makes sense, will drop that commit
| vehicle_thrust_setpoint_0.xyz[1] * vehicle_thrust_setpoint_0.xyz[1] + | ||
| vehicle_thrust_setpoint_0.xyz[2] * vehicle_thrust_setpoint_0.xyz[2]); | ||
| } | ||
| const float throttle_sp = _tecs_status.throttle_sp; |
There was a problem hiding this comment.
I think I chose the vehicle_thrust_setpoint over the tecs_status.throttle to have the synthetic airspeed also in non-TECS-controlled modes (like stabilized).
How about we move the battery scaling from the rate controller to the allocator? Would be cleaner anyway, and solve some other issues on the way (fyi @mahima-yoga as this just today came up in another PR).
There was a problem hiding this comment.
makes sense, completely missed that possibility. will propose that solution probably in a separate PR then
Problem
When the battery scale rises significantly above 1:
ASPD_DO_CHECKS=31This is caused by mixing different throttle units in both of these places. The
_throttle_filteredinairspeed_selector_mainintroduced by #24522 is calculated fromvehicle_thrust_setpoint, which already includes battery scaling. BUT:Solution
We change the
_throttle_filteredto be based on the TECS thrust. It more closely reflect what the vehicle actually does, and correspond to the values we compare against in the first principle check.This includes some cleanups:
calibraded->calibratedget_synthetic_airspeedTesting
In simulation, with gazebo_standard_vtol (old gazebo for failure injection) and params:
param set EKF2_GPS_CTRL 0andfailure airspeed offDrawbacks
Like the current version of the synthetic airspeed, this does not handle transitions elegantly. During transition we consistently give trim airspeed. This means we accumulate some additional position error compared to the "correct" solution, which would be:
VtolTypeThe extra complexity is probably not worth it to shed at best ~tens of meters of position error, especially considering that synthetic airspeed dead reckoning is already a backup solution for the case when both global navigation and airspeed have failed, when the user is clearly warned that they should RTL.
TODO: