Skip to content

fix: deliver poller events with MakeCallback - #239

Open
Azq2 wants to merge 1 commit into
serialport:mainfrom
siemens-mobile-hacks:fix/poller-makecallback
Open

Azq2 wants to merge 1 commit into
serialport:mainfrom
siemens-mobile-hacks:fix/poller-makecallback

Conversation

@Azq2

@Azq2 Azq2 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fix native poller callbacks to use MakeCallback, so promise continuations scheduled by readable events run immediately instead of waiting for a later event-loop tick.

Example of bug:

    serialport/bindings-cpp/unixRead readable fired! +11ms <--- debug inside readable()
    serialport/bindings-cpp/unixRead Starting read +10s <--- await readable() is delayed until external setTimeout(() => { /* noop */ }, 10000) finished

I saw this problem when EAGAIN happens.

@Nerivec

Nerivec commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

@reconbot might this be the fix for serialport/node-serialport#3148 (or at least part of it)?
We're starting to see issues popup a bit everywhere about Node >=26.4 & serialport 😁

Source appears to be nodejs/node#62969

CC: @AlCalzone @Koenkk

@AlCalzone

Copy link
Copy Markdown

I can confirm that this fixes the issue for me.

@DragonWork

Copy link
Copy Markdown

@reconbot 👀

Woodii1998 added a commit to zhouxiyu1997/friendmaker that referenced this pull request Aug 19, 2026
* fix(serial): parse ACKs mixed with ESP-IDF logs and unblock Linux reads

- strip ANSI color codes and recognize ESP-IDF log lines
  (e.g. 'W (4652) BT_HCI: ...') so ACKs polluted by device logs parse
  correctly instead of being treated as malformed and triggering retry/reset
- when an ACK line is mixed with an embedded device log line, re-parse the
  ACK portion before dropping the line so the ACK is never lost
- patch @serialport/bindings-cpp unix-read with a 50ms poller fallback:
  on Linux the poller readable event can be missed (shared uv_poll,
  overriding registration), leaving serial data stranded in the fd for
  2-6s until the next device output arrives; the fallback re-tries fs.read
  so data is always read within 50ms

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(serial): align poller patch with upstream MakeCallback fix

- apply serialport/bindings-cpp#239 to deliver poller callbacks immediately
- keep the patch-package override on the native poller source
- strip complete common ANSI escape sequences before ACK parsing
- preserve ESP-IDF log recognition and mixed-line ACK recovery

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(serial): recover ACKs glued after device logs and dedupe ACK handling

Recognizing ESP-IDF log lines made sanitizeDeviceLine keep mixed
log-before-ACK lines intact, which skipped the old tail-ACK fallback and
dropped those ACKs as plain device logs. Extract a trailing sequenced
ACK from device log lines in waitForAck, forward the leading log
segment, and route both mixed-line paths through a shared
handleSequencedAck helper. Cover the line sanitizing and mixed-line ACK
recovery with unit tests.

Co-authored-by: Woodii <Woodii1998@users.noreply.github.com>

* chore: minimize package-lock diff to patch-package additions

Rebase the lockfile on main and merge in only the patch-package
dependency closure, dropping the unrelated peer/optional flag churn
introduced by regenerating the whole lockfile with a different npm
version.

Co-authored-by: Woodii <Woodii1998@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Woodii <Woodii1998@users.noreply.github.com>
@DragonWork

Copy link
Copy Markdown

@Nerivec @Koenkk Maybe at this point it would make sense to fork the serialport/bindings-cpp repo specifically for Zigbee2MQTT and incorporate both PRs from @Azq2, since the upstream repo appears to have been unmaintained for about eight months. Otherwise, once Node.js 26 becomes LTS, this could turn into a real apocalypse for Zigbee2MQTT, especially if the resulting failures can corrupt the coordinator database or state (even the backup when connection randomly succeeds and then times out later).

@Azq2

Azq2 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Project rip

groupsky added a commit to groupsky/ya-modbus that referenced this pull request Oct 12, 2026
Node.js 20 reached end-of-life on 2026-04-30 and gets no more security
fixes. Several dependency majors (chalk 6, @lerna-lite 5, npm 12) already
require Node.js 22 or newer, so keeping Node.js 20 blocks those updates.

Node.js 26 support waits for serialport/bindings-cpp#239: on Node.js
>=26.4 the serialport poller callback does not run pending promise work,
so serial (RTU) reads time out.

- engines: root "22 || 24" (drives the CI test matrix), packages
  ">=22.0.0" (no production dependency needs a newer 22.x)
- CI: remove the npm@11 install step that only Node.js 20 needed
- @types/node: move to 22.x and pin Dependabot to v22 (ignore >=23)
- dependabot-verify: read the minimum version from the "22 || 24"
  engines format and accept several matching @types/node ignore rules;
  before, the check found no ">=" and skipped with a warning
- docs: update the supported Node.js versions

BREAKING CHANGE: Node.js 20 is no longer supported. Use Node.js 22 or 24.
groupsky added a commit to groupsky/ya-modbus that referenced this pull request Oct 12, 2026
Node.js 20 reached end-of-life on 2026-04-30 and gets no more security fixes. Several dependency majors (chalk 6, @lerna-lite 5, npm 12) already require Node.js 22 or newer, so keeping Node.js 20 blocks those updates.

Node.js 26 support waits for serialport/bindings-cpp#239: on Node.js 26.4 and newer the serialport poller callback does not run pending promise work, so serial (RTU) reads time out.

- engines: root "22 || 24" (drives the CI test matrix), packages ">=22.0.0" (no production dependency needs a newer 22.x)
- CI: remove the npm@11 install step that only Node.js 20 needed
- @types/node: move to 22.x and pin Dependabot to v22 (ignore >=23)
- dependabot-verify: read the minimum version from the "22 || 24" engines format and accept several matching @types/node ignore rules; before, the check found no ">=" and skipped with a warning
- docs: update the supported Node.js versions

BREAKING CHANGE: Node.js 20 is no longer supported. Use Node.js 22 or 24.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants