fix(rapi): size $GI buffers from MCU_ID_LEN, not the target name - #56
Draft
jeremypoulter wants to merge 1 commit into
Draft
fix(rapi): size $GI buffers from MCU_ID_LEN, not the target name#56jeremypoulter wants to merge 1 commit into
jeremypoulter wants to merge 1 commit into
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ESRAPI_BUFLEN and TMP_BUF_SIZE were selected with
#ifdef TARGET_SAMD, while the#errorguards that check them assert onMCU_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.This keys the selection on
MCU_ID_LENinstead, 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 and a 10-byte id still selects 32/34, so AVR RAM cost stays zero —
ESRAPI_BUFLENsizes three buffers there (buffer[],g_rapiSerialBuffer,g_rapiI2ClBuffer).Verification
Temporary compile-time assertions pinned the expected values per target, including a deliberately wrong value to confirm the assertions could fail. Clean builds are byte-identical to baseline:
Both
#errorguards are retained. Include order was checked:target.hlands atopen_evse.h:39andrapi_proc.his only included fromopen_evse.h:1442, soMCU_ID_LENis defined before either block is evaluated. That mattered — if it were not, samd would silently drop to the 32-byte buffer and the guard would be skipped too.Worth having on its own merits: it is a latent trap for whatever target comes next.
🤖 Generated with Claude Code