From 5dc715558f69b8d945df7834a2d11bece3d85576 Mon Sep 17 00:00:00 2001 From: Bastian de Byl Date: Sun, 10 May 2026 09:34:17 -0400 Subject: [PATCH] fix: CheckDelay returns false forever when timedly is negative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream openinverter bug surfaced after recent SKUDAK PRs. Symptom (reported by Kyle on the bench): teslacharger detects EVSE, shows the AC limit, but stays stuck in WaitStart and never proceeds to Enable / Activate / Run. State machine looks like it's the OFF state but it's actually one step further on. Root cause in src/charger.cpp:85-91: bool CheckDelay() { uint32_t now = rtc_get_counter_val(); uint32_t start = Param::GetInt(Param::timedly) * 60; return start <= 0 || (now - startTime) > start; } With timedly = -1 (the param's documented "no delay" sentinel and its default value), the math is: Param::GetInt(timedly) -> -1 (signed int) -1 * 60 -> -60 (signed) assigned to uint32_t -> 0xFFFFFFC4 (~4.29 billion) start <= 0 -> false (uint32_t can't be negative) (now - startTime) > 4294967236 -> false for ~136 years So CheckDelay() returns false forever; charger wedges in WaitStart. Why "it was working before": every SKUDAK PR that bumped the param table (uaux broadcast, expanded 0x210, 0x210->0x211, version suffix) invalidates the saved-params CRC and resets timedly to its -1 default. Kyle had a non-default value (probably 0) saved before. Fix: signed math for the short-circuit so timedly <= 0 still means "start immediately." Cast to uint32_t only on the right operand, which is only evaluated when start > 0 — can't reinterpret negative as huge. The companion CheckTimeout() at line 75 has the same uint32_t pattern but works "correctly by accident" because timelim = -1 makes timeout huge and (now - startTime) > timeout is false, which matches the intended "no timeout" semantics. Leaving that alone — fixing it would change behaviour, not bugs. VER suffix bumped -S1 -> -S2 so the OI web UI clearly shows the deployed firmware (post-fix). Confirmed in built binary as "4=1.19.R-S2". Build: text=25192, links clean, pre-existing pedantic warning unchanged. This bug is also present upstream (johanneshuebner/stm32-charger); worth a PR upstream once we've confirmed the fix on the bench. Co-Authored-By: Claude Opus 4.7 (1M context) --- include/param_prj.h | 4 ++-- src/charger.cpp | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/include/param_prj.h b/include/param_prj.h index 6731ea7..f4ac6f4 100644 --- a/include/param_prj.h +++ b/include/param_prj.h @@ -125,9 +125,9 @@ #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.19.R-S1") and we can visually confirm +// web UI shows a distinct version (e.g. "4=1.19.R-S2") and we can visually confirm // the right firmware is flashed. Match the stm32-sine -S convention. -#define VERSTR STRINGIFY(4=VER-S1) +#define VERSTR STRINGIFY(4=VER-S2) /***** enums ******/ diff --git a/src/charger.cpp b/src/charger.cpp index db1c528..b3cd21d 100644 --- a/src/charger.cpp +++ b/src/charger.cpp @@ -85,9 +85,15 @@ bool CheckTimeout() bool CheckDelay() { uint32_t now = rtc_get_counter_val(); - uint32_t start = Param::GetInt(Param::timedly) * 60; + // Upstream uses uint32_t here, which wraps a negative timedly (the param's + // documented -1 "no delay" sentinel) into a ~4 billion-second timeout that + // never expires — leaving the state machine wedged in WaitStart forever. + // Use signed math for the short-circuit so timedly <= 0 still means + // "start immediately." The cast on the elapsed-comparison side is only + // reached when start > 0, so it can't reinterpret a negative as huge. + int start = Param::GetInt(Param::timedly) * 60; - return start <= 0 || (now - startTime) > start; + return start <= 0 || (now - startTime) > (uint32_t)start; }