Skip to content

fix: carry across octets when computing the nRF DFU MAC (+1) (§4)#110

Merged
g4bri3lDev merged 1 commit into
OpenDisplay:mainfrom
balloob:fix/minor-safety-limits
Jul 5, 2026
Merged

fix: carry across octets when computing the nRF DFU MAC (+1) (§4)#110
g4bri3lDev merged 1 commit into
OpenDisplay:mainfrom
balloob:fix/minor-safety-limits

Conversation

@balloob

@balloob balloob commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

find_nrf_dfu_device MAC+1 wrapped without carry (🟡)

The nRF DFU bootloader advertises at original_address + 1. The code incremented only the last octet with (+1) & 0xFF, so an address ending in 0xFF became …:EE:00 instead of carrying into the previous octet (…:EF:00). The bootloader then advertised at an address the scanner never matched → a silent 30 s scan failure on those devices.

Fix

Increment the full 48-bit address with carry, extracted into a small, tested _increment_mac helper.

Test plan

  • uv run pytest -q → 447 passed (new parametrized carry test: …EE:01→…EE:02, …EE:FF→…EF:00, …FF:FF→…DE:00:00… wait double-carry, FF:…FF→00:…00; updated an existing test that encoded the old no-carry behavior)
  • ruff, mypy clean

🤖 Generated with Claude Code

@balloob balloob requested a review from g4bri3lDev as a code owner July 4, 2026 07:28
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

find_nrf_dfu_device incremented only the last MAC octet with (+1 & 0xFF), so a
device whose address ends in 0xFF produced ...:EE:00 instead of carrying into
the previous octet (...:EF:00). The bootloader then advertised at an address the
scanner never matched, causing a silent 30 s scan failure. Increment the full
48-bit address with carry (extracted to a tested _increment_mac helper).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JRrm95f1qNZzDM9r2SB6KW
@balloob balloob force-pushed the fix/minor-safety-limits branch from 565d417 to 48b8b9c Compare July 4, 2026 11:43
@g4bri3lDev g4bri3lDev merged commit 88ae351 into OpenDisplay:main Jul 5, 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