Skip to content

feat: size the message cap to this device's records - #34

Merged
DavidCozens merged 1 commit into
mainfrom
stage/message-cap
Jul 29, 2026
Merged

feat: size the message cap to this device's records#34
DavidCozens merged 1 commit into
mainfrom
stage/message-cap

Conversation

@DavidCozens

@DavidCozens DavidCozens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Pull request

What this stage adds

SOLIDSYSLOG_MAX_MESSAGE_SIZE drops from the library's default of 2048 to 256.
RFC 5424 section 6.1 says a receiver should accept 2048; over UDP, RFC 5426
section 3.2 guarantees only 480. This device's records are far shorter than
either, so 256 stays well inside every guarantee and anything longer is truncated
rather than dropped.

Flash +6,032 B (unchanged)
RAM +1,956 B (+1,520)
Log stack +680 B (+664)

The cost is all RAM, and not where it looks. Setting the cap made the log task
overflow its stack outright:

[device] FATAL: stack overflow in task log

The record is built on the stack of whichever task calls Log, and the log seam
was still at the FreeRTOS minimum it inherited from the baseline. Measured at 800
bytes used, so the seam grows to 2 KiB by the same two-fold rule everything else
here uses. That is 1,536 bytes of stack allocation against a net rise of 1,520:
the smaller cap and address pool give 16 back.

Worth knowing, and not yet explained: the previous stage formatted and sent the
same record on the same 512-byte stack and reported 136 bytes used, with no
overflow and the record arriving intact. Introducing the tunables file is what
moved the formatter onto the caller's stack. The failure was loud rather than
silent — configCHECK_FOR_STACK_OVERFLOW is 2, with a hook — which is the only
reason this is a paragraph and not a corruption bug.

Files

 CMakeLists.txt                     |  4 ++++
 README.md                          | 13 ++++++++-----
 app/AppConfig.h                    |  9 +++++----
 app/config/solid_syslog_tunables.h | 18 ++++++++++++++++++
 measurements/message-cap.csv       | 13 +++++++++++++
 measurements/stages.tsv            |  1 +
 run-report.md                      | 26 +++++++++++++-------------
 7 files changed, 62 insertions(+), 22 deletions(-)

Checklist

  • The diff touches build infra (1 file(s)) — see Files above; deliberate for this stage.
  • measurements/message-cap.csv committed.
  • Row added to measurements/stages.tsv.
  • README regenerated: python3 scripts/gen-cost-table.py.
  • ./run.sh green (build + QEMU + baseline self-check) — run-report.md committed.

Context for review: 7 of 21 sequential single-commit PRs replaying the integration on top of the Baseline root. Each lands green and reviewed before the next is built on it, because a change to an early commit would force every commit above it to be re-run and re-measured.

Summary by CodeRabbit

  • New Features

    • Added device-specific logging configuration with a 256-byte message limit and support for one address.
    • Added documentation for the new message-cap stage and updated resource usage figures.
  • Performance

    • Increased the logging task’s reserved stack capacity to improve runtime reliability.
  • Documentation

    • Updated run reports, measurements, and self-check results to reflect the message-cap configuration.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2332bb03-41aa-44ec-af51-239ba8c14cd9

📥 Commits

Reviewing files that changed from the base of the PR and between 719742e and b63fd21.

⛔ Files ignored due to path filters (2)
  • measurements/message-cap.csv is excluded by !**/*.csv
  • measurements/stages.tsv is excluded by !**/*.tsv
📒 Files selected for processing (5)
  • CMakeLists.txt
  • README.md
  • app/AppConfig.h
  • app/config/solid_syslog_tunables.h
  • run-report.md
📝 Walkthrough

Walkthrough

The PR adds device-specific SolidSyslog tunables for a 256-byte message cap and one address, wires them through CMake, increases the logging task stack allocation, and updates README and run-report values for the message-cap configuration.

Changes

Message Cap Configuration

Layer / File(s) Summary
SolidSyslog tunables and build wiring
app/config/solid_syslog_tunables.h, CMakeLists.txt
Defines the 256-byte message limit and single-address pool, then configures CMake to use the new tunables header.
Logging task stack sizing
app/AppConfig.h
Changes LOG_TASK_STACK_WORDS to four times configMINIMAL_STACK_SIZE and updates its comment.
Message-cap documentation and measurements
README.md, run-report.md
Documents truncation and revised costs, and updates the run label, memory metrics, ELF size values, timestamp, and message-cap self-check references.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: reducing the message cap for this device's records.
Description check ✅ Passed All required sections are present and the checklist is mostly filled, though one item is intentionally contradicted by the build/config changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stage/message-cap

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@README.md`:
- Around line 19-24: Update the RAM-cost prose in the README to match the
generated authoritative measurement of RAM +1,956 B, or explicitly clarify that
the existing 1,536-byte figure is only a subcomponent of the total. Keep the
generated cost-table block unchanged.
🪄 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: df3f93c7-8986-4ee3-afd0-601f06986daf

📥 Commits

Reviewing files that changed from the base of the PR and between 47835a5 and 719742e.

⛔ Files ignored due to path filters (2)
  • measurements/message-cap.csv is excluded by !**/*.csv
  • measurements/stages.tsv is excluded by !**/*.tsv
📒 Files selected for processing (5)
  • CMakeLists.txt
  • README.md
  • app/AppConfig.h
  • app/config/solid_syslog_tunables.h
  • run-report.md

Comment thread README.md Outdated
SOLIDSYSLOG_MAX_MESSAGE_SIZE drops from the library's default of 2048 to 256.
RFC 5424 section 6.1 says a receiver should accept 2048; over UDP, RFC 5426
section 3.2 guarantees only 480. This device's records are far shorter than
either, so 256 stays well inside every guarantee and anything longer is truncated
rather than dropped.

  Flash    +6,032 B  (unchanged)
  RAM      +1,956 B  (+1,520)
  Log stack   +680 B  (+664)

The cost is all RAM, and not where it looks. Setting the cap made the log task
overflow its stack outright:

  [device] FATAL: stack overflow in task log

The record is built on the stack of whichever task calls Log, and the log seam
was still at the FreeRTOS minimum it inherited from the baseline. Measured at 800
bytes used, so the seam grows to 2 KiB by the same two-fold rule everything else
here uses. That is 1,536 bytes of stack allocation against a net rise of 1,520:
the smaller cap and address pool give 16 back.

Worth knowing, and not yet explained: the previous stage formatted and sent the
same record on the same 512-byte stack and reported 136 bytes used, with no
overflow and the record arriving intact. Introducing the tunables file is what
moved the formatter onto the caller's stack. The failure was loud rather than
silent — configCHECK_FOR_STACK_OVERFLOW is 2, with a hook — which is the only
reason this is a paragraph and not a corruption bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@DavidCozens
DavidCozens merged commit c31b54c into main Jul 29, 2026
2 checks passed
@DavidCozens
DavidCozens deleted the stage/message-cap branch July 29, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant