Repository navigation
fix: keep sequence counter in sync with device-initiated packets - #709
Open
atwright147 wants to merge 1 commit into
Open
atwright147 wants to merge 1 commit into
atwright147 wants to merge 1 commit into
Conversation
The device advances its own sequence number for every packet it sends,
including unsolicited ones — e.g. the STATUS/DP_REFRESH push a v3.4 device
emits immediately after a SET. `_currentSequenceN` was only advanced when *we*
sent a packet, so after such a push the client's next request reused a sequence
number the device had already consumed. The device then answered with a
different number and the resolver registered for the request was never called,
leaving the pending get()/set() unresolved until the socket was recycled.
Reproduced against a Tuya v3.4 panel heater:
connect -> _currentSequenceN = 53896
SET -> request seq 53897 ; device acks seq 53897
(push) -> device sends STATUS seq 53898 ; client counter stays 53897
GET -> request seq 53898 ; device replies seq 53899
-> no resolver for 53899 -> get() hangs
Re-sync `_currentSequenceN` from every received packet in `_packetHandler()`.
Only moving it forward keeps it monotonic. This is safe for the session-key
negotiation: the SESS_KEY_NEG_RES branch that follows still rebases the counter
with `sequenceN - 1`.
Adds test/sequence.js covering the re-sync, a no-rewind guard, and the
end-to-end case where a request must still resolve after an unsolicited push.
This branch has not been deployed
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.
Problem
A Tuya device advances its own sequence number for every packet it sends, including unsolicited ones — most notably the
STATUS/DP_REFRESHpush a v3.4 device emits immediately after aSET.TuyAPI only advances
_currentSequenceNwhen it sends a packet. After an unsolicited push the two counters drift, so the client's next request reuses a sequence number the device has already consumed. The device answers with a different number, no resolver matches, and the pendingget()/set()never resolves — it hangs until the socket is recycled.This makes
get()immediately afterset()hang deterministically on a v3.4 device, even though theSETitself succeeded.Reproduction (Tuya v3.4 panel heater)
Packet trace from the real device:
connect -> get()on its own works, because the counters happen to stay aligned.connect -> set -> get()always hangs. Every subsequentget()in that session hangs too, because the offset persists.The same is visible with
shouldWaitForResponse: false:set()returns immediately (fire-and-forget), but the asynchronous push still leaves the counter behind.Fix
Re-sync
_currentSequenceNfrom each received packet in_packetHandler():Only moving it forward keeps it monotonic (an out-of-order or older packet never rewinds it), so the next request uses the sequence number the device will answer with.
This is intentionally placed at the top of
_packetHandler()so it also applies to packets handled by the early-return branches (theSETack, the emptyDP_REFRESHresponse). It's safe for the session-key negotiation because theSESS_KEY_NEG_RESbranch that follows rebases the counter withsequenceN - 1regardless.Tests
Adds
test/sequence.js:Without the fix the first and third fail; with it, the full suite passes (
42 tests passed).Verification
Checked against two ECOSO Torr panel heaters (Tuya v3.4) via a Homey app:
get()now succeeds immediately afterset(), and repeatedly (previously it timed out after every write).Notes
get()/refresh()requests that are never answered. That mitigates the leak; this change prevents the hang at its source, so the two are complementary.