fix: unbrick boot — revert -S6 Clear(), add reentrancy guard, S7
Build Firmware / Build stm32-teslacharger (pull_request) Successful in 1m1s

PR #8 (-S6) caused chip boot fault: MapChargerMessages() uncommented
canMap->Clear(). Clear() ends with canHardware->ClearUserMessages(),
which fires the HandleClear callback (main.cpp:170-173) — and
HandleClear calls MapChargerMessages(). Recursive chain:

  main() -> MapChargerMessages
         -> Clear() -> ClearUserMessages -> HandleClear
                                         -> MapChargerMessages (re-entry)
                                            -> Clear() (recurse...)
                                            -> ... stack overflow -> HardFault

Bench symptom (Kyle): ESP8266 web UI stops responding after flashing
-S6. Recovery required re-flashing an older firmware (3-5 attempts).

This commit:
1. Re-commented canMap->Clear() inside MapChargerMessages — the safe
   path. The upgrade rebuild now requires a manual "canclear" via the
   openinverter terminal/web UI, which fires HandleClear once and
   triggers an additive MapMessages + Save.
2. Added a static `in_progress` reentrancy guard so any future
   accidental Clear() inside this function or its callees turns into
   a no-op re-entry instead of a chip-bricking fault.
3. Kept the canary as Param::tmpobcmax (from PR #8) — useful as a
   schema marker, just no longer auto-triggers a destructive rebuild.
4. Bumped VERSTR S5 -> S7 (skips S6 so any bench reporting "S6" in
   the openinverter UI is visibly the broken build and gets flashed).

Note: this does NOT solve the underlying "73 entries trying to fit in
50-slot CANPOS pool" issue — chargercan.cpp still has more AddRecv +
AddSend calls than MAX_ITEMS. Step 2 (reduce entries) is a separate
follow-up PR.

#patch

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bastian de Byl
2026-05-15 11:30:56 -04:00
parent 2c9e7a8bbe
commit 23716a260b
2 changed files with 32 additions and 6 deletions
+6 -1
View File
@@ -129,7 +129,12 @@
// SKUDAK customization suffix: bump on every Skudak-side change so the OpenInverter
// web UI shows a distinct version (e.g. "4=1.20.R-S4") and we can visually confirm
// the right firmware is flashed. Match the stm32-sine -S<N> convention.
#define VERSTR STRINGIFY(4=VER-S5)
// -S6 was withdrawn (PR #8 closed): the canary-rebuild attempt called
// canMap->Clear() inside MapChargerMessages, which recursed through the
// HandleClear callback and stack-overflowed at boot before the terminal
// came up. Jumping straight to -S7 so any bench that still reports -S6
// in the openinverter UI is visibly the broken build and gets re-flashed.
#define VERSTR STRINGIFY(4=VER-S7)
/***** enums ******/
+26 -5
View File
@@ -108,20 +108,41 @@ static void Ms100Task(void)
static void MapChargerMessages()
{
// SKUDAK: Reentrancy guard. canMap->Clear() (libopeninv canmap.cpp) ends
// with canHardware->ClearUserMessages(), which fires the HandleClear
// callback below — which calls back into THIS function. If this function
// ever calls Clear() itself, the chain becomes infinite recursion → stack
// overflow → HardFault before the terminal/web UI come up (this is what
// bricked -S6 on Kyle's bench, see PR #8 postmortem). The static guard
// makes any future "accidental Clear() inside the rebuild path" a no-op
// re-entry rather than a chip-bricking fault. Safe because this function
// is only invoked from main() and from the HandleClear callback, both
// single-threaded contexts; never from an ISR.
static bool in_progress = false;
if (in_progress) return;
uint32_t dummyId;
uint8_t dummyOfs;
int8_t dummyAdd, dummyLen;
float dummyGain;
bool dummyrx;
//check sample value, if it is mapped assume valid CAN map
if (canMap->FindMap(Param::hwaclim, dummyId, dummyOfs, dummyLen, dummyGain, dummyAdd, dummyrx)) return;
//canMap->Clear();
// SKUDAK: tmpobcmax is the canary for the -S5+ CAN-map schema. If it's
// already mapped, the persistent flash map is up-to-date — preserve it
// and any user customizations. If it's NOT mapped, the saved map is
// from an older firmware that didn't broadcast OBC temp on 0x211 byte 7.
//
// We deliberately do NOT call canMap->Clear() here. Clear() triggers
// HandleClear, and the recursion is fatal (see guard comment above). The
// upgrade path is: user issues "canclear" via the openinverter terminal
// or web UI to nuke the stale map manually. That single Clear fires
// HandleClear once, this function re-runs additively, and Save() persists.
if (canMap->FindMap(Param::tmpobcmax, dummyId, dummyOfs, dummyLen, dummyGain, dummyAdd, dummyrx)) return;
in_progress = true;
ChargerCAN::MapMessages(canMap);
canMap->Save();
in_progress = false;
}
/** This function is called when the user changes a parameter */