feat: S08.03 slice 9 timeQuality SD + no-RTC FreeRTOS example#313
Conversation
Wire timeQuality SD into the FreeRTOS SingleTask example and switch the example to a no-RTC product stance per RFC 5424 §6.2.3.1. Bundles a syslog-ng pin (4.11 latest aborts on STATS over the control socket and takes the dev-container network down). Closes #312. 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 (2)
📝 WalkthroughWalkthroughThis PR implements the no-RTC product stance for FreeRTOS ( ChangesFreeRTOS No-RTC Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Bdd/features/time_quality.feature (1)
16-21: ⚡ Quick winMake coexist scenarios assert full timeQuality pair (
tzKnown+isSynced).On Line 16–21 and Line 30–35, the coexist scenarios currently validate
sequenceId+tzKnownonly. AddingisSyncedhere improves parity with the dedicated time_quality scenarios and avoids partial regressions.As per coding guidelines, “`Bdd/**/*.{feature,py,cpp,c}`: BDD test infrastructure with Gherkin features, step definitions, and syslog-ng config; out of scope for Tier 1 support”.Proposed BDD assertion additions
Scenario: Time quality and sequence ID coexist @@ Then the structured data contains sequenceId "1" And the structured data contains tzKnown "1" + And the structured data contains isSynced "1" @@ Scenario: Time quality and sequence ID coexist (no RTC) @@ Then the structured data contains sequenceId "1" And the structured data contains tzKnown "0" + And the structured data contains isSynced "0"Also applies to: 30-35
🤖 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 `@Bdd/features/time_quality.feature` around lines 16 - 21, Update the two "Time quality and sequence ID coexist" scenarios to assert the full timeQuality pair: add an assertion that the structured data contains isSynced "1" in addition to the existing tzKnown "1" and sequenceId "1" checks; specifically modify the scenario titled "Time quality and sequence ID coexist" (and the second identical coexist scenario) to include a step verifying isSynced "1" so the tests validate both tzKnown and isSynced together.
🤖 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.
Nitpick comments:
In `@Bdd/features/time_quality.feature`:
- Around line 16-21: Update the two "Time quality and sequence ID coexist"
scenarios to assert the full timeQuality pair: add an assertion that the
structured data contains isSynced "1" in addition to the existing tzKnown "1"
and sequenceId "1" checks; specifically modify the scenario titled "Time quality
and sequence ID coexist" (and the second identical coexist scenario) to include
a step verifying isSynced "1" so the tests validate both tzKnown and isSynced
together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0dee0768-47ad-4f13-b9c4-d2bbb5868391
📒 Files selected for processing (9)
.devcontainer/docker-compose.yml.github/workflows/ci.ymlBdd/README.mdBdd/features/origin.featureBdd/features/time_quality.featureBdd/features/timestamp.featureExample/FreeRtos/SingleTask/main.cci/docker-compose.bdd.ymldocs/containers.md
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
…arios - timestamp.feature: U+2192 (right arrow) in the feature description trips Windows behave's cp1252 stdout codec with UnicodeEncodeError, failing bdd-windows-otel before any scenario runs. § (U+00A7) survives because it's in cp1252; → is not. Use ASCII '->' instead. - time_quality.feature: extend the coexist scenarios (both @rtc and the new @no_rtc) to assert isSynced alongside tzKnown for parity with the dedicated time-quality scenarios. CodeRabbit nitpick on #313. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
Closes #312. Last slice of S08.03 (#268) — wires
timeQualitySD into theFreeRTOS SingleTask example and switches the example to a no-RTC product
stance per RFC 5424 §6.2.3.1. Bundled with a syslog-ng pin to keep the
dev/CI oracles stable.
Change Description
No-RTC reference example (
Example/FreeRtos/SingleTask/main.c)TEST_TIMESTAMPplaceholder andGetTimestampcallback. Setconfig.clock = NULL; the library's NilClock then yields an all-zerotimestamp that fails
TimestampIsValidand emits NILVALUE on the wire.GetTimeQualityreturningtzKnown=false, isSynced=false, syncAccuracyMicroseconds=SOLIDSYSLOG_SYNC_ACCURACY_OMIT; wireSolidSyslogTimeQualitySdintosdList[]betweenmetaSdandoriginSd.BDD tag taxonomy —
@rtc/@no_rtcreplace@freertoswipontime-related scenarios.
time_quality.featureandorigin.featuregain
@no_rtcsiblings that assert the no-RTC field values(
tzKnown="0",isSynced="0").timestamp.featurebecomesfeature-level
@rtc.Deviation from #312 scope — the issue listed a new
"Timestamp is NILVALUE when no RTC is present" scenario; dropped because
syslog-ng silently substitutes receipt time for both
${ISODATE}and${S_ISODATE}when the wire TIMESTAMP is NILVALUE, so a BDD assertionagainst the oracle can't actually distinguish "wire empty" from
"wire valid".
Tests/SolidSyslogTest.cpp::NullClockProducesNilvalueplus ten sibling boundary tests already cover NILVALUE emission
end-to-end through the formatter.
syslog-ng pin to 4.8.2 (
.devcontainer/docker-compose.yml,ci/docker-compose.bdd.yml,docs/containers.md) —balabit/syslog-ng:latestresolves to 4.11.0, which has a regression instats-control.cthat aborts the daemon (signal 6 / exit 134) whenanything sends
STATS\nover the control socket. Reproduced standalone.When
syslog-ng-freertosaborts,freertos-targetloses its networknamespace (
network_mode: service:syslog-ng-freertos) and VS Code canno longer reach the dev-container API — the only recovery is restarting
Docker. 4.8.2 (4.8 LTS, ships syslog-ng 4.9.0) handles
STATS\ncorrectly and keeps the same behaviour our existing config relies on.
Test Evidence
cmake --build --preset freertos-cross --target SolidSyslogFreeRtosSingleTask— clean.behave --tags='not @wip and not @freertoswip and not @rtc and @udp' Bdd/features/inside
freertos-target: 7 features / 20 scenarios pass, 0 fail(4 newly-untagged
@no_rtcscenarios run against the QEMU FreeRTOS target).behave --tags='@udp and @rtc'for the Linux pair: timestamp,origin, time_quality, header_fields features all green.
cmake --build --preset debug --target junit— 1084/1088 tests pass(no library change, no deltas).
clang-format --dry-run --WerroronCore/Interface Core/Source Tests Example— clean.client; verified 4.8.2 returns CSV stats with the daemon staying healthy.
Areas Affected
Example/FreeRtos/SingleTask/main.c(Tier 3 — example).Bdd/features/{time_quality,origin,timestamp}.featureandBdd/README.md— tag taxonomy + new no-RTC scenarios.ci/docker-compose.bdd.yml(Linux + FreeRTOS),.github/workflows/ci.yml(Windows OTel)..devcontainer/docker-compose.yml,ci/docker-compose.bdd.yml,docs/containers.md.No production library code changed.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Chores