Fix operator tracks collapsing onto op-Unknown-BasicID_0 (v2.3.2) - #10
Closed
ampledata wants to merge 4 commits into
Closed
Fix operator tracks collapsing onto op-Unknown-BasicID_0 (v2.3.2)#10ampledata wants to merge 4 commits into
ampledata wants to merge 4 commits into
Conversation
Serial/MAVLink receivers (DroneScout Bridge, SiK) report no MAC at all,
so a Location- or System-only message from one has neither a MAC nor a
serial to identify it and rendered under the shared placeholder
Unknown-BasicID_0. Measured live on a DroneScout Bridge feeding
dronecot-dronescout: 57 correctly identified events alongside 3
placeholders, and the placeholder collides across every such source.
_rid_identity() now falls back to the feed/sensor id (FEED-<sensor_id>)
before the placeholder, so unidentified contacts from different receivers
stay distinct instead of piling onto one track.
Deliberately NOT keyed on the feed for AGGREGATION. I tried that first
and it is wrong twice over:
* It SPLITS rather than merges -- a BasicID message keys on the serial
while a Location message from the same aircraft keys on the feed, so
they land in two tracks and neither completes. Verified: track_key
gave ('rid','SERIAL9') and ('feed','dronescout') for one transmitter.
* Worse, it would MERGE DISTINCT AIRCRAFT. One DroneScout receiver
reports many drones over a single port, so a feed key would collapse
them into one track -- exactly the bug this module was written to fix,
reintroduced by another route.
So a record with neither MAC nor serial still passes through
unaggregated; only its rendered UID changes. track_key() carries a
comment explaining why the tempting fallback is absent, and a test
asserts it stays absent.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197da7dhvcPoHxYKamrYqyM
ASTM F3411 accuracy fields are ENUM CODES; CoT <point ce= le=> are
metres. Emitting the raw code claimed a precision the aircraft never
reported, and the worst case inverted the meaning entirely: code 0 means
"unknown / >= 18.52 km" but rendered as ce="0", a PERFECT fix.
Observed live on a DroneBeacon DB120: HorizAccuracy=9 (ODID < 30 m) was
drawn on the map at 9 m -- 3.3x better than reported -- and
BaroAccuracy=0 (unknown) as zero error.
Codes now decode to metres via tables mirroring
decodeHorizontalAccuracy()/decodeVerticalAccuracy() in
opendroneid-core-c. Each code is an upper bound ("less than X") so X is
the conservative reading. Unknown (0), reserved, unparseable and missing
values all emit the CoT unknown sentinel rather than a fabricated number.
Also: <height value> defaulted to 0, rendering an absent altitude as sea
level. It now uses the same unknown sentinel as hae/ce/le.
Four existing tests in test_functions.py asserted the raw codes
(ce="12", le="5") and so encoded the bug; corrected to the decoded
metres (1.0 m and 3.0 m, from ODID_HOR_ACC_1_METER and
ODID_VER_ACC_3_METER).
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197da7dhvcPoHxYKamrYqyM
Measured on a live box (aryaos-4f11, DroneScout DS110), 30s of CoT on the
Mesh SA group attributed by source IP:
from 192.168.0.13 (DroneScout, MAVLink serial):
op-Unknown-BasicID_0 x62 <-- every operator on ONE track
RID.1787F04BM24010011195.uas x54 correct
op-1787F04BM24010011195 x54 correct
from 192.168.0.44 (Wi-Fi RID, SAME drone):
op-4A:BE:41:9D:E3:34 x14 correct, MAC fallback working
So the aircraft UID resolved while the operator UID did not, and every
operator seen by that receiver merged into a single track.
Cause: the MAVLink path called rid_normalize.odid_parsed_to_rid_dict
directly and attached NO "data" metadata at all, unlike the wireless path
(bytes_to_rid_dict, which builds it via uas_meta_defaults). With neither a
MAC nor a sensor_id, a pack carrying only System/OperatorID and no BasicID
had no transmitter identity to key on, so functions._rid_identity fell
through every rung to Unknown-BasicID_0.
MAVLink OpenDroneID carries a 20-byte id_or_mac naming the transmitter,
which the code ignored. It is the only per-aircraft grouping key a serial
feed offers -- there is no radio MAC in the metadata. Measured on the
DS110: populated on 58 of 58 packs, db:13:81:94:13:55 + 14 zero bytes.
* attach metadata on both MAVLink paths, carrying id_or_mac as the
advertiser MAC and the CONFIGURED SENSOR_ID
* only accept a MAC-shaped id_or_mac (6 significant bytes then padding).
The field may instead hold an opaque ID string, and calling that a MAC
would put a wrong-looking value in the UID, so those feeds keep the
previous behaviour until there is hardware to test against.
* _rid_identity: look for sensor_id at BOTH levels. The MAC was already
checked at both because "MQTT and serial feeds put it at the top
level"; the same is true of sensor_id, so that rung silently never
fired for them.
Second effect of the missing metadata, also fixed: CoT was attributed to
the default dronecot_<hostname> instead of the configured SENSOR_ID, so on
a box with two receivers you could not tell which one saw a contact.
_adsb_vehicle_to_rid_dict stays a static pure function with metadata
injected by the caller, so its existing unit tests keep working.
13 new tests using the real captured id_or_mac. Verified they catch the
bug: reverting the metadata attachment fails exactly the three identity
tests with 'Unknown-BasicID_0' == 'Unknown-BasicID_0'. Full suite 85
passed, 1 skipped.
This is the same CLASS of bug as the aggregator fix in 2.3.0, which I
verified on the UAS path only and not the operator path.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197da7dhvcPoHxYKamrYqyM
The version lives in src/dronecot/VERSION (setup.cfg reads it with 'version = file:'), not in __init__.py. My bump targeted __init__.py, the assertion caught that it was not there, but the commit went ahead anyway -- so the fix landed on 2.3.1's version string.
Contributor
Author
|
Superseded by a rebase onto current |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Measured on a live box (
aryaos-4f11, DroneScout DS110), 30 s of CoT on the Mesh SA group, attributed by source IP:The aircraft UID resolved; the operator UID did not. Every operator seen by that receiver merged into one track.
Cause
The MAVLink path called
rid_normalize.odid_parsed_to_rid_dictdirectly and attached nodatametadata at all — unlike the wireless path, which builds it viabytes_to_rid_dict→uas_meta_defaults. With neither a MAC nor asensor_id, a pack carrying only System/OperatorID and no BasicID had no transmitter identity to key on, so_rid_identityfell through every rung toUnknown-BasicID_0.MAVLink OpenDroneID carries a 20-byte
id_or_macnaming the transmitter, which the code ignored. It's the only per-aircraft grouping key a serial feed offers — there's no radio MAC in the metadata. Measured on the DS110: populated on 58 of 58 packs,db:13:81:94:13:55+ 14 zero bytes, one distinct value for one drone.Changes
id_or_macas the advertiser MAC and the configuredSENSOR_ID.id_or_mac(6 significant bytes then padding). The field may instead hold an opaque ID string, and calling that a MAC would put a wrong-looking value in the CoT UID — those feeds keep the previous behaviour until there's hardware to test against._rid_identity: look forsensor_idat both levels. The MAC was already checked at both because "MQTT and serial feeds put it at the top level" — the same is true ofsensor_id, so that rung silently never fired for them.Second bug the missing metadata caused, also fixed: CoT was attributed to the default
dronecot_<hostname>instead of the configuredSENSOR_ID, so on a box with two receivers you couldn't tell which one saw a contact._adsb_vehicle_to_rid_dictstays a static pure function with metadata injected by the caller, so its existing unit tests keep working untouched.Tests
13 new tests using the real captured
id_or_mac, including that two transmitters don't collapse (the DS110 reports many aircraft over one port).Verified they catch the bug — reverting the metadata attachment fails exactly the three identity tests:
Full suite: 85 passed, 1 skipped.
Worth saying plainly
This is the same class of bug as the aggregator fix in 2.3.0. I verified that one on the UAS path and not the operator path, so it shipped half-fixed.
🤖 Generated with Claude Code
https://claude.ai/code/session_0197da7dhvcPoHxYKamrYqyM