fix: BLE advertising/TX-power/LED/button parity polish#8
Merged
Conversation
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
CMSIS 6 (framework-zephyr 3.40400 for the LM20A) gates DSP intrinsic wrappers on __ARM_FEATURE_DSP; forcing the macro exposes __sxtb16/__sxtab16 which g++ never declares as builtins, failing every C++ TU. No code in this tree uses DSP intrinsics, and the compiler predefines the macro itself when the target arch actually supports DSP. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
…reakage Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
Restore nRF52840-reference parity in the nRF54 BLE peripheral: - Advertising interval floor 160 ms (was fixed 1000 ms) - MSD status bit2 connectionRequested flag + setter hook - Apply power_option.tx_power via HCI VS Write_Tx_Power_Level (adv + conn) - LED flash: full 8-level software-PWM brightness ramp (was 2-step) - Buttons: both-edges GPIO interrupt + pending flag, honor pulldowns mask - 0x0052 deep sleep: no response (match reference nRF52840 build) 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
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.
What
A set of small parity fixes bringing the nRF54 BLE peripheral in line with the nRF52840 reference firmware (
OpenDisplay-Firmware, Arduino/Bluefruit). Each fix was verified against the cited reference code. No behavioral guesses — where the reference has no producer, that is documented rather than invented.Both CI environments (
seeed-xiao-nrf54l15,seeed-xiao-nrf54lm20a) build green.Per-fix summary
MIN=MAX=BT_GAP_ADV_SLOW_INT_MIN(opendisplay_ble.c:32-33)OD_ADV_INTERVAL_MIN=256u(opendisplay_ble.c:37)MIN=256/MAX=1600(ble_init.cpp:48-49,63)connectionRequestedopendisplay_ble.c:145-149)opendisplay_ble_set_connection_requested()(opendisplay_ble.c:157,344;opendisplay_ble.h)((connectionRequested & 0x01) << 2)(display_service.cpp:1296)power_option.tx_powerWrite_Tx_Power_Levelfor advertising (init + config reload) and per-connection (apply_tx_power,opendisplay_ble.c:366)Bluefruit.setTxPower(power_option.tx_power)(ble_init.cpp:90)od_flash_led)opendisplay_led.c:82)delayMicroseconds(100)slices inflashLed(device_control.cpp:471-499)opendisplay_button.c:69passedfalse)nrf54_gpio_configure_interrupt()(ISR sets a pending flag, no I2C/BLE); polling kept as fallback; honorspulldownsmask (nrf54_gpio.c:98,opendisplay_button.c:80,86)pulldowns(device_control.cpp:589-605)0x0052deep-sleep response parity{0x00,0x52}then no-opsopendisplay_pipe.c:1177)device_control.cpp:691-705)%06lX(opendisplay_ble.c:102)getChipIdHex()pads to 6 hex digits (encryption.cpp:725-737)Notes on individual fixes
Fix 2 — In the reference,
connectionRequestedis declared and read into the status byte but is never set anywhere (main.h:127, "Reserved for future features"). So there is no producer to port, on this branch or the reference. This PR adds the structural hook (a static flag +opendisplay_ble_set_connection_requested(bool)in the public header) so a future feature / sibling branch can drive it; bit2 stays 0 until then, matching the reference exactly.Fix 3 — On Zephyr + SoftDevice Controller there is no stable
bt_le_*runtime TX-power API, so this uses the standard HCI vendor-specificWrite_Tx_Power_Levelcommand (the approach in Zephyr'shci_pwr_ctrlsample), gated onCONFIG_BT_HCI_VS(now enabled explicitly inprj.conf). The controller clamps the requested dBm to its supported set and returns the value it actually selected, which is logged (clamp + log, no hand-rolled table). The config'stx_powerbyte is interpreted as a signed int8 dBm, matching the reference's direct pass-through. Note: this Zephyr version renamedbt_hci_cmd_create()tobt_hci_cmd_alloc()(opcode/len now set bybt_hci_cmd_send_sync); the code uses the new name.Fix 6 — This composes with the separate DFU-honesty question:
0x0051(ENTER_DFU) still falsely ACKs on nRF54; that is intentionally left for a separate change and not touched here.Fix 7 — Verified only, no code change: both sides emit exactly 6 uppercase hex digits (the reference from
DEVICEID[1] & 0xFFFFFF, nRF54 fromhwinfomasked to 24 bits via%06lX). Value source differs by chip, but the width — the thing clients key device names on — matches.Hardware testing
I have no hardware, so nothing here was runtime-tested — everything below is compile-verified only (both CI envs green). A hardware smoke test should confirm:
[OD] tx_power … requested=X selected=Y dBmlog line appears at boot and on config reload,selectedis sane for the nRF54L radio, and RSSI at a fixed distance tracks the configured value. Also confirm the per-connection apply on connect does not disturb the link.pulldowns-configured input reads correctly.Stacking & conflicts
feat/ci): this branch is based onfeat/ci, so this PR's diff againstmaincurrently includes the CI workflow from ci: build both nRF54 environments on push and PR #1. Merge after ci: build both nRF54 environments on push and PR #1.opendisplay_ble.c/opendisplay_led.c/opendisplay_button.c: feat: add passive buzzer support (opcode 0x0077 + config packet 0x29) #3 (feat/buzzer), feat: add GT911 touch controller support (config packet 0x28) #5 (feat/touch-gt911), feat: battery measurement and SHT40/BQ27220 sensor telemetry #7 (feat/sensors-battery). These are textual, not semantic — this PR deliberately stays based onfeat/ciand does not pull in their changes. TheconnectionRequestedsetter from Fix 2 is the intended hook for producers those branches may add.🤖 Generated with Claude Code
https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR