feat: send the first record — passthrough buffer over UDP - #31
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe application now initializes a lwIP UDP syslog sender after TCP/IP setup, emits a BOOT record, gates readiness on successful emission, and updates documentation and run measurements for the first-record UDP flow. ChangesUDP syslog first-record flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HarnessTask
participant Syslog_Start
participant LogTask
participant UDPCollector
HarnessTask->>Syslog_Start: start after TCP/IP initialization
Syslog_Start->>UDPCollector: configure UDP destination
HarnessTask->>LogTask: emit one BOOT record
LogTask->>UDPCollector: send BOOT syslog record
LogTask-->>HarnessTask: return emission result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@app/syslog/Syslog.c`:
- Around line 1-9: Update the module comment near the Syslog implementation to
state that the emitted RFC 5424 message uses MSGID=BOOT and only the other
listed unset fields use NILVALUE; also update README.md lines 15-17 to replace
the claim that every header field is “-” with the precise NILVALUE fields. No
direct code behavior change is needed.
In `@app/tasks/LogTask.c`:
- Around line 41-44: The send result from SolidSyslog_Log must be propagated
before signaling completion: update app/tasks/LogTask.c lines 41-44 so
s_emitDone communicates success or failure instead of being given
unconditionally, and make LogTask_EmitOnce() return that outcome. In app/main.c
lines 69-76, preserve the existing ready/logged flow while ensuring a failed log
send cannot set ready or produce a successful exit.
🪄 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: 7ad0454f-8cb9-4630-a619-574e9eb677bf
⛔ Files ignored due to path filters (2)
measurements/stages.tsvis excluded by!**/*.tsvmeasurements/udp.csvis excluded by!**/*.csv
📒 Files selected for processing (6)
README.mdapp/main.capp/syslog/Syslog.capp/tasks/LogTask.capp/tasks/LogTask.hrun-report.md
The smallest wiring that delivers anything: a UDP sender over lwIP's raw API, with a passthrough buffer in front of it. Passthrough means Log sends inline on the calling task — no queue, no background drain, nothing to service — so this is the cheapest thing that can be called working. Flash +4,716 B RAM +372 B Log stack +16 B What the collector received: <134>1 - - - - BOOT - device started Timestamp, hostname, app-name and procid are all NILVALUE. RFC 5424 defines one for each, so the record is valid and syslog-ng parses it — filling them in is a later stage with a cost of its own, and separating the two is what lets the cost of each be seen. The three bad-config reports from the previous stage are gone, which is the other half of that stage's point. Two details worth knowing. Every lwIP raw call has to happen on the thread that owns the lwIP core; lwipopts.h sets LWIP_TCPIP_CORE_LOCKING, so taking the core lock in the caller's own task is simpler than posting to the tcpip mailbox and is unconditionally synchronous, which the marshal contract requires. And the collector address is a numeric literal, which keeps the resolver numeric-only — no DNS, so no LWIP_DNS and no resolver component compiled in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pull request
What this stage adds
The smallest wiring that delivers anything: a UDP sender over lwIP's raw API,
with a passthrough buffer in front of it. Passthrough means Log sends inline on
the calling task — no queue, no background drain, nothing to service — so this is
the cheapest thing that can be called working.
Flash +4,716 B
RAM +372 B
Log stack +16 B
What the collector received:
<134>1 - - - - BOOT - device started
Every header field is a NILVALUE. RFC 5424 defines one for all of them, so the
record is valid and syslog-ng parses it — filling them in is a later stage with a
cost of its own, and separating the two is what lets the cost of each be seen.
The three bad-config reports from the previous stage are gone, which is the other
half of that stage's point.
Two details worth knowing. Every lwIP raw call has to happen on the thread that
owns the lwIP core; lwipopts.h sets LWIP_TCPIP_CORE_LOCKING, so taking the core
lock in the caller's own task is simpler than posting to the tcpip mailbox and is
unconditionally synchronous, which the marshal contract requires. And the
collector address is a numeric literal, which keeps the resolver numeric-only —
no DNS, so no LWIP_DNS and no resolver component compiled in.
Files
Checklist
measurements/udp.csvcommitted.measurements/stages.tsv.python3 scripts/gen-cost-table.py../run.shgreen (build + QEMU + baseline self-check) —run-report.mdcommitted.Context for review: 4 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
BOOTrecord (“device started”) via the syslog path.Bug Fixes
Documentation