fix: CheckDelay returns false forever when timedly is negative
Build Firmware / Build stm32-teslacharger (pull_request) Successful in 1m1s
Build Firmware / Build stm32-teslacharger (pull_request) Successful in 1m1s
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) <noreply@anthropic.com>
This commit is contained in:
+2
-2
@@ -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<N> convention.
|
||||
#define VERSTR STRINGIFY(4=VER-S1)
|
||||
#define VERSTR STRINGIFY(4=VER-S2)
|
||||
|
||||
/***** enums ******/
|
||||
|
||||
|
||||
+8
-2
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user