Companion charging flag + board power detection fixes (T1000-E, WisMesh Tag)#1710
Companion charging flag + board power detection fixes (T1000-E, WisMesh Tag)#1710Specter242 wants to merge 6 commits intomeshcore-dev:mainfrom
Conversation
Append an optional is_charging byte to RESP_CODE_BATT_AND_STORAGE using board external power state so clients can render a charging indicator without breaking older parsers. Co-authored-by: Cursor <cursoragent@cursor.com>
Use T1000-E specific EXT_PWR_DETECT and EXT_CHRG_DETECT lines in isExternalPowered() so the companion battery charging byte reflects actual hardware charging/power state on this board. Co-authored-by: Cursor <cursoragent@cursor.com>
variants/t1000-e/T1000eBoard.cpp
Outdated
|
|
||
| bool T1000eBoard::isExternalPowered() { | ||
| // T1000-E exposes dedicated detect lines for external power and charge state. | ||
| // Use these first, then fall back to NRF52 USB VBUS detection. |
There was a problem hiding this comment.
Does NRF52 fallback provide us with anything? Wouldn't it always be false currently? :)
There was a problem hiding this comment.
Good catch — you're essentially right, and it's worth explaining the reasoning so it doesn't look accidental.
Why EXT_PWR_DETECT / EXT_CHRG_DETECT already cover everything
Both pins are unconditionally defined in variants/t1000-e/variant.h:
#define EXT_CHRG_DETECT (35) // P1.3 — PMIC charge-status, active-LOW
#define EXT_PWR_DETECT (5) // P0.5 — external power rail present, active-HIGH
Because neither is behind a conditional, the two #ifdef guards in isExternalPowered() are always satisfied at compile time. Those two pin reads together cover every charging scenario the T1000-E supports (magnetic pogo-pin connector).
Why the NRF52 USB fallback is dead code in practice
NRF52Board::isExternalPowered() reads NRF_POWER->USBREGSTATUS.VBUSDETECT (or its SoftDevice equivalent). The T1000-E's charging PMIC is wired to the pogo connector, not to the nRF52840 USB VBUS line, so in any real charging scenario USBREGSTATUS stays 0 and the fallback contributes nothing.
The only case where it would ever be non-zero is when a USB cable is plugged in for DFU/flashing — not a charging event and not something you'd want to report to the companion app as "externally powered."
Why I left it in
It was originally a defensive backstop for the case where a board derives from T1000eBoard without EXT_PWR_DETECT/EXT_CHRG_DETECT defined, and to keep the call chain consistent with the base class. In hindsight, with both pins unconditionally defined in the canonical variant, it's more confusing than helpful.
I'll remove the NRF52Board::isExternalPowered() tail call from T1000eBoard::isExternalPowered() — the function can just return externalPowerDetected || chargingDetected;. Happy to push that cleanup if that direction works for you.
There was a problem hiding this comment.
Pushed in aade3b9. I removed the NRF52Board::isExternalPowered() tail call here, so T1000-E power reporting now relies only on EXT_PWR_DETECT and EXT_CHRG_DETECT. That makes the variant behavior explicit instead of keeping the generic nRF52 USB/VBUS fallback in the mix.
Specter242
changed the title
examples/companion_radio/MyMesh.cpp
Outdated
| uint16_t battery_millivolts = board.getBattMilliVolts(); | ||
| uint32_t used = _store->getStorageUsedKb(); | ||
| uint32_t total = _store->getStorageTotalKb(); | ||
| // Optional extra byte for companion clients: |
There was a problem hiding this comment.
is the extra byte always included? would this break existing parsers of this message type?
There was a problem hiding this comment.
Yes, in this PR the extra byte is always included now. The response is 12 bytes total: battery mV, used storage, total storage, plus the trailing charging/external-power flag. Existing parsers should tolerate the longer frame and ignore the final byte if they do not consume charging state yet. I also pushed f339514 to clarify that in both the inline comment and docs/companion_protocol.md.
3 participants
Summary
This PR enables reliable companion charging-state reporting for app battery indicators by:
is_chargingbyte toRESP_CODE_BATT_AND_STORAGE (0x0C)Closes #1870.
Problem
Companion battery status was available, but charging state was not consistently surfaced across devices. As a result, charging indicators in client apps could remain off even while externally powered.
Root Cause
Changes
examples/companion_radio/MyMesh.cppis_chargingbyte inRESP_CODE_BATT_AND_STORAGE.variants/t1000-e/T1000eBoard.cppsrc/helpers/NRF52Board.cppdocs/companion_protocol.mdCompatibility
is_chargingis appended as an optional trailing byte.>= 12.Validation
pio run -e t1000e_companion_radio_blepio run -e RAK_WisMesh_Tag_companion_radio_ble/dev/ttyACM2) with BLE companion firmware:adafruit-nrfutil dfu serial --package .pio/build/RAK_WisMesh_Tag_companion_radio_ble/firmware.zip -p /dev/ttyACM2 -b 115200 --singlebank --touch 1200sz=12), and app-side charging indicator now updates correctly.