fix: ITM display never syncs on an SRM Conversion Kit

[kelchm]

Fixes #74

Summary

The ITM display never comes up on a wheel run through an SRM Conversion Kit — Device Status parks at Unavailable — retry in … and cycles the retry backoff forever. Reported on a Podium Bentley GT3 (kit firmware 6.10) on v0.6.0, but it affects any ITM-capable wheel behind a conversion kit.

Why it broke

The ITM display confirms every page change by sending a message back to the host; that reply is the protocol's only acknowledgment, and the lifecycle draws nothing until it arrives. Those replies are harvested in FanatecWheelbase.UpdateIdentity(), the once-per-frame device-status pass.

A conversion kit's identity is a one-shot — it is fixed for the life of the connection — so that pass returns early once the kit has been identified. The early return was placed above the ITM drain:

if (IsSrmConverter)
    return false;                                    // ← identity is settled, nothing to do
...
DrainFamily(Transport.ItmReports, _bufferItm);       // ← unreachable

That DrainFamily call is the only producer for the ITM buffer. The transport's reader thread keeps classifying and queueing the display's replies correctly, but nothing ever dequeues them, so DrainItmReports hands the driver an empty batch every frame and ItmLifecycleController never sees a push.

With no push, bring-up runs the entire recovery ladder — re-select the page twice, flip away and back, gate cycle — and then parks in Unavailable on the 5 s / 30 s / 300 s backoff. That is exactly the state the field report captured.

Regressed in v0.5.0 by #47, which restored conversion-kit wheel detection; the guard was added above what was then a single combined drain call. ITM support shipped in the same release, so this has never worked on a converted wheel.

The fix

Move the ITM drain above the converter short-circuit. Identity on a converter is a one-shot; the ITM channel is not.

Evidence from the report

• Outbound writes were fine. Reaching Unavailable requires the wheel to have accepted every command in the ladder (display reset, gate on, enable, page select) — a declined write parks the lifecycle in BringUp instead. The failure is entirely inbound.
• The identity source is the conversion kit's own channel, so IsSrmConverter is true and the early return is live.
• PSWBENT resolves correctly (display: itm, itmDeviceId: 4), so the driver was created and started as intended.

Scope

Nothing else is affected. Tuning drains its replies through its own path, and LED / 7-segment output is outbound-only. Genuine bases never take the early return, so their behaviour is unchanged.

Testing

• New regression test ItmReports_StillDrained_OnAnSrmConverter — verified failing on the current main and passing with this change.
• Full suite: 555 passed, 0 failed.
• Not yet verified on hardware. Draft until a converted wheel confirms it.

Open question

This fix is necessary but may not be sufficient. It guarantees FanaBridge reads whatever the kit sends; whether the kit relays the display's replies upstream at all is unverified, and the diagnostic report gives no evidence either way. Distinguishable from the SimHub log — a successful bring-up logs ITM: push confirmed — page N: …. Continued silence after this change would point at the kit, not at FanaBridge.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3499768b-e984-466b-9c4f-4c140340d58a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kelchm]

Adversarial cross-model review pass (Grok 4.5 + GPT-5.6, neither wrote the fix). Both independently verified the sole-producer claim and returned diagnosis CORRECT / fix correct and complete for the diagnosed bug. Neither could construct a merge-blocking new defect from the reorder, the drain bounds, the connected guard, or the thread model.

Applied in ad1ca02:

• Changelog overstated the outcome. It asserted the display now works, which is unverified on hardware. Reworded to what the code does, with the open possibility stated plainly.
• Test frame was unrealistic. It drained a 4-byte marker that the real parser would discard, so a future framing or device-id regression could have left it green. It now carries a valid device-4 subscription entry and asserts the parsed result.
• Recorded the drain-site rationale in the comment (below).

One point the two reviewers split on, resolved against the code: GPT-5.6 flagged that draining ahead of the identity work widens the window in which a late push from a previous attachment outlives the wheel-change clear, and suggested duplicating the drain into the converter branch to keep genuine bases byte-identical. Grok argued the new order is safer because the old one drained ITM after the clear — that is incorrect, git show b32669c has the ITM drain at step 2 and the clear at step 6, so it was always before. GPT-5.6's read is the accurate one, but the race is pre-existing in both orderings and is widened only by a few microseconds of same-thread work. Kept the single drain site: two sites that can drift apart is the worse trade, and the file's own AbandonInFlight comment makes that argument already.

Still draft — unchanged from before, this needs hardware confirmation. See #74 for the other faults that produce an identical symptom and how the SimHub log separates them.