feat: S11 - explicit VCU stop command on 0x212 byte 3 (replaces udclim hack)

The -S10 udclim=200V mechanism was a hack — VCU lowered udclim below
pack to fire CheckVoltage() udc>udclim and force STOP. It didn't work
reliably because once VCU opens HV contactors, the OBC per-module
voltage probes (c1udc/c2udc/c3udc) collapse below 200V and udc =
MAX(c1,c2,c3) never accumulates 10 ticks above udclim. Bench symptom:
VCU reaches CHARGE_COMPLETE, opens contactors, but teslacharger stays
in EVSEACTIVATE, OBC modules keep drawing ~0.5kW AC, water pumps run,
CP signal stays charging, EVSE doesn't shut down.

S11 adds an explicit vcustop byte on 0x212 byte 3 — VCU sets it to 1
when it wants charging stopped. ChargerStateMachine checks vcustop
FIRST in the EVSEACTIVATE case and transitions to STOP immediately.
Mirrors how the Dilong OBC obeys OBC_ControlCMD = Stopped.

Wire format (additive, no breaking change to S10):
  Byte 0:    vcuchglim_pct  (existing)
  Bytes 1-2: udclim_V       (existing, DEPRECATED — VCU sends 398V no-op)
  Byte 3:    vcustop        (NEW, 0=normal / 1=force stop)
  Bytes 4-7: reserved

Defensive: Param::Change(udclim) now clamps received <50V values to the
398V flash default. Protects against a pre-PR47 VCU sending all-zeros
in bytes 1-2 (which would otherwise make CheckVoltage() fire constantly
and brick charging).

After this VCU also flashed: the WPUMP issue resolves naturally because
VCU_IsCharging() reads OIOBC.State < ACTIVATE once teslacharger goes
to STOP. The VCU/teslacharger ownership becomes symmetric with the
Dilong path — VCU is authoritative for "should we charge."

Pool: 43 → 44 / 50 slots used.

#patch

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.gitignore

@@ -4,3 +4,4 @@ stm32_charger*
 linker.map
 *.layout
 *.out
+.DS_Store

include/param_prj.h

@@ -47,7 +47,7 @@
    3. Display values
  */
 //Next param id (increase when adding new parameter!): 24
-//Next value Id: 2051
+//Next value Id: 2053
 /*              category     name         unit       min     max     default id */
 #define PARAM_LIST \
     PARAM_ENTRY(CAT_CHARGER, idclim,      "A",       0,      45,     45,     3   ) \
@@ -108,6 +108,8 @@
     VALUE_ENTRY(c3iac,       "A",     2034 ) \
     VALUE_ENTRY(c3udc,       "V",     2035 ) \
     VALUE_ENTRY(c3idc,       "A",     2036 ) \
+    VALUE_ENTRY(tmpobcmax,   "°C",    2051 ) \
+    VALUE_ENTRY(vcustop,     OFFON,   2052 ) \
     VALUE_ENTRY(test_time,   "s",     3000 ) \
     VALUE_ENTRY(test_timer_flag,   "X",     3001 ) \
     VALUE_ENTRY(test_timer_icvalue,   "X",     3002 ) \
@@ -126,9 +128,30 @@
 #define CAT_COMM     "Communication"
 
 // SKUDAK customization suffix: bump on every Skudak-side change so the OpenInverter
-// web UI shows a distinct version (e.g. "4=1.20.R-S3") and we can visually confirm
+// 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-S3)
+// -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.
+// -S8 trims chargercan.cpp from 73 → 37 entries so the libopeninv 50-slot
+// pool doesn't silently drop the SKUDAK 0x211 broadcast on rebuild.
+// -S9 restores 8 c2/c3 RX entries (uac/flag/idc/udc per module). -S8 over-
+// trimmed multi-module Tesla OBC support: CalcTotals was summing only
+// module 1's contribution, so reported pack power was 1/3 of actual delivery.
+// -S10 added udclim RX on 0x212 bytes 1-2. The VCU sent udclim=200V to force
+// CheckVoltage() to fire (udc > udclim → STOP). DEPRECATED in -S11: the
+// per-module DC voltage readings (c1udc/c2udc/c3udc) collapse below 200V
+// once VCU opens HV contactors, so the trigger never accumulates 10 ticks.
+// udclim RX is kept mapped but the VCU now always sends 398V (no-op).
+//
+// -S11 adds vcustop RX on 0x212 byte 3 — an explicit VCU command lever that
+// mirrors the Dilong path's OBC_ControlCMD. When VCU enters CHARGE_COMPLETE
+// it sets vcustop=1; ChargerStateMachine's EVSEACTIVATE case transitions to
+// STOP immediately, the J1772 CP signal drops, EVSE stops delivering AC,
+// VCU_IsCharging() returns 0, water pumps power down naturally.
+#define VERSTR STRINGIFY(4=VER-S11)
 
 /***** enums ******/
 

src/charger.cpp

@@ -353,15 +353,28 @@ void ChargerStateMachine()
          DigIo::evseact_out.Set();
          DigIo::acpres_out.Set();
 
-         if (CheckVoltage() || CheckTimeout())
+         // SKUDAK-S11: explicit VCU stop command takes priority over every
+         // other exit condition. When the VCU's BMS-side gate determines the
+         // charge session is complete (or the user lowered ChargeLimit below
+         // current SOC), it sets vcustop=1 on 0x212 byte 3. We obey
+         // immediately — drop AC pres + EVSE-active GPIOs and transition to
+         // STOP. Mirrors how the Dilong path's OBC_ControlCMD = Stopped works.
+         // Replaces the brittle udclim=200V mechanism from -S10.
+         if (Param::GetInt(Param::vcustop))
+         {
+            DigIo::acpres_out.Clear();
+            DigIo::evseact_out.Clear();
             state = STOP;
-         if (CheckUnplugged())
+         }
+         else if (CheckVoltage() || CheckTimeout())
+            state = STOP;
+         else if (CheckUnplugged())
          {
             DigIo::acpres_out.Clear();
             DigIo::evseact_out.Clear();
             state = OFF;
          }
-         if (CheckChargerFaults())
+         else if (CheckChargerFaults())
          {
             DigIo::acpres_out.Clear();
             state = OFF;

src/chargercan.cpp

@@ -18,47 +18,62 @@
  */
 #include "chargercan.h"
 
+// SKUDAK-S8/S9: trimmed entry list to fit libopeninv's MAX_ITEMS=50 CANPOS pool.
+// The upstream teslacharger registers ~73 AddRecv+AddSend mappings, which
+// silently overflows the 50-slot pool and drops the last ~23 entries
+// (including the SKUDAK 0x211 broadcast). We dropped things we don't use:
+//
+//   - hwaclim duplicates on 0x209/0x20B (keep just 0x207; the three modules
+//     broadcast identical hardware AC limit values)                       -2
+//   - c2*/c3* iac, stt, tmp1/2/in (12 entries — full per-module diagnostics
+//     not needed; CheckChargerFaults only uses uac+flag, CalcTotals only
+//     uses idc+udc, and tmpobcmax aggregates fine from c1's 3 probes)    -12
+//   - CHAdeMO RX on 0x102 (canenable/idcspnt/udclim/soc — Polarity VCU
+//     doesn't use CHAdeMO at all)                                         -4
+//   - 0x368 version-only idle frame (purely cosmetic for stock OI UI)     -7
+//   - CHAdeMO TX on 0x108/0x109 (version, idclim, udc, idc, opmode —
+//     unused on this build)                                               -5
+//
+// Total dropped: 30 entries. From 73 → 42, with 8-slot headroom.
+//
+// -S9 RESTORED 8 entries the -S8 trim wrongly killed for 3-module Tesla OBCs:
+// c2/c3 uac (CheckChargerFaults active2/active3 detection),
+// c2/c3 flag (clears CheckAlive timeout under fault-check logic),
+// c2/c3 idc (CalcTotals current SUM — was reporting 1/3 of true delivery),
+// c2/c3 udc (CalcTotals voltage MAX). Without these the bench reported
+// ~2.7kW for what was actually 9.7kW going to the pack.
+//
+// 0x43c/0x44c TX kept on purpose — they're commands to OBC sub-modules 2/3,
+// required for multi-module operation. Drop them too only if pool pressure
+// returns and you've confirmed the bench is single-module.
 void ChargerCAN::MapMessages(CanMap* can)
 {
    can->AddRecv(Param::hwaclim, 0x207, 32, 9, 0.06666f); //gain 0.06666
-   can->AddRecv(Param::hwaclim, 0x209, 32, 9, 0.06666f); //gain 0.06666
-   can->AddRecv(Param::hwaclim, 0x20B, 32, 9, 0.06666f); //gain 0.06666
    can->AddRecv(Param::c1iac, 0x207, 41, 9, 0.06666f); //gain 0.06666
-   can->AddRecv(Param::c2iac, 0x209, 41, 9, 0.06666f); //gain 0.06666
-   can->AddRecv(Param::c3iac, 0x20B, 41, 9, 0.06666f); //gain 0.06666
    can->AddRecv(Param::c1uac, 0x207, 8, 8, 1);
-   can->AddRecv(Param::c2uac, 0x209, 8, 8, 1);
-   can->AddRecv(Param::c3uac, 0x20B, 8, 8, 1);
    can->AddRecv(Param::c1flag, 0x207, 17, 2, 1);
-   can->AddRecv(Param::c2flag, 0x209, 17, 2, 1);
-   can->AddRecv(Param::c3flag, 0x20B, 17, 2, 1);
    can->AddRecv(Param::c1stt, 0x217, 0, 8, 1);
-   can->AddRecv(Param::c2stt, 0x219, 0, 8, 1);
-   can->AddRecv(Param::c3stt, 0x21B, 0, 8, 1);
    can->AddRecv(Param::c1idc, 0x227, 32, 16, 0.000839233f); //gain 0.000839233
-   can->AddRecv(Param::c2idc, 0x229, 32, 16, 0.000839233f); //gain 0.000839233
-   can->AddRecv(Param::c3idc, 0x22B, 32, 16, 0.000839233f); //gain 0.000839233
    can->AddRecv(Param::c1udc, 0x227, 16, 16, 0.01052856f); //gain 0.01052856
-   can->AddRecv(Param::c2udc, 0x229, 16, 16, 0.01052856f); //gain 0.01052856
-   can->AddRecv(Param::c3udc, 0x22B, 16, 16, 0.01052856f); //gain 0.01052856
    can->AddRecv(Param::c1tmp1, 0x237, 0, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c2tmp1, 0x239, 0, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c3tmp1, 0x23B, 0, 8, 1, -40); //offset -40°C
    can->AddRecv(Param::c1tmp2, 0x237, 8, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c2tmp2, 0x239, 8, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c3tmp2, 0x23B, 8, 8, 1, -40); //offset -40°C
    can->AddRecv(Param::c1tmpin, 0x237, 40, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c2tmpin, 0x239, 40, 8, 1, -40); //offset -40°C
-   can->AddRecv(Param::c3tmpin, 0x23B, 40, 8, 1, -40); //offset -40°C
-   //We don't have enough space for all messages, so we discard these
-   //can->AddRecv(Param::c1tmplim, 0x247, 0, 8, 7); //gain 0.234375
-   //can->AddRecv(Param::c2tmplim, 0x249, 0, 8, 7); //gain 0.234375
-   //can->AddRecv(Param::c3tmplim, 0x24B, 0, 8, 7); //gain 0.234375
-   /***** CHAdeMO RX *****/
-   can->AddRecv(Param::canenable, 0x102, 40, 1, 1);
-   can->AddRecv(Param::idcspnt,   0x102, 24, 8, 1);
-   can->AddRecv(Param::udclim,    0x102, 8, 16, 1);
-   can->AddRecv(Param::soc,       0x102, 48, 8, 1);
+
+   // SKUDAK-S9: module 2 RX. uac/flag prevent the CheckChargerFaults false-
+   // positive that would silently mask module-2 failure (active2 = (chargerena
+   // & 2) && (c2uac > 70V); without c2uac mapped, active2==false short-circuits
+   // the flag check). idc/udc are required so CalcTotals reports the SUM
+   // current and MAX voltage across all three Tesla OBC sub-modules.
+   can->AddRecv(Param::c2uac,  0x209, 8,  8,  1);
+   can->AddRecv(Param::c2flag, 0x209, 17, 2,  1);
+   can->AddRecv(Param::c2idc,  0x229, 32, 16, 0.000839233f);
+   can->AddRecv(Param::c2udc,  0x229, 16, 16, 0.01052856f);
+
+   // SKUDAK-S9: module 3 RX, same rationale.
+   can->AddRecv(Param::c3uac,  0x20B, 8,  8,  1);
+   can->AddRecv(Param::c3flag, 0x20B, 17, 2,  1);
+   can->AddRecv(Param::c3idc,  0x22B, 32, 16, 0.000839233f);
+   can->AddRecv(Param::c3udc,  0x22B, 16, 16, 0.01052856f);
 
    /***** Charger TX ******/
    can->AddSend(Param::udcspnt, 0x45c, 0, 16, 100); //gain 100
@@ -85,39 +100,43 @@ void ChargerCAN::MapMessages(CanMap* can)
    can->AddSend(Param::aclim, 0x44c, 16, 16, 1500);
    can->AddSend(Param::opmode, 0x44c, 32, 8, 154, 100);
 
-   can->AddSend(Param::version, 0x368, 0, 8, 0, 0x03);
-   can->AddSend(Param::version, 0x368, 8, 8, 0, 0x49);
-   can->AddSend(Param::version, 0x368, 16, 8, 0, 0x29);
-   can->AddSend(Param::version, 0x368, 24, 8, 0, 0x11);
-   can->AddSend(Param::version, 0x368, 40, 8, 0, 0x0c);
-   can->AddSend(Param::version, 0x368, 48, 8, 0, 0x40);
-   can->AddSend(Param::version, 0x368, 56, 8, 0, (int8_t)0xff);
-
-   /***** CHAdeMO TX *****/
-   can->AddSend(Param::version, 0x108, 8, 16, 107); //output 428V max = 4*107
-   can->AddSend(Param::idclim, 0x108, 24, 8, 1);
-   can->AddSend(Param::udc, 0x109, 8, 16, 1);
-   can->AddSend(Param::idc, 0x109, 24, 16, 1);
-   can->AddSend(Param::opmode, 0x109, 40, 3, 5); //Set charging and connlock at once
-
-   /***** SKUDAK VCU command RX (0x212) — SKUDAK-516 user ChargeLimit *****/
-   // Byte 0: ChargeLimit_pct (0-100). Bytes 1-7 reserved 0x00.
-   // Param::Change(vcuchglim) translates % to udcspnt setpoint (see src/main.cpp).
-   // VCU broadcasts every 200 ms once teslacharger detected; charger holds last
-   // value if VCU goes silent.
-   can->AddRecv(Param::vcuchglim, 0x212, 0, 8, 1);
+   /***** SKUDAK VCU command RX (0x212) — SKUDAK-516 chglim + S10 udclim + S11 stop *****/
+   // Byte 0:    ChargeLimit_pct (0-100). Param::Change(vcuchglim) translates
+   //            % to udcspnt setpoint (see src/main.cpp).
+   // Bytes 1-2: udclim_V (uint16 LE). DEPRECATED in -S11 but mapping kept for
+   //            backward compat with -S10 VCU TX. New VCU always sends 398V
+   //            (= the flash default), so this RX is a no-op. Old -S10 trick
+   //            (VCU sends 200V to force CheckVoltage() STOP) was unreliable
+   //            because c1udc/c2udc/c3udc collapse < 200V once VCU opens HV
+   //            contactors, never accumulating 10 ticks > udclim. Defensive
+   //            clamp in Param::Change(udclim) ignores values < 50 V (would
+   //            otherwise turn an old-VCU bench into a never-charges brick).
+   // Byte 3:    vcustop (uint8, NEW in -S11). 0 = no override / normal state
+   //            machine. 1 = VCU commands STOP. Checked at the top of
+   //            EVSEACTIVATE in ChargerStateMachine; transition to STOP fires
+   //            within one 100 ms tick. Mirrors how the Dilong path uses
+   //            OBC_ControlCMD = Stopped to authoritatively halt charging.
+   // Bytes 4-7: reserved 0x00.
+   // VCU broadcasts every 200 ms once teslacharger detected; charger holds
+   // last received values if VCU goes silent.
+   can->AddRecv(Param::vcuchglim, 0x212, 0,  8,  1);
+   can->AddRecv(Param::udclim,    0x212, 8,  16, 1);
+   can->AddRecv(Param::vcustop,   0x212, 24, 8,  1);
 
    /***** SKUDAK VCU telemetry (0x211) — SKUDAK-448 expanded charger telemetry *****/
-   // Byte 0:    state (0=Off, 1=WaitStart, 2=Enable, 3=Activate, 4=Run, 5=Stop)
-   // Bytes 1-2: udc (V, gain=1)
-   // Bytes 3-4: idc (0.1A, gain=10)
-   // Byte 5:    uaux (decivolts, gain=10) — 12V aux battery rail
-   // Byte 6:    soc (%, gain=1)
-   // Byte 7:    reserved
+   // Byte 0:    state     (0=Off, 1=WaitStart, 2=Enable, 3=Activate, 4=Run, 5=Stop)
+   // Bytes 1-2: udc       (V, gain=1)
+   // Bytes 3-4: idc       (0.1A, gain=10)
+   // Byte 5:    uaux      (decivolts, gain=10) — 12V aux battery rail
+   // Byte 6:    soc       (%, gain=1)
+   // Byte 7:    tmpobcmax (°C, gain=1, offset=-40)  — worst-case across all
+   //                       9 OBC probes; matches Tesla OBC's own CAN offset
+   //                       so receivers can decode with the same formula.
    // Consumed by stm32-hal-vcu Core/Src/main.c CHARGER_CAN_ID_OI_OBC_AUX handler.
-   can->AddSend(Param::state, 0x211,  0, 8,  1);
-   can->AddSend(Param::udc,   0x211,  8, 16, 1);
-   can->AddSend(Param::idc,   0x211, 24, 16, 10);
-   can->AddSend(Param::uaux,  0x211, 40, 8,  10);
-   can->AddSend(Param::soc,   0x211, 48, 8,  1);
+   can->AddSend(Param::state,     0x211,  0, 8,  1);
+   can->AddSend(Param::udc,       0x211,  8, 16, 1);
+   can->AddSend(Param::idc,       0x211, 24, 16, 10);
+   can->AddSend(Param::uaux,      0x211, 40, 8,  10);
+   can->AddSend(Param::soc,       0x211, 48, 8,  1);
+   can->AddSend(Param::tmpobcmax, 0x211, 56, 8,  1, 40);  // offset 40 → wire byte = temp + 40
 }

src/main.cpp

@@ -77,25 +77,72 @@ static void Ms100Task(void)
 
    EvseRead();
 
+   // SKUDAK: derive a single "worst case" OBC temperature for downstream
+   // consumers (Polarity VCU broadcasts it on STATUS_SCREEN bytes 23/26).
+   // Aggregates the 9 per-module probes the Tesla OBC publishes on CAN
+   // 0x237/0x239/0x23B — max wins. Modules without a sensor read -40 °C
+   // (the DBC offset; raw byte 0); we ignore those so an absent module
+   // doesn't drag the worst-case down. If no probes are valid we leave
+   // tmpobcmax at -40 °C, which the VCU will display as "—".
+   {
+      const Param::PARAM_NUM probes[] = {
+         Param::c1tmp1, Param::c1tmp2, Param::c1tmpin,
+         Param::c2tmp1, Param::c2tmp2, Param::c2tmpin,
+         Param::c3tmp1, Param::c3tmp2, Param::c3tmpin,
+      };
+      int worst   = -40;
+      bool any_ok = false;
+      for (unsigned i = 0; i < sizeof(probes) / sizeof(probes[0]); i++) {
+         int t = Param::GetInt(probes[i]);
+         if (t <= -40) continue;  // No sensor / module not present
+         if (!any_ok || t > worst) {
+            worst  = t;
+            any_ok = true;
+         }
+      }
+      Param::SetInt(Param::tmpobcmax, any_ok ? worst : -40);
+   }
+
    canMap->SendAll();
 }
 
 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 */
@@ -114,6 +161,18 @@ void Param::Change(Param::PARAM_NUM paramNum)
          spnt = MIN(Param::Get(Param::idcspnt), Param::Get(Param::idclim));
          dcCurController.SetRef(spnt);
          break;
+      case Param::udclim:
+         // SKUDAK-S11: defensive clamp. udclim is now reverse-mapped from CAN
+         // (chargercan.cpp), and a pre-PR47 VCU sends 0 in 0x212 bytes 1-2.
+         // udclim=0 would make CheckVoltage() fire on every tick (udc > 0)
+         // and prevent any charging. If we see a value below the documented
+         // min (50V), restore the safe default. Trivial cost — fires only on
+         // CAN RX of udclim (200 ms cadence) and only ever does work when
+         // the value is bogus.
+         if (Param::GetInt(Param::udclim) < 50) {
+            Param::SetInt(Param::udclim, 398);
+         }
+         break;
       case Param::vcuchglim:
       {
          // SKUDAK-516: VCU sends user-set ChargeLimit_pct on CAN 0x212. Translate
@@ -158,7 +217,11 @@ extern "C" void tim2_isr(void)
    scheduler->Run();
 }
 
-extern "C" int main(void)
+// C++ forbids a linkage specification on `main` (-Wpedantic). The reset_handler
+// in libopencm3's vector.c forward-declares `int main(void)` with default
+// linkage and resolves it at link time by symbol name, so dropping `extern "C"`
+// here doesn't affect how main is invoked at boot.
+int main(void)
 {
    extern const TERM_CMD termCmds[];