Repository navigation
fix(alerts): stop restart replays and drone false positives - #69
Merged
Merged
Conversation
Retained alert state outlives the process. A rule that was `on` when the
service exited stays retained `on`, and `on_connect` published
`status: online` before any evaluation had run — so HA flipped the
binary_sensor `unavailable -> on` off a stale value and fired the
notification automation for a detection that never happened. Every
restart while a track was alerting produced a spurious push.
`client.py` already guards `status` against exactly this ("we deliberately
publish offline ... so HA never sees stale `online` retained state"); the
alert topics never got the same treatment. They also could not
self-correct: `publish_alert_active` is driven only by transitions, so a
rule that is quiet after startup never republishes and the stale value
just stands.
Publish the truth first, then availability. The payload is the genuine
current set, not a blanket clear: on a mid-run broker reconnect rules are
legitimately active, and asserting `off` there would flap them off and
straight back on — the same duplicate notification by another route. That
is why this reads through `AlertEvaluator.alert_states()` (every
configured rule, not just the matching ones) instead of hardcoding false.
Snapshot is taken synchronously and deliberately without the tracker
lock: the hook runs on the MQTT loop, and a tick blocked enqueueing a
publish while holding that lock would deadlock against it. No await point
can interleave a poll into an allocation-only dict comprehension.
Considered clearing alert state on graceful shutdown instead; rejected
because it does nothing for a SIGKILL, which is how this add-on usually
dies.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Alert MatchBlock keys are purely geometric — distance, altitude, closing
vector. Nothing in the grammar could express "this is a drone", so the
rules we document as drone_conflict and drone_nearby were geometry with
aspirational names: they matched any low, close track. Observed in the
field pushing N1629A (hex a0fcb0, a PIAT at 1575 ft and 3.70 nm) as a
drone notification.
This was latent, not new, but two recent changes surfaced it. The AGL fix
(1.1.6) made these rules match drones for the first time, so they went
from firing *only* on manned aircraft to firing on both — the misfire
became visible instead of being the entire behavior. Widening the
documented distance in the same release then increased the manned-aircraft
hit rate.
Derived flag rather than a new match key: the Phase-3 notes already
anticipated it ("Flag rules can still apply (e.g. a `drone` flag)"), and
ORBIT_FLAG / SPOOF_FLAG set the precedent for a flag composing with the
existing grammar for free. A `band:` matcher would have been more
expressive but needs a config schema change to say something the flag
already says.
Applied in Enricher rather than a detector because evaluate_flags
reassigns state.flags wholesale (anything added earlier is dropped), and
because being a drone needs neither history nor a config toggle, unlike
the stateful opt-in orbit/spoof detectors.
Backwards-compatible: the flag is always available but changes no
existing rule until composed into it. The example configs now do so, with
the naming trap called out explicitly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit added flags: ["drone"] to both example drone rules. That is correct for drone_nearby and wrong for drone_conflict, which exists to warn that a low MANNED aircraft is inbound over the area your own drone is flying in so you can land it (docs/automations.example.yaml #5). The drone in that rule's name is yours and is never tracked; gating it on the flag would match only drones and silence exactly the traffic it exists to report. The shared prefix hides that the two rules have opposite subjects, which is what made the mistake easy — so say it plainly where the rules are defined rather than leaving it implicit. Also reconciles drone_conflict's bounds with the documented intent (max_closest_approach_nm 10.8 / within_eta_s 600 — the ~20 km consumer drone radio range, i.e. the volume you might be flying in) instead of the 0.5 nm that had been sitting there, which only made sense under the misreading that the rule tracks drones. The drone_nearby rationale was written under the same misreading — it described itself as covering what drone_conflict's predictive gate misses, as if both watched drones. Rewritten: it is a pure-presence rule because hovering drones never project an ETA, not because it is drone_conflict's other half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Two independent drone-alert bugs, both surfaced by field testing after 1.1.6.
1. Restart replays the last alert as a new one
Retained
alert/<rule>/activeoutlives the process. A rule that wasonat exit stayed retainedon, andon_connectpublishedstatus: onlinebefore any evaluation ran — so Home Assistant flipped thebinary_sensorunavailable -> onoff a stale value and fired the notification automation for a detection that never happened. It couldn't self-correct either:publish_alert_activeonly runs on transitions, so a quiet rule never republished.on_connectnow asserts the true state of every configured rule while still marked offline, then flips availability.client.pyalready guardedstatusthis way; the alert topics never got the same treatment.It publishes the genuine active set (
AlertEvaluator.alert_states()), not a blanket clear — on a mid-run broker reconnect rules are legitimately active, and forcingoffwould flap them off and back on, i.e. the same duplicate push by another route. The snapshot is synchronous and takes no tracker lock: the hook runs on the MQTT loop, and a tick blocked enqueueing a publish while holding that lock would deadlock.Clearing on graceful shutdown was considered and rejected — it does nothing for a SIGKILL.
2. Light aircraft pushed as drones
MatchBlockkeys are purely geometric (distance, altitude, closing vector), so nothing could express "this is a drone". Adrone_nearbyrule built from geometry matched any low, close track — observed pushing a light twin at ~1,500 ft as a drone.Adds a derived
droneflag on every Remote ID track, composable like any flag (flags: ["drone"]). The Phase-3 notes already anticipated it, andORBIT_FLAG/SPOOF_FLAGset the precedent. Applied inEnricherafterevaluate_flags, which reassignsstate.flagswholesale.Latent rather than new, but 1.1.6 surfaced it: the AGL fix made these rules match drones for the first time, so the manned-aircraft matches stopped being the whole behavior and became visible false positives.
Backwards-compatible — the flag changes no rule until composed into it.
The flag does not belong on
drone_conflictThe two example rules have opposite subjects behind a shared prefix:
flags: ["drone"]drone_conflictdrone_nearbyThe example config now says so explicitly, restores
drone_conflictto its documented bounds (10.8 nm / 600 s — consumer-drone radio range), and rewrites thedrone_nearbyrationale, which had described itself as covering whatdrone_conflictmisses as though both watched drones.Verification
alert_states()snapshots, flag derivation and its ordering against configured flags, and a regression pinning the observed false positive.Enricher+rule_matches: four manned aircraft that matched the geometry-only rule do not match the flag-gated rule.drone_nearbyfires for the drone (flags: ["drone"],hex: null) and not for overhead manned traffic.Reviewer notes
"drone"inflags— a new value in an existing field, soschema_versionstays at 1.flags: ["drone"]is added todrone_nearbyin their own config; the code makes it expressible, it doesn't apply it.version-guardjob.🤖 Generated with Claude Code