Skip to content

srxl2: a frame received in this cycle is not 49 days old - #12002

Open
MrScothh wants to merge 1 commit into
iNavFlight:maintenance-10.xfrom
MrScothh:fix/srxl2-telemetry-timestamp
Open

MrScothh wants to merge 1 commit into
iNavFlight:maintenance-10.xfrom
MrScothh:fix/srxl2-telemetry-timestamp

Conversation

@MrScothh

Copy link
Copy Markdown
Contributor

What this fixes

srxl2ProcessEsc() takes the time once, before draining the port, and then compares it
against the timestamps that draining the port has just written. When the millisecond turns
over in between, the unsigned difference wraps: the reading that has only now arrived reads
as 49 days old, the telemetry is marked invalid, and it stays invalid until the next frame.

Measured on an Avian, which answers every telemetry request but rotates between sensors, so
its ESC readings arrive about twice a second:

Blackouts in 180 s Longest
Before 19 1.0 s
After 0 -

While the telemetry is invalid the OSD, Blackbox, current estimation and the RPM filter have
no ESC data, and the link itself never drops, so nothing else shows that anything is wrong.

The same wrapped comparison sits on the link timeout one line below. On a bus that agreed to
a higher rate it drops back to 115200, forgets the device and goes looking for the ESC again,
which is a heavier consequence of the same cause.

What I changed

The time is taken again once the port has been drained, so every comparison in the cycle is
made against a clock reading no older than the frames it is compared with.

Testing

On the bench, an Avian on a TBS Lucid H7 Wing, sampled 47 times a second for 180 s: no
telemetry blackouts and no link drops, against 19 blackouts in the same run before the
change. The instrumentation also showed what was happening inside a blackout: requests kept
going out and replies kept arriving, only the ESC readings among them were being thrown away.

srxl2ProcessEsc() takes the time once, before draining the port, and then
compares it against the timestamps that draining the port has just written.
When the millisecond turns over in between, the unsigned difference wraps: the
reading that has only now arrived reads as 49 days old, the telemetry is
marked invalid, and it stays invalid until the next frame.

On the bench, with an Avian answering every telemetry request but rotating
between sensors, so that the ESC readings themselves arrive about twice a
second, that cost 19 blackouts in 180 seconds, each lasting from 100 ms to a
second. The OSD, Blackbox, current estimation and the RPM filter all lose the
ESC data for that long. After taking the time again once the port is drained,
the same run had none, with the link up throughout.

The same wrap also sits on the link timeout, one line below, which on a bus
that agreed to a higher rate would drop back to 115200 and go looking for the
ESC again.
@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Refresh SRXL2 timestamps after draining received frames

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Refreshes SRXL2 processing time after draining newly received serial frames.
• Prevents timestamp wrap from invalidating telemetry or triggering false link timeouts.
Diagram

sequenceDiagram
    participant Loop as Motor Loop
    participant Clock as Millisecond Clock
    participant Proc as ESC Processor
    participant RX as Serial RX
    participant Frame as Frame Handler
    participant State as ESC State
    Loop->>Clock: Read cycle time
    Clock-->>Loop: Initial now
    Loop->>Proc: Process ESC
    Proc->>RX: Drain frames
    RX->>Frame: Parsed frame
    Frame->>Clock: Read receive time
    Clock-->>Frame: Receive timestamp
    Frame->>State: Stamp telemetry and link
    Proc->>Clock: Refresh after drain
    Clock-->>Proc: Fresh now
    Proc->>State: Check age and timeout
Loading
High-Level Assessment

Refreshing now immediately after receive draining is the smallest and safest approach because all subsequent comparisons use a clock value at least as recent as frame timestamps. Passing timestamps through the receive stack or changing subtraction semantics would add coupling or obscure the underlying ordering issue without improving correctness.

Files changed (1) +8 / -0

Bug fix (1) +8 / -0
motor_srxl2.cRefresh timeout clock after draining SRXL2 frames +8/-0

Refresh timeout clock after draining SRXL2 frames

• Re-reads the millisecond clock after receive processing, ensuring newly stamped telemetry and link activity are never compared against an older cycle timestamp. Adds context explaining the unsigned wraparound and its observed telemetry impact.

src/main/io/motor_srxl2.c

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