From 3dd8e3c8ce667aba7d27c4c667d1b8782bf59b95 Mon Sep 17 00:00:00 2001 From: Jeremy Poulter Date: Wed, 2 Sep 2026 21:37:07 +0100 Subject: [PATCH] fix(rapi): size $GI buffers from MCU_ID_LEN, not the target name ESRAPI_BUFLEN and TMP_BUF_SIZE were selected with #ifdef TARGET_SAMD while the #error guards that check them assert on MCU_ID_LEN. Any new target with a 16-byte MCU id therefore failed to build, even though the requirement it violated was expressed purely in terms of the id length. Key the selection on MCU_ID_LEN so a target inherits the right buffer from the property that actually drives it. Both existing targets are unchanged: a 16-byte id still selects 40/48, a 10-byte id still selects 32/34, so AVR RAM cost stays zero (ESRAPI_BUFLEN sizes three buffers there). Verified with temporary compile-time assertions pinning the expected values per target, including a deliberate wrong value to confirm the assertions could fail. Clean builds of m328p_core, m328p_LCD_WIFI and samd are byte-identical to baseline: m328p_core text 22752 data 256 bss 684 m328p_LCD_WIFI text 26834 data 348 bss 698 samd text 46716 data 560 bss 4140 Both #error guards are retained. Include order was checked: target.h lands at open_evse.h:39 and rapi_proc.h is only included from open_evse.h:1442, so MCU_ID_LEN is defined before either block is evaluated. Co-Authored-By: Claude Opus 5 --- firmware/open_evse/open_evse.h | 8 +++++++- firmware/open_evse/rapi_proc.h | 16 +++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/firmware/open_evse/open_evse.h b/firmware/open_evse/open_evse.h index 53dbceaa..c591d893 100644 --- a/firmware/open_evse/open_evse.h +++ b/firmware/open_evse/open_evse.h @@ -527,7 +527,13 @@ extern AutoCurrentCapacityController g_ACCController; // 10-byte MCU id needs only 33 for $GI, so the LCD-derived 34 still fits; // keep AVR unchanged so its RAM footprint does not grow. See the matching // #error guard in rapi_proc.cpp. -#ifdef TARGET_SAMD +// +// Select from the MCU id length rather than the target name: take the +// LCD-derived size unless the $GI reply needs more. Values are unchanged +// from when this was keyed on TARGET_SAMD -- a 16-byte id needs 45 and so +// selects 48, a 10-byte id needs 33 and so keeps the LCD-derived 34. +#define TMP_BUF_GI_REPLYLEN (3 + 1 + (2*MCU_ID_LEN) + 4 + 4 + 1) +#if defined(MCU_ID_LEN) && (TMP_BUF_GI_REPLYLEN > ((LCD_MAX_CHARS_PER_LINE+1)*2)) #define TMP_BUF_SIZE 48 #else #define TMP_BUF_SIZE ((LCD_MAX_CHARS_PER_LINE+1)*2) diff --git a/firmware/open_evse/rapi_proc.h b/firmware/open_evse/rapi_proc.h index 1bf02146..c6e4535c 100644 --- a/firmware/open_evse/rapi_proc.h +++ b/firmware/open_evse/rapi_proc.h @@ -487,11 +487,17 @@ Z0 closems holdpwm // buffer[] holds an inbound RAPI command and is reused to build the // outbound response text. The longest response text is $GI (get MCU id), -// whose #else branch writes 2*MCU_ID_LEN hex chars plus a NUL. On SAMD -// MCU_ID_LEN is 16, so that is 2*16+1 = 33 bytes and the historic 32-byte -// buffer overflowed by one, corrupting the adjacent bufCnt member. Size -// per target so AVR RAM cost stays zero. -#ifdef TARGET_SAMD +// whose #else branch writes 2*MCU_ID_LEN hex chars plus a NUL. With a +// 16-byte MCU id that is 2*16+1 = 33 bytes and the historic 32-byte buffer +// overflowed by one, corrupting the adjacent bufCnt member. +// +// Size from the MCU id length rather than the target name, so any target +// inherits the right buffer automatically. The values are unchanged from +// when this was keyed on TARGET_SAMD: a 16-byte id selects 40, a 10-byte +// id selects 32, so AVR RAM cost stays zero (ESRAPI_BUFLEN sizes three +// buffers there -- buffer[], g_rapiSerialBuffer and g_rapiI2ClBuffer). +#define ESRAPI_GI_RESPLEN (2*MCU_ID_LEN + 1) +#if defined(MCU_ID_LEN) && (ESRAPI_GI_RESPLEN > 32) #define ESRAPI_BUFLEN 40 #else #define ESRAPI_BUFLEN 32