feat: S21.02 tune freertos-cross MAX_MESSAGE_SIZE to 512 + tunable-driven BDD gating#350
Conversation
…iven BDD gating Points the freertos-cross preset's SOLIDSYSLOG_USER_TUNABLES_FILE at a new Bdd/Targets/FreeRtos/solidsyslog_user_tunables.h that redefines SOLIDSYSLOG_MAX_MESSAGE_SIZE to 512 (pre-S12.12 default). Empirically reclaims 4.5KB of per-Log/Service stack frame on the Cortex-M3 BDD target. Linux/Windows stay at 2048 for RFC 5424 SHOULD compliance. Bakes a permanent [stack-hwm] high-water-mark print into the FreeRTOS BDD target's InteractiveTask on quit so every bdd-freertos-qemu run leaves an empirical stack-usage trail in CI for future E21 tuning. Replaces the static `not @requires_message_size_1500` exclusion in ci/docker-compose.bdd.yml with a runtime before_feature/before_scenario hook in Bdd/features/environment.py. The hook parses any @requires_<tunable>_N tag, compares N to the build's tunable value from the generated solidsyslog_tunables.py, and skips when the gate isn't met. Single source of truth for tunable-driven test selection. Closes #349
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR implements S21.02: it adds a FreeRTOS user-tunables header setting SOLIDSYSLOG_MAX_MESSAGE_SIZE=512, wires it into the freertos-cross CMake preset, enables and reports FreeRTOS task stack high-water marks in the example, and adds runtime Behave hooks that skip scenarios tagged ChangesFreeRTOS Message Size Tuning and Stack Monitoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1112 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Bdd/Targets/FreeRtos/main.c`:
- Around line 467-468: The ServiceTask creation result from xTaskCreate is not
being checked causing interactiveTaskCreated to be set to pdTRUE even if
ServiceTask fails; update the code around the xTaskCreate(ServiceTask, ...) call
to check its return value (pdPASS) before setting interactiveTaskCreated to
pdTRUE and before marking startup complete, log or handle the failure (e.g.,
clear/NULL serviceTaskHandle and emit an error) so subsequent code (including
the printf using uxTaskGetStackHighWaterMark(serviceTaskHandle)) doesn't report
misleading values; ensure serviceTaskHandle is only used when the task was
successfully created.
In `@Bdd/Targets/FreeRtos/solidsyslog_user_tunables.h`:
- Around line 11-14: Update the header comment in solidsyslog_user_tunables.h to
reflect that the test exclusion is now performed at runtime in
Bdd/features/environment.py (runtime skip logic), not via the
ci/docker-compose.bdd.yml tag filter; reword the comment to note the
`@requires_message_size_1500` feature is skipped at runtime by environment.py so
readers won't look for a compose-level exclusion and include a brief pointer to
the environment.py runtime skip behavior.
In `@DEVLOG.md`:
- Line 63: The DEVLOG line uses ambiguous notation `INTERACTIVE_TASK_STACK_DEPTH
= *48`; update the text to spell out the actual multiplier expression used in
code so it's unambiguous for readers—e.g., reference the exact expression like
`INTERACTIVE_TASK_STACK_DEPTH = configMINIMAL_STACK_SIZE * 48U` (or whatever
constant is used) and mention the related symbol `BddTargetInteractive` so the
log clearly matches the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3475e683-240c-49ba-bfbf-69c57d20aea7
📒 Files selected for processing (9)
Bdd/Targets/FreeRtos/FreeRTOSConfig.hBdd/Targets/FreeRtos/README.mdBdd/Targets/FreeRtos/main.cBdd/Targets/FreeRtos/solidsyslog_user_tunables.hBdd/features/environment.pyBdd/features/udp_mtu.featureCMakePresets.jsonDEVLOG.mdci/docker-compose.bdd.yml
- main.c [stack-hwm] print: read serviceTaskHandle into a local guarded against NULL so a Service xTaskCreate failure can't make uxTaskGetStackHighWaterMark(NULL) silently double-report the interactive task's HWM. - solidsyslog_user_tunables.h: header comment said udp_mtu.feature was excluded by the compose tag filter — corrected to point at the runtime gate in Bdd/features/environment.py. - DEVLOG.md: spell out INTERACTIVE_TASK_STACK_DEPTH = configMINIMAL_STACK_SIZE * 48U instead of the ambiguous "*48" shorthand. (Punted out of S21.02: CodeRabbit's broader fix that reorders the two xTaskCreate calls so an interactive-task failure also tears down the service task. Pre-existing failure-handling shape, not introduced here.)
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1112 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
First use of the S21.01 build-time tunables mechanism (#347). Points the
freertos-crosspreset at a FreeRTOS-specific user-config header that redefinesSOLIDSYSLOG_MAX_MESSAGE_SIZEfrom the library default 2048 down to 512 (pre-S12.12 default; comfortably above RFC 5424 §6.1's 480-byte minimum). Empirically reclaims 4.5KB of per-call stack frame on the Cortex-M3 BDD target.Linux and Windows presets keep 2048 so they still exercise path-MTU clipping (
udp_mtu.feature).Closes #349.
Change Description
Bdd/Targets/FreeRtos/solidsyslog_user_tunables.hwith#define SOLIDSYSLOG_MAX_MESSAGE_SIZE 512. Lives alongsideFreeRTOSConfig.h/FreeRTOSIPConfig.hso it reads as part of the FreeRTOS port surface, and serves as the worked example for the new "Tuning for your MCU" section inBdd/Targets/FreeRtos/README.md.CMakePresets.json—freertos-cross.cacheVariablesaddsSOLIDSYSLOG_USER_TUNABLES_FILEpointing at the absolute path of the new header. The S21.01 INTERFACE-target plumbing then propagates-DSOLIDSYSLOG_USER_TUNABLES_FILE="…"to all consumers.quit. The FreeRTOS BDD target'sInteractiveTasknow prints[stack-hwm] interactive=N words service=M wordsbefore destroy. Captured into everybdd-freertos-qemuCI run for regression detection on future E21 tuning. AddsINCLUDE_uxTaskGetStackHighWaterMark = 1toFreeRTOSConfig.hand captures the Service task handle out ofxTaskCreate.not @requires_message_size_1500exclusion inci/docker-compose.bdd.ymlwith a runtimebefore_feature/before_scenariohook inBdd/features/environment.py. The hook parses any@requires_<tunable>_Ntag, compares N against the build's value from the generatedsolidsyslog_tunables.py(the S21.01 Python mirror), and callsnode.skip(...)when the gate isn't met.udp_mtu.featurere-enables itself with no compose-file edit.@requires_block_size_N,@requires_buffer_capacity_N) plug in by adding one entry to_TUNABLE_TAG_GATESinenvironment.py.Test Evidence
send 1; quitunderqemu-system-arm):cmake --preset freertos-crossincpputest-freertos-cross:sha-10a14aeconfigures clean; generatedsolidsyslog_tunables.pymirrors 512.SolidSyslogBddTarget.elfcross-builds clean; QEMU smoke run completesSent 1 message+[stack-hwm]print.ghcr.io/davidcozens/behave:sha-3faff14:udp_mtu.feature1 feature skipped, 2 scenarios skipped, 7 steps skipped, no oracle callsAreas Affected
Bdd/Targets/FreeRtos/): new tunables header,INCLUDE_uxTaskGetStackHighWaterMarkinFreeRTOSConfig.h, stack-HWM print + task-handle capture inmain.c, README "Tuning for your MCU" section.Bdd/features/):environment.pygains the parametric@requires_<tunable>_Nskip hook;udp_mtu.featuregains@requires_message_size_1500.ci/docker-compose.bdd.yml): drops the no-longer-needed staticnot @requires_message_size_1500clause frombehave-freertos.freertos-crosspreset only — Linux/Windows/clang/sanitize/coverage/tidy/cppcheck/tunable-override-debug presets untouched.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests