Repository navigation
fix: normalize peer address before duplicate-identity comparison - #48
Merged
Merged
Conversation
On two fixed-MAC peers both running central + peripheral, the same physical peer is represented by two address strings: the peripheral (GATT) path stores "dev:AA:BB:.." (BlueZ D-Bus device-path form) while the central (scan) path carries the bare "AA:BB:..". The _check_duplicate_identity comparison (and the v2.2 scan-loop comparison) saw "dev:AA:BB" != "AA:BB", concluded a false Android MAC rotation, rejected the connection, and detached the peer interface after the grace period. The data path then dropped: discovery announces routed to 0 peers and the peer never appeared in the destination table. Strip the "dev:" prefix and case-fold before comparing, so the same physical MAC compares equal regardless of which form it arrived in, while genuinely different MACs (true MAC rotation) still differ. TDD: test_ble_dup_identity_mac_normalize.py reproduces the false positive (RED before, GREEN after) and guards that true MAC rotation is still rejected. Bind the new _normalize_address helper in the zombie/blacklist test harnesses that call the real method.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
The normalized same-address branch in _select_peers_to_connect (dev:-prefix peripheral form vs bare central form for the same fixed-MAC peer) is now pinned by TestScanLoopSameAddressRegression: the skip decision (interface already exists, so the peer is not re-added as a 'MAC rotation') is asserted, with a stale-interface (not-in-self.peers) state, and a contrast test proving that the skip is caused by the branch rather than another gate. Also documents that the same-MAC reconnect path is the standard top-level gate (address in self.peers), not this branch.
Owner
Author
|
@greptile review |
Android (AndroidBLEInterface) and iOS subclass the shared BLEInterface and wire on_duplicate_identity_detected -> _check_duplicate_identity with no override. Their drivers produce bare, same-case MACs (never the BlueZ dev: prefix that is the headline Linux fix), so the normalizer must be a no-op for the common mobile case and must not disable true MAC-rotation rejection. Add TestBareMacMobileSafety to pin that contract on the real method: * bare same-case MAC, alive -> not a duplicate (unchanged) * bare different MAC, alive -> still rejected (guard holds) so a future edit cannot silently alter Android/iOS behavior.
Owner
Author
|
@greptile review |
…nitpick) The TestScanLoopSameAddressRegression class lives in tests/test_v2_2_mac_sorting.py (not test_ble_dup_identity_mac_normalize.py); fix the pointer in the code comment.
Owner
Author
|
@greptile review |
The v2.2 same-identity normalization in _select_peers_to_connect is pinned by tests/test_v2_2_mac_sorting.py, but that file is excluded from the integration coverage run (--ignore=...), so codecov/patch sees the changed scan-loop lines as uncovered and fails. Add tests/test_scan_loop_normalize_coverage.py that drives the REAL _select_peers_to_connect from an included module, pinning each branch (rotation-alive skip, rotation-dead reselect + cleanup, same-normalized address skip) plus the _normalize_address falsy-input branch. Verified: the three changed BLEInterface regions (check_duplicate_identity, scan-loop norm, _normalize_address) now show 0 uncovered lines under the CI-equivalent coverage run.
torlando-tech
added a commit
that referenced
this pull request
Oct 3, 2026
Adds the Keep-a-Changelog entry for the v0.2.3 release covering the scanner wedge health-check, the ifac_size inheritance fix that was dropping all inbound BLE packets, announce-rate stats, duplicate- identity normalization, the package rename, and related fixes (#45, #47, #48, #49 plus the #29-#44 window). This was the missing precondition blocking the v0.2.3 release workflow: the tag points at a commit where pyproject.toml still reads 0.2.2 and no CHANGELOG.md entry exists. Co-authored-by: torlando-tech <torlando-tech@users.noreply.github.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.
Summary
On two fixed-MAC peers both running central + peripheral mode, the same physical peer is represented by two address strings: the peripheral (GATT) path stores
dev:AA:BB:..(BlueZ D-Bus device-path form) while the central (scan) path carries the bareAA:BB:... The_check_duplicate_identitycomparison sawdev:AA:BB!=AA:BB, concluded a false Android MAC rotation, rejected the connection, and detached the peer interface after the grace period.The data path then dropped: discovery announces routed to 0 peers and the peer never appeared in the destination table.
Fix
Add
_normalize_address()that strips thedev:prefix and case-folds. Apply it to both comparison sites in_check_duplicate_identityand the v2.2 scan loop. Genuinely different MACs (true Android MAC rotation) still differ and are still handled.TDD
test_ble_dup_identity_mac_normalize.pyreproduces the false positive (RED before, GREEN after) and guards that true MAC rotation is still rejected. Bind the new_normalize_addresshelper in the zombie/blacklist test harnesses that call the real method.Test results
330 passed, 6 failed (all pre-existing:
No module named 'dbus', fail identically on clean main).