Fix Peak ISO-TP instability during large UDS transfers (#425) - #426
Merged
Conversation
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
marked this pull request as ready for review
August 24, 2026 12:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 pointsPCANTP_STATUS_QUEUE_TX_FULLfromCANTP_Write_2016causing the bus to be closed_read()Root causes found
_writeBaseand_writeTpcalledclose()onPCANTP_STATUS_QUEUE_TX_FULL, tearing down the whole channel during bursty multi-frame TX._read()returned after the first loopback/RX message viasetImmediate, leaving later messages (including completed ISO-TP RX) queued until the next event.INDICATION_RXprogress reachedCOMPLETED, the code freed the message but never emitted the read event, causingreadTp()to time out.CANTP_MsgDataFree_2016; the INDICATION_RX completed path could double-free.Changes (Peak only, TypeScript-only)
QUEUE_TX_FULL,LOCK_TIMEOUT) in_writeBaseand_writeTp; no bus teardown on these errors._read()to drain the full PCAN queue per callback and always free each read message exactly once (try/finally).COMPLETEDprogress; emit ISO-TP RX reads only after multi-frameCOMPLETED.src/main/docan/peak/helpers.tswith unit tests intest/docan/peakHelpers.spec.ts.Verification
npm run test -- test/docan/peakHelpers.spec.ts— passnpm run typecheck:node— passRequested re-test (@ptsiewie)
Please verify on a PCAN dongle with Windows 11:
PCANTP_STATUS_QUEUE_TX_FULLno longer closes the bus.TP_TIMEOUT_UPPER_READ.Thank you for offering to help validate on real hardware — that confirmation would be very valuable for closing #425.