Skip to content

fix(ota): align nRF DFU MAC increment to nrf-ota no-carry convention + add discovery fallback#121

Merged
g4bri3lDev merged 3 commits into
OpenDisplay:mainfrom
balloob:fix/dfu-mac-increment-and-fallback
Jul 6, 2026
Merged

fix(ota): align nRF DFU MAC increment to nrf-ota no-carry convention + add discovery fallback#121
g4bri3lDev merged 3 commits into
OpenDisplay:mainfrom
balloob:fix/dfu-mac-increment-and-fallback

Conversation

@balloob

@balloob balloob commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Fixes finding MJ-23 (py-opendisplay side): two DFU-rediscovery issues in src/opendisplay/ota.py.

1. MAC+1: align to the Nordic / nrf-ota no-carry convention

_increment_mac incremented the address as a full 48-bit integer, so …:EE:FF carried into the previous octet (…:EF:00). The standalone nrf-ota scanner (nrf_ota/scan.py) instead increments only the last octet as a uint8 — it wraps 0xFF → 0x00 with no carry (…:EE:FF → …:EE:00), matching documented Nordic bootloader behaviour. This aligns py-opendisplay to that convention so both implementations agree.

Note: the exact bootloader wrap behaviour isn't captured in the repos, so this should be validated on real hardware. The discovery fallback below keeps rediscovery robust regardless of the exact MAC guess.

2. Discovery fallback by DFU service UUID (no name-based selection)

find_nrf_dfu_device previously matched only the MAC+1 (and, after 10 s, the original) address, so a wrong MAC guess meant a silent 30 s scan miss. It now also selects a device advertising the Legacy DFU service UUID (00001530-…). The scan switches to discover(return_adv=True) to read advertisement service UUIDs.

Selection deliberately does not match on device name. A name that merely contains "DFU"/"DfuTarg" would pick the wrong tag when two devices are in bootloader mode at once — which is the core of finding MJ-23. This matches the discriminator chosen in nrf-ota #7 (MAC+1 OR service-UUID, name-based selection dropped). The UUID fallback is safe to return before the 10 s mark because the stale app-mode entry does not expose the DFU service.

Tests

  • _increment_mac now asserts the no-carry wrap (…:FF → …:00, previous octets untouched).
  • Added a find_nrf_dfu_device case matching by service UUID when the MAC doesn't match, and a case asserting a DfuTarg-named advert with no matching UUID/MAC is not selected.
  • Full suite: 501 passed.

Companion change on the nrf-ota side is tracked in its own PR (#7); both now use the same no-carry MAC+1 convention and the same MAC+1 OR service-UUID selection discriminator.

🤖 Generated with Claude Code

https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR


Companion PR: OpenDisplay/nrf-ota#7 (standalone tool — same MAC+1 no-carry + UUID-only device selection). Both now select by MAC+1 OR Legacy DFU service UUID, never by name.

…+ add discovery fallback

Two DFU-rediscovery issues in the nRF OTA path:

- `_increment_mac` used a full 48-bit carry, disagreeing with the
  Nordic-reference convention (a uint8 increment of the last octet: wrap
  0xFF -> 0x00, no carry) that the standalone nrf-ota scanner uses. Align
  py-opendisplay to the no-carry convention so both implementations agree.

- `find_nrf_dfu_device` matched only the MAC+1 address, so a wrong guess
  meant a silent scan miss. Add a fallback that also matches the Legacy DFU
  service UUID or a DFU-style advertised name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
@balloob balloob requested a review from g4bri3lDev as a code owner July 6, 2026 06:22
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…-ota OpenDisplay#7)

Selecting a DFU target by a name merely containing "DFU"/"DfuTarg" picks the
wrong tag when two devices are in bootloader mode at once — the core of finding
MJ-23. Align with nrf-ota OpenDisplay#7: select only by MAC+1 OR the Legacy DFU service
UUID. Replace `_is_dfu_advertisement` with `_advertises_dfu_service` (UUID-only)
and update the fallback test to assert a DfuTarg-named advert with no matching
UUID/MAC is NOT selected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
@g4bri3lDev g4bri3lDev merged commit 0ed1128 into OpenDisplay:main Jul 6, 2026
6 checks passed
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