Skip to content

ADS-B: newly assigned vehicle slot can briefly carry stale calculatedVehicleValues from previous occupant #11773

Description

@daijoubu

Summary

When a new ADS-B vehicle claims a free slot in adsbVehiclesDictionary, the slot's calculatedVehicleValues (distance, bearing, valid flag) are not refreshed for that vehicle before the slot is marked active. For one scheduler tick, an active (ttl > 0) slot can present a real vehicle's identity (icao, GPS position) paired with a different, expired vehicle's stale distance/bearing, still flagged valid == true.

Where

src/main/io/adsb.c, in adsbNewVehicle():

if (vehicle != NULL) {
    memcpy(&(vehicle->vehicleValues), vehicleValuesLocal, sizeof(vehicle->vehicleValues));  // new vehicle's data written
    recalculateVehicle(vehicle);                                                            // no-op: ttl is still 0 here
    vehicle->ttl = MAX(0, ADSB_MAX_SECONDS_KEEP_INACTIVE_PLANE_IN_LIST - vehicleValuesLocal->tslc); // ttl now > 0
    return;
}

recalculateVehicle() opens with:

static void recalculateVehicle(adsbVehicle_t* vehicle) {
    if (vehicle->ttl == 0) {
        return;
    }
    ...

When vehicle was just obtained from findFreeSpaceInList() (which matches on ttl == 0), recalculateVehicle() is called while ttl is still 0, so it returns immediately without touching calculatedVehicleValues. ttl is only set to a nonzero value on the next line, after the no-op call.

Root cause

The ttl == 0 guard inside recalculateVehicle() was added to stop taskAdsb()'s per-tick loop from recalculating vehicles that have already expired:

for (uint8_t i = 0; i < MAX_ADSB_VEHICLES; i++) {
    if (adsbVehiclesDictionary[i].ttl > 0) {
        if (shouldDecrementTtl) {
            adsbVehiclesDictionary[i].ttl--;
        }
        recalculateVehicle(&adsbVehiclesDictionary[i]);
    }
}

That call site already wraps the call in if (ttl > 0), so the internal guard is redundant there. But the same guard silently defeats the other call site in adsbNewVehicle(), where ttl genuinely hasn't been assigned yet at the moment of the call. This looks like an oversight rather than an intentional design: the guard was written with only the expiry/decrement use case in mind.

Why the stale data isn't harmless

Ordinary expiry never clears calculatedVehicleValues.valid:

  • The natural per-tick ttl-- in taskAdsb() only decrements ttl; it never touches calculatedVehicleValues.
  • The tslc timeout branch in adsbNewVehicle() (vehicle->ttl = 0;) also doesn't touch calculatedVehicleValues.
  • Only the out-of-range branch inside recalculateVehicle() (dist > ADSB_LIMIT_CM) explicitly sets valid = false.

So an expired slot typically still has calculatedVehicleValues.valid == true, holding the last real distance/bearing computed for the previous occupant. When that slot is reused as described above, ttl becomes nonzero (active, per convention) while valid is still true and dist/dir are the old, unrelated vehicle's values.

Consumers that follow the standard ttl > 0 && calculatedVehicleValues.valid pattern — findVehicleForWarning(), findVehicleForAlert(), findVehicleFarthest() — would pass both checks for this slot and could report/alert on the wrong distance/bearing, attributed to the new vehicle's icao, until the next taskAdsb() tick actually recalculates the slot.

Suggested fix

Set vehicle->ttl before calling recalculateVehicle(vehicle) in adsbNewVehicle()'s GPS-fix branch, so the calculation isn't skipped for a freshly claimed slot:

if (vehicle != NULL) {
    memcpy(&(vehicle->vehicleValues), vehicleValuesLocal, sizeof(vehicle->vehicleValues));
    vehicle->ttl = MAX(0, ADSB_MAX_SECONDS_KEEP_INACTIVE_PLANE_IN_LIST - vehicleValuesLocal->tslc);
    recalculateVehicle(vehicle);
    return;
}

This doesn't affect the taskAdsb() call site, which is a separate, already-guarded call.

Impact

Transient (bounded by one taskAdsb() scheduling period) but reachable in normal operation any time a new aircraft is picked up in a slot previously occupied by an aircraft that went out of range/silent. Affects OSD proximity warnings/alerts and any MSP consumer reading vehicle state in that window.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions