Fix charging wake radio startup#84
Conversation
tadelv
left a comment
There was a problem hiding this comment.
Approve. No WiFi.setSleep(false) introduced (coexistence footgun #2 not tripped). Button wake runs on the loop task, so ble_init() / wifi_init() are safe to call there. The if (!b_ble_enabled) / if (!b_wifiEnabled) guards are a real improvement — the old path called ble_init() unconditionally on every charging wake, and ble_init() is not idempotent (BLEDevice::init without de-init). Cross-task flags used in the guards are already volatile.
Minor (non-blocking):
b_is_charging = falseinwakeFromChargingUiis misleading dead code — the USB cable is still connected, so the device is still charging. The loop re-assertsb_is_charging = truenext pass anyway, andpower_off(-1)in the button wrapper ran against the pre-clear value, so no functional bug, but the line reads wrong. Either drop it or mirror the real condition (digitalRead(USB_DET/BATTERY_CHARGING) == HIGH).- The contract test doesn't pin the guard clauses it's meant to protect — a refactor that drops
if (!b_ble_enabled)/if (!b_wifiEnabled)still passes. Add assertions for those. - PR body slightly overstates WiFi coverage:
wifi_init()early-returns on!b_wifiOnBoot, so WiFi starts on charging wake only if the user opted into WiFi-on-boot in the menu. Code is consistent with boot behavior; only the description overstates it.
Suggestion (separate from this PR): same testing note as #81/#83 — the contract test is static string-matching. Moving the wake/init logic into a pure host-compilable unit with real unit tests would catch the guard-drop regression the current test can't.
Summary
Validation