Skip to content

fix(wifi): read neighbour scan strength as dBm, not the raw quality index - #32

Merged
drench44 merged 1 commit into
gneitzke:mainfrom
drench44:fix/neighbour-rssi-units
Aug 1, 2026
Merged

fix(wifi): read neighbour scan strength as dBm, not the raw quality index#32
drench44 merged 1 commit into
gneitzke:mainfrom
drench44:fix/neighbour-rssi-units

Conversation

@drench44

@drench44 drench44 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

The bug

stat/rogueap reports neighbour strength two ways:

  • signal is the real RSSI in dBm (negative)
  • rssi is the driver's 0-based quality index above the noise floor (positive, roughly 2..50 in practice)

ingest/unifi/models.py already documents exactly that distinction on the mesh-uplink fields, but RogueAp dropped signal entirely and the shared neighbour decoder passed the index straight through as "rssi".

Both wifi.neighbor_density and wifi.rogue_ap then compare that value against a *_rssi_floor_dbm threshold. A positive number is never <= -75, so the "strong enough to matter" filter never excluded anything and every audible BSS qualified.

On my own store that reported 258 of 265 scanned networks as crowding the 2.4 GHz channels, in a neighbourhood nowhere near that dense. All 619 neighbour rows carried positive values, range 2 to 49, not one negative. The count above the real -75 dBm line is 14.

The fix

Convert once in _neighbor_rows, the shared decoder both detectors already read through, so the two cannot disagree about what the scan said.

  • Prefer signal when the poll captured it
  • Fall back to the index offset by the noise floor for rows collected before it did
  • The offset is tunable as noise_floor_dbm under wifi.neighbor_density, read from that one key by the shared decoder because it describes the scan, not either detector's policy
  • Capture and persist signal on RogueAp so new scans carry exact dBm rather than an offset estimate

Sign alone does not prove a field is dBm. Drivers use small negative sentinels for "no reading", and this module already carries or -127.0 fallbacks for that case. A -1 taken verbatim would read as an absurdly strong neighbour that clears every floor and silently inflates the count, so both branches require a plausible sighting (-120..-20 dBm) and treat anything else as junk.

Verification

Full suite: 2002 passed, 1 skipped.

New tests cover the index reading as dBm, a genuinely strong index still counting, signal winning when both are present, noise_floor_dbm reaching the decoder end to end, and the sentinel cases (-1, -400, a negative index) being rejected rather than trusted.

Replayed against a real 619-row store, the 2.4 GHz finding goes from 258 qualifying to 14:

Stronger than Count
-65 dBm 3
-70 dBm 5
-75 dBm (the floor) 14
-80 dBm 28

The 5 GHz finding falls below density_min_count and clears. Top-offender ordering is unchanged and the values are now real dBm.

Known gap, not addressed here

A finding's rssi_dbm evidence does not say whether it came from a measured signal or an offset index. On a mixed-vintage store those are indistinguishable, and the estimated ones move if noise_floor_dbm is retuned. An rssi_source field would fix it; happy to add if you want it in this PR.

…y index

stat/rogueap reports a neighbour's strength two ways. `signal` is the real
RSSI in dBm (negative). `rssi` is the driver's 0-based quality index above the
noise floor (positive, roughly 2..50 in practice). models.py already documents
that distinction on the mesh-uplink fields, but the RogueAp model dropped
`signal` and the shared neighbour decoder passed the index straight through.

Both neighbour detectors then compared that index against a `*_rssi_floor_dbm`
threshold. A positive number is never <= -75, so the "strong enough to matter"
filter never excluded anything and every audible BSS qualified. On a real
site this reported 258 of 265 scanned networks as crowding our 2.4 GHz
channels; the true count above -75 dBm is 14.

Convert once in `_neighbor_rows`, the shared decoder both detectors already go
through, so they cannot disagree. Prefer `signal` when the poll captured it and
fall back to the index offset by the noise floor for rows collected before it
did. The offset is tunable as `noise_floor_dbm` under wifi.neighbor_density,
read from that one key by the shared decoder because it describes the scan
rather than either detector's policy.

Sign alone does not prove a field is dBm: drivers use small negative sentinels
for "no reading", and this module already carries `or -127.0` fallbacks for
that case. A -1 taken verbatim would read as an absurdly strong neighbour that
clears every floor and silently inflates the count, so both branches now
require a plausible sighting (-120..-20 dBm) and treat anything else as junk.

Also capture `signal` on RogueAp and persist it, so new scans carry the exact
dBm instead of relying on the offset.

Verified against a 619-row production store: the 2.4 GHz finding drops from
258 qualifying to 14, and the 5 GHz finding falls below density_min_count and
clears. Full suite 2002 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drench44
drench44 merged commit 53c0043 into gneitzke:main Aug 1, 2026
5 checks passed
@gneitzke

gneitzke commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Thanks @drench44, great catch. Reading the quality index as dBm was quietly inflating the neighbour count — I'd been seeing "123 neighbouring networks" on my own 2.4 GHz during a UI pass, and this is exactly why. Converting once at the boundary (with the noise-floor guard so the two detectors can't disagree) is the right call, and the test coverage is appreciated. Pinning it into the next release now.

gneitzke added a commit that referenced this pull request Aug 2, 2026
…y index

Ships @drench44's fix (#32): stat/rogueap reports a neighbour's strength as both
a real RSSI (signal, negative dBm) and a 0-based driver quality index (rssi,
positive). The detectors were comparing the positive index against a negative
dBm floor, so the strength filter never fired and a quiet network reported
hundreds of "neighbouring networks" -- the "123 neighbours on 2.4 GHz" seen in
the last UI pass was exactly this. The index is now converted to dBm once at the
ingest boundary, behind a shared noise-floor reference so the two neighbour
detectors cannot disagree.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014DGXNizASgdJWegya9JnYx
gneitzke added a commit that referenced this pull request Aug 2, 2026
… class in CI

Came out of hunting the bug class @drench44's #32 belongs to — a value in one
unit compared against a threshold in another, so a check silently mis-fires.

The confirmed bug: the SLE isp_loss classifier compared a raw per-interval
packet-drop COUNT against wan_loss_threshold=1.0, which its own docstring calls
"ISP loss >1%". So any bucket with more than one dropped packet branded isp_loss
for every active client — ordinary light loss of two or three packets tripped
it. It survived three releases because it is masked on a gateway-less site
(stat/health carries no WAN metrics, loss stays None) and invisible in the demo,
which hand-writes SLE rows instead of running the engine.

isp_loss now judges the failed-probe fraction with a floor and a minimum lost-
probe count, exactly as the sibling wan.py detector already does it correctly;
the raw drop count is demoted to corroborating evidence and never the verdict.
A guard also ensures a probe that fails every poll (a non-functional ICMP path,
e.g. a container with no ICMP) is read as a measurement artifact, not loss —
reachability defers to the DNS anchor, so the fix does not trade one false
positive for another. wan_loss_threshold (packets) becomes wan_loss_fraction
(rate) + wan_loss_min_probes.

Two CI guards close the class, not just this instance:
- A unit-suffix contract test AST-walks every config threshold and every
  ctx.threshold() key and fails on any unsuffixed numeric threshold. The bug and
  a naming gap were the same gap: every threshold already carried a unit suffix
  except this one. A new unlabeled threshold now fails CI at authoring time
  (verified against an injected field).
- The real SLE minutes engine now runs over a slice of the seeded demo and
  asserts isp_loss == 0 for the healthy window. Pre-fix that slice produced an
  isp_loss avalanche (630 minutes); the engine never ran over the demo before,
  which is precisely why this hid.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014DGXNizASgdJWegya9JnYx
@drench44
drench44 deleted the fix/neighbour-rssi-units branch August 7, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants