feat: S08.03 slice 3b.2 SingleTask example with SolidSyslogUdpSender + interactive command channel#301
Conversation
…+ interactive command channel Replaces 3b.1's link-up "ping" smoke with a real SolidSyslog wiring: NullBuffer + SolidSyslogUdpSender on the slice-1 SolidSyslogFreeRtosDatagram and the slice-3a SolidSyslogFreeRtosStaticResolver, exposed via Example/Common/ExampleInteractive over qemu -serial stdio. CmsdkUart grew a host-TDD'd RX path (CmsdkUart_GetChar; Init now writes TX_EN | RX_EN); Syscalls.c::_read calls into it with CR->LF translation and echo so fgets behaves as a cooked-mode TTY. Toolchain file gained -mcpu=cortex-m3 -mthumb in CMAKE_C_FLAGS_INIT so libSolidSyslog.a comes back as Thumb-2 and links into the cross binary; without this the first cross-library call hard-faulted on entry. Closes #296 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request implements S08.03 slice 3b.2, evolving the FreeRTOS SingleTask example from a one-shot UDP smoke test to an interactive SolidSyslog-driven application. It adds UART receive support, integrates newlib _read for stdin, updates cross-compilation flags, and wires SolidSyslog formatters/senders to emit RFC 5424 datagrams driven by interactive UART commands. ChangesInteractive SingleTask SolidSyslog Example with UART I/O
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 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 (✔️ 1096 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Example/FreeRtos/SingleTask/main.c (1)
116-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRound sub-tick UART sleeps up to one tick.
Line 118 currently converts the driver's
sleep(1)contract intovTaskDelay(0)wheneverpdMS_TO_TICKS(1)truncates to zero on the configured 100 Hz tick rate. SincevTaskDelay(0)returns without yielding the CPU, the RX/TX wait loops busy-spin instead of blocking the task.💡 Suggested fix
static void RtosSleep(int milliseconds) { - vTaskDelay(pdMS_TO_TICKS((TickType_t) milliseconds)); + TickType_t ticks = pdMS_TO_TICKS((TickType_t) milliseconds); + if ((milliseconds > 0) && (ticks == 0U)) + { + ticks = 1U; + } + vTaskDelay(ticks); }🤖 Prompt for 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. In `@Example/FreeRtos/SingleTask/main.c` around lines 116 - 119, RtosSleep currently calls vTaskDelay(pdMS_TO_TICKS((TickType_t) milliseconds)) which truncates small non-zero millisecond values to 0 on low tick rates and causes vTaskDelay(0) to return immediately; change RtosSleep to compute TickType_t ticks = pdMS_TO_TICKS((TickType_t) milliseconds) and if milliseconds > 0 && ticks == 0 set ticks = 1 before calling vTaskDelay(ticks) so that any non-zero millisecond sleep yields at least one tick and prevents busy-spin in RX/TX wait loops.
🧹 Nitpick comments (1)
Example/FreeRtos/SingleTask/main.c (1)
145-156: ⚡ Quick winPromote the placeholder timestamp to a named
TEST_*default.The other walking-skeleton defaults are all surfaced as named
TEST_*fixtures, but this callback still embeds the RFC 5424 placeholder date as raw literals.As per coding guidelines "For walking skeleton stories, use hard-coded 'test default' values named `TEST_*` (e.g. `TestHost`, `42`, RFC 5424 date `2009-03-23T00:00:00.000Z`)."♻️ Suggested refactor
+static const struct SolidSyslogTimestamp TEST_TIMESTAMP = { + .year = 2009U, + .month = 3U, + .day = 23U, + .hour = 0U, + .minute = 0U, + .second = 0U, + .microsecond = 0U, + .utcOffsetMinutes = 0, +}; + /* RFC 5424 publication date — walking-skeleton placeholder until S08.03 * slice 4+ injects a real RTC-backed clock callback. */ static void GetTimestamp(struct SolidSyslogTimestamp* timestamp) { - timestamp->year = 2009U; - timestamp->month = 3U; - timestamp->day = 23U; - timestamp->hour = 0U; - timestamp->minute = 0U; - timestamp->second = 0U; - timestamp->microsecond = 0U; - timestamp->utcOffsetMinutes = 0; + *timestamp = TEST_TIMESTAMP; }🤖 Prompt for 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. In `@Example/FreeRtos/SingleTask/main.c` around lines 145 - 156, Replace the hard-coded RFC5424 timestamp literals inside GetTimestamp with named TEST_* defaults: declare constants (e.g. TEST_TIMESTAMP_YEAR, TEST_TIMESTAMP_MONTH, TEST_TIMESTAMP_DAY, TEST_TIMESTAMP_HOUR, TEST_TIMESTAMP_MINUTE, TEST_TIMESTAMP_SECOND, TEST_TIMESTAMP_MICROSECOND, TEST_TIMESTAMP_UTCOFFSET_MINUTES) or a single TEST_TIMESTAMP fixture and assign those constants in GetTimestamp instead of raw numbers so the walking-skeleton timestamp is surfaced as a named test default.
🤖 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 `@Tests/FreeRtos/CmsdkUartFake.c`:
- Around line 173-183: CmsdkUartFake_SetReceivedByte currently leaves prior RX
readiness bits/countdown intact which can leak state between re-arms; modify the
function to clear prior RX readiness by clearing RX_FULL_BIT from fake.state and
resetting fake.readsRemainingBeforeRxReady to 0 before applying the new byte and
the readsBeforeRxReadyDefault logic (refer to CmsdkUartFake_SetReceivedByte,
fake.state, RX_FULL_BIT, and fake.readsRemainingBeforeRxReady).
---
Outside diff comments:
In `@Example/FreeRtos/SingleTask/main.c`:
- Around line 116-119: RtosSleep currently calls
vTaskDelay(pdMS_TO_TICKS((TickType_t) milliseconds)) which truncates small
non-zero millisecond values to 0 on low tick rates and causes vTaskDelay(0) to
return immediately; change RtosSleep to compute TickType_t ticks =
pdMS_TO_TICKS((TickType_t) milliseconds) and if milliseconds > 0 && ticks == 0
set ticks = 1 before calling vTaskDelay(ticks) so that any non-zero millisecond
sleep yields at least one tick and prevents busy-spin in RX/TX wait loops.
---
Nitpick comments:
In `@Example/FreeRtos/SingleTask/main.c`:
- Around line 145-156: Replace the hard-coded RFC5424 timestamp literals inside
GetTimestamp with named TEST_* defaults: declare constants (e.g.
TEST_TIMESTAMP_YEAR, TEST_TIMESTAMP_MONTH, TEST_TIMESTAMP_DAY,
TEST_TIMESTAMP_HOUR, TEST_TIMESTAMP_MINUTE, TEST_TIMESTAMP_SECOND,
TEST_TIMESTAMP_MICROSECOND, TEST_TIMESTAMP_UTCOFFSET_MINUTES) or a single
TEST_TIMESTAMP fixture and assign those constants in GetTimestamp instead of raw
numbers so the walking-skeleton timestamp is surfaced as a named test default.
🪄 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: 82873159-4581-4bd6-9f96-0bfb7d3033b6
📒 Files selected for processing (12)
Core/Source/CMakeLists.txtDEVLOG.mdExample/FreeRtos/Common/CmsdkUart.cExample/FreeRtos/Common/CmsdkUart.hExample/FreeRtos/Common/Syscalls.cExample/FreeRtos/SingleTask/CMakeLists.txtExample/FreeRtos/SingleTask/main.cExample/FreeRtos/cmake/arm-none-eabi.cmakePlatform/FreeRtos/Source/SolidSyslogAddress.cTests/FreeRtos/CmsdkUartFake.cTests/FreeRtos/CmsdkUartFake.hTests/FreeRtos/CmsdkUartTest.cpp
Three findings, all valid: 1. CmsdkUartFake.c::SetReceivedByte didn't clear leftover RX_FULL_BIT or readsRemainingBeforeRxReady before applying the new mode, so back-to-back arms could carry stale state into the new arm. Added a regression test (SetReadsBeforeRxReady(0)+SetReceivedByte then SetReadsBeforeRxReady(2)+SetReceivedByte; the second GetChar must spin) which fails on the unfixed fake and passes once SetReceivedByte resets RX_FULL_BIT and the countdown up front. 2. main.c::RtosSleep called vTaskDelay(pdMS_TO_TICKS(milliseconds)) which truncates 1 ms to 0 ticks at configTICK_RATE_HZ=100. vTaskDelay(0) returns without yielding, so CmsdkUart's spin-with-sleep(1) loops would busy-spin instead of blocking the task. Latent on QEMU (TX spin never iterates; RX spin gets preempted by SysTick→IP task) but a real correctness bug on silicon. Floor non-zero requests at one tick. 3. main.c hardcoded the RFC 5424 publication-date timestamp inline, inconsistent with the other walking-skeleton TEST_* defaults (TEST_HOSTNAME, TEST_APP_NAME, etc.). Promoted to TEST_TIMESTAMP fixture; GetTimestamp now does *timestamp = TEST_TIMESTAMP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CodeRabbit review feedback addressed in 6c12ab6. The inline thread on 1. 2. Pre-PR re-checks after the fixup commit:
|
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1096 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Summary
SolidSyslogConfig+SolidSyslogUdpSenderbehind the slice-1SolidSyslogFreeRtosDatagramand slice-3aSolidSyslogFreeRtosStaticResolver, withExample/Common/ExampleInteractiverunning overqemu -serial stdio(CmsdkUart RX path host-TDD'd in this slice; Syscalls.c::_read does the CR→LF + echo).send N/quitover the UART produce N RFC 5424 datagrams to {10.0.2.2, 5514} and a clean teardown.Example/FreeRtos/Common/CmsdkUart.{h,c}rather than introducing a siblingUartRx.{c,h}— one CMSDK UART driver inCommon/consumed by both HelloWorld and SingleTask.CmsdkUart_Initnow writesCTRL ← TX_EN | RX_EN;CmsdkUart_GetCharblocks onSTATE.RXFULLwith the samesleep(1)yield idiom asPutChar. Five ZOMBIES cycles drove this against an extendedCmsdkUartFakethat models RXFULL gating and DATA-read-clears-RXFULL.Example/FreeRtos/cmake/arm-none-eabi.cmakenow sets-mcpu=cortex-m3 -mthumbinCMAKE_C_FLAGS_INITsolibSolidSyslog.ais Thumb-2 (without this, the first cross-library call hard-faulted becausearm-none-eabi-gccdefaulted to ARM mode); (b) the InteractiveTask runs atconfigMINIMAL_STACK_SIZE * 32(16 KB) — overkill but safe, with a follow-up to characterise the real budget once CMake-driven memory scaling lands.Detailed rationale in
DEVLOG.md(2026-05-09 entry).Closes #296
Test plan
clang-format --dry-run --Werroron every changed fileTests/FreeRtos/CmsdkUartTest— 14 / 14 (5 new RX cycles)Tests/FreeRtos/SolidSyslogFreeRtosDatagramTest— 21 / 21 (unchanged)Tests/FreeRtos/SolidSyslogFreeRtosStaticResolverTest— 10 / 10 (unchanged)ctest --preset debug— greencmake --build --preset freertos-cross— both ELFs link cleansend 1produces 1 datagram,send 3produces 3,quitexits🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores