Skip to content

Fix Peak ISO-TP instability during large UDS transfers (#425) - #426

Merged
frankie-zeng merged 2 commits into
masterfrom
cursor/fix-peak-docan-iso-tp-425-43af
Aug 24, 2026
Merged

Fix Peak ISO-TP instability during large UDS transfers (#425)#426
frankie-zeng merged 2 commits into
masterfrom
cursor/fix-peak-docan-iso-tp-425-43af

Conversation

@frankie-zeng

Copy link
Copy Markdown
Collaborator

Fixes #425

Problem

DoCan / ISO-TP communication with PEAK (PCAN) was unstable during large firmware updates (many UDS 0x36 requests, ~2500 bytes each). Symptoms included:

  • TP_TIMEOUT_UPPER_READ ("upper layer read timeout") at random points
  • PCANTP_STATUS_QUEUE_TX_FULL from CANTP_Write_2016 causing the bus to be closed
  • Incomplete receive-queue draining and inconsistent message lifetime handling in _read()

Root causes found

  1. Transient TX queue full treated as fatal_writeBase and _writeTp called close() on PCANTP_STATUS_QUEUE_TX_FULL, tearing down the whole channel during bursty multi-frame TX.
  2. Receive queue not fully drained_read() returned after the first loopback/RX message via setImmediate, leaving later messages (including completed ISO-TP RX) queued until the next event.
  3. Multi-frame RX never delivered — when INDICATION_RX progress reached COMPLETED, the code freed the message but never emitted the read event, causing readTp() to time out.
  4. Message lifetime bugs — several early-return paths skipped CANTP_MsgDataFree_2016; the INDICATION_RX completed path could double-free.

Changes (Peak only, TypeScript-only)

  • Added bounded retry with short backoff for transient write statuses (QUEUE_TX_FULL, LOCK_TIMEOUT) in _writeBase and _writeTp; no bus teardown on these errors.
  • Rewrote _read() to drain the full PCAN queue per callback and always free each read message exactly once (try/finally).
  • Resolve ISO-TP TX writes only after multi-frame COMPLETED progress; emit ISO-TP RX reads only after multi-frame COMPLETED.
  • Extracted testable helpers in src/main/docan/peak/helpers.ts with unit tests in test/docan/peakHelpers.spec.ts.

Verification

  • npm run test -- test/docan/peakHelpers.spec.ts — pass
  • npm run typecheck:node — pass
  • No native module rebuild required

Requested re-test (@ptsiewie)

Please verify on a PCAN dongle with Windows 11:

  1. Run the built-in S19 firmware update script (or equivalent repeated UDS 0x36 traffic, ~2500-byte payloads).
  2. Confirm PCANTP_STATUS_QUEUE_TX_FULL no longer closes the bus.
  3. Confirm full firmware uploads complete without TP_TIMEOUT_UPPER_READ.
  4. Spot-check normal single-frame UDS request/response still works.

Thank you for offering to help validate on real hardware — that confirmation would be very valuable for closing #425.

Open in Web Open in Cursor 

Treat PCANTP_STATUS_QUEUE_TX_FULL and LOCK_TIMEOUT as transient write
errors with bounded retries instead of closing the bus.

Rewrite _read to drain the full receive queue per callback, always free
read messages once, and emit multi-frame RX only after COMPLETED progress.
Resolve multi-frame TX writes only after COMPLETED as well.

Add unit tests for the extracted Peak helper logic.
Reporter feedback on #426: after the queue-drain rewrite, multi-frame ISO-TP
TX failed with N_TIMEOUT_BS even though FC (30 00 00) was visible in the trace.

Root cause: draining the entire PCAN receive queue inside one
ThreadSafeFunction BlockingCall never yielded, which starved Peak's ISO-TP TX
path (self-receive / FC→CF continuation). Restore one-message-per-callback
processing with setImmediate continuation while still freeing each read
buffer exactly once.

Also align with PCAN-ISO-TP API User Manual:
- detect LOOPBACK with bitwise AND (LOOPBACK may be OR-ed with ISOTP_FRAME)
- call GetMsgProgress_2016 on the Read loopback/indication buffer
- emit RX only for non-INDICATION messages (complete payload)
- surface Peak netstatus (e.g. TIMEOUT_Bs) on TX confirmations
@frankie-zeng
frankie-zeng marked this pull request as ready for review August 24, 2026 12:42
@frankie-zeng
frankie-zeng merged commit 7d155a1 into master Aug 24, 2026
2 checks passed
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.

[BUG] DoCan communication not stable with Peak

2 participants