Skip to content

fix(zboss): keep reader/writer stream plumbing intact across port reopen - #1828

Open
tostmann wants to merge 1 commit into
Koenkk:masterfrom
tostmann:fix/zboss-uart-reopen-stream-plumbing
Open

fix(zboss): keep reader/writer stream plumbing intact across port reopen#1828
tostmann wants to merge 1 commit into
Koenkk:masterfrom
tostmann:fix/zboss-uart-reopen-stream-plumbing

Conversation

@tostmann

Copy link
Copy Markdown
Contributor

Two stream-lifecycle bugs around the zboss uart's port reopen paths — the in-place reopen in onPortClose while inReset is set, and the stop() + resetNcp() reconnect flow. Both bite on TCP transports (NCP behind a serial-to-TCP bridge), where the peer legitimately closes the connection whenever the NCP reboots.

1. pipe(this.reader) without end: false

A port that emits end (remote FIN) also end()s the shared reader Transform — for good, per pipe()'s default end: true. The in-place reopen then pipes the new port into a finished stream, and every received byte is dropped as a silent write-after-end: the post-reboot boot frame never reaches onPackage, so NCP_RESET times out after 10 s although the device answered within ~2 s. Diagnosis signature: Socket ready in the log, then zero zh:zboss:read lines ever after.

2. closePort() never unpipes the writer

destroy() schedules the socket's close event for the next tick, but removeAllListeners() runs synchronously right after and strips the pipe machinery's own close-cleanup handler — so Node never auto-unpipes. After a reconnect the writer is piped to both the dead and the live port and keeps directing writes at the dead one; in the field this wedged outgoing frames after the first reconnect (command timeout, adapter restart).

Fix: pipe(reader, {end: false}) on both the serial and the TCP branch, and an explicit this.writer.unpipe() at the top of closePort().

Tests: new test/adapter/zboss/uart.test.ts runs the real ZBOSSUart against a two-sided fake socket with genuine stream semantics (a Duplex; incoming bytes via push(), remote FIN via push(null), outgoing captured in _write). One test drives a captured, CRC-valid NCP_RESET response frame through FIN + reopen and asserts frames keep flowing; the other drives the stop()/resetNcp() flow and asserts not a single write is directed at the dead port. Both fail on the previous code and pass with the fix. Full vitest suite and pnpm run check are green.

The combination was root-caused and verified end-to-end on test hardware: over a TCP-bridged ZBOSS NCP, factory reset, network formation and NVRAM-restore reconnect flows now complete where they previously timed out.

Two stream-lifecycle bugs around the zboss uart's port reopen paths (the
in-place reopen in onPortClose while `inReset` is set, and the stop() +
resetNcp() reconnect flow):

1. pipe(reader) without `end: false`: a port that emits `end` (a remote FIN —
   e.g. the NCP behind a serial-to-TCP bridge rebooting on NCP_RESET) ends the
   shared reader Transform for good. The reopen then pipes the new port into a
   finished stream and every received byte is dropped as a silent
   write-after-end — the post-reboot boot frame never reaches onPackage and
   the reset times out although the device answered.

2. closePort() never unpiped the writer: destroy() schedules the socket's
   `close` for the next tick, but removeAllListeners() runs synchronously
   right after and strips the pipe machinery's own close-cleanup handler, so
   Node never auto-unpipes. After a reconnect the writer is piped to BOTH the
   dead and the live port and keeps directing writes at the dead one.

Both are covered by new unit tests against a two-sided fake socket with real
stream semantics; each test fails on the previous code and passes with the
fix. The combination was root-caused and verified end-to-end on test
hardware (TCP-bridged ZBOSS NCP: factory reset, NVRAM restore and reconnect
flows now complete where they previously timed out).
@Koenkk

Koenkk commented Jul 30, 2026

Copy link
Copy Markdown
Owner

@kirovilya could you check this?

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.

2 participants