PX4 / PX4-Autopilot

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

Airspeed selector: un-initialized airspeed validators #17754

Open pjdewitte opened 3 years ago

pjdewitte commented 3 years ago

Describe the bug When an airspeed sensor becomes available more than 2 seconds after system boot, the corresponding airspeed validator does not get the correct parameters until a parameter update occurs.

As a result, ASPD_SCALE_EST is not honored, and the aircraft starts to estimate the airspeed scale while it shouldn't. In our case, the estimate was sometimes wrong, so that we had to abort flights to avoid stalling or worse.

We believe this is especially relevant for UAVCAN-based sensors. We also get 2 warnings shortly after each other directly after boot, showing the airspeed selector started before the sensor was available:

[airspeed_selector] Airspeed: switched from sensor (...) to -1
[airspeed_selector] Airspeed: switched from sensor -1 to 1

To Reproduce Steps to reproduce the behavior:

  1. Use the settings ASPD_SCALE_EST = 0 and ASPD_SCALE = 1
  2. Connect an airspeed sensor after boot, e.g. by plugging it in on the UAVCAN network
  3. Fly in forward flight
  4. Update any parameter
  5. Observe that in the logged messages wind_estimate.tas_scale (of the relevant airspeed selector wind estimator) is not 1 but is estimated in-flight, until any parameter was updated.

Note: this behaviour was observed with v1.11.3, but I have no reason to think this behaviour has changed since then.

Expected behavior wind_estimate.tas_scale should be equal to ASPD_SCALE when ASPD_SCALE_EST is 0.

Log Files and Screenshots

The screenshot below shows the TAS scale from the relevant wind estimator. In-flight, we switched back to hover and adjusted a parameter before continuing. After that, the scale was correct.

image

Drone: Atmos Marlyn: tailsitter VTOL

Possible fix We're happy to make a pull request with the fix that works for us: In AirspeedModule::update_params(), loop over all validator instances (MAX_NUM_AIRSPEED_SENSORS), instead of only the ones for which there currently is an airspeed sensor (_number_of_airspeed_sensors).

Alternatively, AirspeedModule::update_params() could be called when an airspeed sensor is added. Personally I think it is better to initialize all the airspeed validators as soon as they exist, as that is more robust.

We also increased the initial timeout before the airspeed validator starts working, to avoid the warning messages right after boot. For our airspeed sensor it takes 4.5s to become available. If anyone can think of a more generic solution, we're happy to contribute to that as well.

sfuhrer commented 3 years ago

Thanks for the report.

Connect an airspeed sensor after boot, e.g. by plugging it in on the UAVCAN network

Not sure how much of a general use case it is to plug in a sensor after boot. Is there a reason why you need to do it? If not, then I would assume that increasing the timeout before starting the selector to e.g. 5s would fix it for you, no?

pjdewitte commented 3 years ago

Connect an airspeed sensor after boot, e.g. by plugging it in on the UAVCAN network

I included this as an easy way to reproduce; I agree that it is not a common use case. (I can imagine situations though, e.g. when you plug in devices one after another to distinguish between 2 identical sensors, so you can give them a fixed UAVCAN node id.)

Because the airspeed validator is structured such that airspeed sensors can become available and unavailable, I think it should handle this without edge cases. Increasing the timeout to 5s would make it work for our current setup, but I don't consider that to be the best and most robust solution.

In the end, in addition to the MAX_NUM_AIRSPEED_SENSORS solution, we choose to replace the 2s timeout by a check that only runs the airspeed validator when we are pre-armed. I guess that is not generally applicable though.