Repository navigation
Sender: CAN-Fehler gedrosselt loggen, ESP-NOW-Backoff ohne Display (2.1.0-beta.2) - #27
Conversation
…bsent - CAN error passive / bus error / RX queue full: one summary per 5 s instead of one flash-log entry and radio packet per alert (~200/s on the bench). - ESP-NOW: delivery callback feeds a LinkBackoff; after 5 failed deliveries only one packet per second until the display acknowledges again. Skipped packets consume no sequence numbers. Send errors logged at most every 10 s. - Version V2.1.0-beta.2. Hardware test (kalipi, both boards): display on/off/on transitions detected, no error flood. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7076f85dc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (uint32_t i = seenUndelivered; i != undelivered; ++i) linkBackoff.reportResult(false); | ||
| if (delivered != seenDelivered && lastDeliveryOk.load()) linkBackoff.reportResult(true); |
There was a problem hiding this comment.
When multiple delivery callbacks arrive between calls to updateLinkState (for example, after the heartbeat queues a burst of packets), these counters discard their ordering and ignore every success unless the last callback succeeded. A batch of failure, success, failure, success, failure, success, failure, success, failure therefore triggers offline mode despite never having consecutive failures, unnecessarily reducing telemetry to one packet per second on a working but lossy link. Preserve the consecutive-failure streak across callbacks, or process ordered results, rather than replaying all failures together.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved findings remain active—a misleading header comment referencing a nonexistent summary() method and a newly added linkOffline() accessor with no callers—so the change still has open issues to address before approval.
2 open issues
What changed in this PR
This PR hardens the sender's logging and radio behavior based on first hardware-bench findings. Previously, CAN controller alerts (error passive, bus error, RX queue full) each produced a flash-log entry plus a radio packet on every poll — ~200/s on a bench without a vehicle. It also adds a real ESP-NOW delivery-based backoff so the sender throttles to ~1 packet/s when the display stops acknowledging, without wasting sequence numbers on skipped packets. Two small, header-only utilities (EventThrottle, LinkBackoff) encapsulate the logic and are unit-tested, and the firmware version is bumped to V2.1.0-beta.2.
Changes:
- Introduce
EventThrottle(rate-limit repeating events into periodic summaries) andLinkBackoff(offline detection + one-probe-per-interval), with a new native test suite. - Wire ESP-NOW send-callback delivery results into
SenderTelemetryvia atomics, gate all transmits behind the backoff (skipped packets don't consume sequence numbers), and throttle CAN alert logging/status to one summary per 5s. - Bump version and update README/HARDWARE docs.
| File | Description |
|---|---|
lib/logging/EventThrottle.h |
New rate limiter; one report per interval while counting all occurrences (header comment still references a nonexistent summary()). |
lib/transport/LinkBackoff.h |
New ESP-NOW backoff: offline after N consecutive failures, one probe per interval, resets on first success. |
src/sender/SenderTelemetry.cpp |
Integrates delivery callback via atomics, gates transmits, throttles send-failure logging; adds unused linkOffline(). |
src/sender/SenderTelemetry.h |
Declares onDelivery() and linkOffline() public API. |
src/sender/SenderEspNow.cpp |
Registers ESP-NOW send callback forwarding MAC-level ack to SenderTelemetry::onDelivery. |
src/sender/SenderCanAlerts.cpp |
Throttles error-passive/bus-error/queue-full alerts to one summary line per 5s via EventThrottle. |
test/test_throttle_backoff/test_throttle_backoff.cpp |
New unit tests for throttle and backoff behavior. |
VERSION.txt / README.md / docs/HARDWARE.md |
Version bump to V2.1.0-beta.2 and documentation of the new throttling/backoff behavior. |
Notes:
- The core throttle/backoff logic, atomic-based cross-task handoff, and transmit gating are correct and match the new tests; the ESP-NOW send-callback signature (
esp_now_send_info_t) matches the pinned espressif32 8.0.0 / ESP-IDF 5.x framework. - Two minor comments were stored: an outdated
summary()reference inEventThrottle.hdocs, and the unused newlinkOffline()accessor (optional).
Review effort: Balanced
| // Rate limiter for repeating events (CAN bus errors, radio send failures). | ||
| // | ||
| // record() counts every occurrence but returns true at most once per | ||
| // interval; summary() then tells how many events happened since the last |
| bool linkOffline() { | ||
| return linkBackoff.offline(); | ||
| } |
Zusammenfassung
Aus dem ersten Hardwaretest:
V2.1.0-beta.2.Tests
Native Tests: 174/174, neu
test_throttle_backoffHardware (kalipi, beide Boards):
🤖 Generated with Claude Code