fix(iot-client): send resetFactory, and let the caller choose it - #28
Merged
Conversation
The request body was `{"t":…}` only. tuya.device.reset requires resetFactory, so
the call as shipped came back rejected -- and rejected in the way that reads
like a network fault rather than a missing field, since the return code alone
says nothing about which parameter the cloud objected to.
The field is not a formality: it picks between the cloud's two meanings for
removing a device, and they differ in whether they can be undone. New
iot_reset_scope_t makes that the caller's decision:
IOT_RESET_UNBIND_ONLY (false) -- give up the user-device binding, leave the
device's cloud-side data in place. Re-pairing can pick it up again.
IOT_RESET_FACTORY (true) -- also discard the data the cloud holds for this
device, business-specific exclusions aside. NOT reversible: re-pairing
yields a new binding, not the old state.
Same pair of meanings the inbound protocol-11 notice already carries, in the
opposite direction -- which is why this is a separate type rather than a reuse
of iot_reset_type_t: those constants are named REMOTE_ because they describe a
push the cloud initiated, and read backwards on a call the device makes. An enum
rather than a bool because a bare `true` at the call site would not say which
one it is, and picking the wrong one cannot be walked back.
Neither scope is a way to reconnect cleanly or recover from an error -- both
give the binding up. The header and reference doc say so and point at
disconnect/connect instead.
The interface version is 5.0. A first attempt used 3.0 and the cloud answered
API_OR_API_VERSION_WRONG on a real device -- an error that does not even say
whether the name or the version was at fault. Worth recording because it marks
the limit of the mock's version pin: the mock only knows the value this repo
told it, so it catches an edit to the constant, not a wrong constant. Only the
cloud can confirm the pair, and the test comment now says so.
The mock also requires resetFactory and pins the choice, because nothing in the
response can show it: the interface answers with an empty result either way. A
devId containing "factory" must arrive with resetFactory=true, any other with
false, so a scope that fails to reach the wire is rejected instead of passing
silently. Verified by inverting the mapping -- iot_reset_test drops to 2/5. The
success envelope still returns an empty result, matching the real interface,
which is itself something the suite asserts.
api-activate --release passes IOT_RESET_UNBIND_ONLY so the demo can be re-run;
its comment names what a real decommission would pass instead.
Verified: full build clean; ctest --timeout 180 14/14; iot_reset_test 5/5
(1 new); activate_demo builds against the new signature. The api/version pair
itself is not verified by any of that -- it needs a real device.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heshaoqiong-tuya
added a commit
that referenced
this pull request
Aug 27, 2026
Applies convention 6 to the section that motivated it: 619 lines down to 139. The entries were commit bodies pasted under a heading — motivation, rejected alternatives, test counts and internal reasoning, several running past fifty lines for one change. All of that is still in the commits, which is where a reader who wants it looks. What is left here is what a reader acts on: the symbol, the behaviour change, the migration step. Most entries now carry a PR number. They could not be recovered from git history because these landed as squash or rebase merges, which leave no "Merge pull request #N" commit; the closed-PR list on GitHub has them, and each attribution was confirmed against that PR's own commit list rather than inferred from a branch name. Four entries carry none because they were pushed straight to master with no PR to cite. Two merged PRs turned out to have no entry at all, and are added: - #21, the APP-confirmed OTA (protocol 15) callback — a public callback on both config structs. - #23, the sizable ATOP response buffer — user-visible, since the sizes are set with -D. Three defects the rewrite surfaced, all from entries being appended rather than amended as the work continued: - Added and Fixed each appeared twice, and the second Fixed held Added-type material (the generic ATOP call). Merged into one of each, in the order Keep a Changelog defines, since release notes are generated from those headings. - The auto-connect default was documented both ways: Changed said it is now on by default, while the connect/disconnect entry still said "The default stays false". The later change never revisited the earlier entry. - The music-play demo was credited to #15, which is the region-wire-codes fix. It is #12. Two entries are dropped rather than shortened: the mqtt_abort_connect() extraction and a test-only over-read fix. Convention 2 scopes the CHANGELOG to what SDK users see, and neither is visible outside the repo. Rebased onto five commits that landed meanwhile, whose entries are folded in at the new length: the session-token-reason API (#29), the reset scope (#28), the POSIX binary renaming, and audio_chat_demo's header-shadowing and device-VAD fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
heshaoqiong-tuya
added a commit
that referenced
this pull request
Aug 27, 2026
Applies convention 6 to the section that motivated it: 619 lines down to 139. The entries were commit bodies pasted under a heading — motivation, rejected alternatives, test counts and internal reasoning, several running past fifty lines for one change. All of that is still in the commits, which is where a reader who wants it looks. What is left here is what a reader acts on: the symbol, the behaviour change, the migration step. Most entries now carry a PR number. They could not be recovered from git history because these landed as squash or rebase merges, which leave no "Merge pull request #N" commit; the closed-PR list on GitHub has them, and each attribution was confirmed against that PR's own commit list rather than inferred from a branch name. Four entries carry none because they were pushed straight to master with no PR to cite. Two merged PRs turned out to have no entry at all, and are added: - #21, the APP-confirmed OTA (protocol 15) callback — a public callback on both config structs. - #23, the sizable ATOP response buffer — user-visible, since the sizes are set with -D. Three defects the rewrite surfaced, all from entries being appended rather than amended as the work continued: - Added and Fixed each appeared twice, and the second Fixed held Added-type material (the generic ATOP call). Merged into one of each, in the order Keep a Changelog defines, since release notes are generated from those headings. - The auto-connect default was documented both ways: Changed said it is now on by default, while the connect/disconnect entry still said "The default stays false". The later change never revisited the earlier entry. - The music-play demo was credited to #15, which is the region-wire-codes fix. It is #12. Two entries are dropped rather than shortened: the mqtt_abort_connect() extraction and a test-only over-read fix. Convention 2 scopes the CHANGELOG to what SDK users see, and neither is visible outside the repo. Rebased onto five commits that landed meanwhile, whose entries are folded in at the new length: the session-token-reason API (#29), the reset scope (#28), the POSIX binary renaming, and audio_chat_demo's header-shadowing and device-VAD fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
heshaoqiong-tuya
added a commit
that referenced
this pull request
Aug 27, 2026
Applies convention 6 to the section that motivated it: 619 lines down to 139. The entries were commit bodies pasted under a heading — motivation, rejected alternatives, test counts and internal reasoning, several running past fifty lines for one change. All of that is still in the commits, which is where a reader who wants it looks. What is left here is what a reader acts on: the symbol, the behaviour change, the migration step. Most entries now carry a PR number. They could not be recovered from git history because these landed as squash or rebase merges, which leave no "Merge pull request #N" commit; the closed-PR list on GitHub has them, and each attribution was confirmed against that PR's own commit list rather than inferred from a branch name. Four entries carry none because they were pushed straight to master with no PR to cite. Two merged PRs turned out to have no entry at all, and are added: - #21, the APP-confirmed OTA (protocol 15) callback — a public callback on both config structs. - #23, the sizable ATOP response buffer — user-visible, since the sizes are set with -D. Three defects the rewrite surfaced, all from entries being appended rather than amended as the work continued: - Added and Fixed each appeared twice, and the second Fixed held Added-type material (the generic ATOP call). Merged into one of each, in the order Keep a Changelog defines, since release notes are generated from those headings. - The auto-connect default was documented both ways: Changed said it is now on by default, while the connect/disconnect entry still said "The default stays false". The later change never revisited the earlier entry. - The music-play demo was credited to #15, which is the region-wire-codes fix. It is #12. Two entries are dropped rather than shortened: the mqtt_abort_connect() extraction and a test-only over-read fix. Convention 2 scopes the CHANGELOG to what SDK users see, and neither is visible outside the repo. Rebased onto five commits that landed meanwhile, whose entries are folded in at the new length: the session-token-reason API (#29), the reset scope (#28), the POSIX binary renaming, and audio_chat_demo's header-shadowing and device-VAD fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sedawwk
pushed a commit
that referenced
this pull request
Aug 27, 2026
* docs: require terse CHANGELOG entries with a PR number The Unreleased section had drifted into commit bodies pasted under a heading: single entries running fifteen-plus lines through motivation, rejected alternatives and test counts. That is the right material, in the wrong file -- release notes are skimmed, and a reader who wants the reasoning goes to the commit. Convention 2 already said *when* an entry is needed; this says what it should look like. `## [0.3.0]` is named as the reference because it is the last section written that way: one line per change, sub-bullets only for specifics a reader acts on. Two things the section had also been losing: the PR number, which is the only link from a one-line summary back to the reasoning, and the Added / Changed / Fixed split, which release notes are generated from -- a fix landing under Added is published as a feature. No CHANGELOG entry for this commit, per convention 2: repo-internal, and this file is explicitly excluded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(changelog): condense Unreleased to the new convention Applies convention 6 to the section that motivated it: 619 lines down to 139. The entries were commit bodies pasted under a heading — motivation, rejected alternatives, test counts and internal reasoning, several running past fifty lines for one change. All of that is still in the commits, which is where a reader who wants it looks. What is left here is what a reader acts on: the symbol, the behaviour change, the migration step. Most entries now carry a PR number. They could not be recovered from git history because these landed as squash or rebase merges, which leave no "Merge pull request #N" commit; the closed-PR list on GitHub has them, and each attribution was confirmed against that PR's own commit list rather than inferred from a branch name. Four entries carry none because they were pushed straight to master with no PR to cite. Two merged PRs turned out to have no entry at all, and are added: - #21, the APP-confirmed OTA (protocol 15) callback — a public callback on both config structs. - #23, the sizable ATOP response buffer — user-visible, since the sizes are set with -D. Three defects the rewrite surfaced, all from entries being appended rather than amended as the work continued: - Added and Fixed each appeared twice, and the second Fixed held Added-type material (the generic ATOP call). Merged into one of each, in the order Keep a Changelog defines, since release notes are generated from those headings. - The auto-connect default was documented both ways: Changed said it is now on by default, while the connect/disconnect entry still said "The default stays false". The later change never revisited the earlier entry. - The music-play demo was credited to #15, which is the region-wire-codes fix. It is #12. Two entries are dropped rather than shortened: the mqtt_abort_connect() extraction and a test-only over-read fix. Convention 2 scopes the CHANGELOG to what SDK users see, and neither is visible outside the repo. Rebased onto five commits that landed meanwhile, whose entries are folded in at the new length: the session-token-reason API (#29), the reset scope (#28), the POSIX binary renaming, and audio_chat_demo's header-shadowing and device-VAD fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The request body was
{"t":…}only. tuya.device.reset requires resetFactory, so the call as shipped came back rejected -- and rejected in the way that reads like a network fault rather than a missing field, since the return code alone says nothing about which parameter the cloud objected to.The field is not a formality: it picks between the cloud's two meanings for removing a device, and they differ in whether they can be undone. New iot_reset_scope_t makes that the caller's decision:
IOT_RESET_UNBIND_ONLY (false) -- give up the user-device binding, leave the
device's cloud-side data in place. Re-pairing can pick it up again.
IOT_RESET_FACTORY (true) -- also discard the data the cloud holds for this
device, business-specific exclusions aside. NOT reversible: re-pairing
yields a new binding, not the old state.
Same pair of meanings the inbound protocol-11 notice already carries, in the opposite direction -- which is why this is a separate type rather than a reuse of iot_reset_type_t: those constants are named REMOTE_ because they describe a push the cloud initiated, and read backwards on a call the device makes. An enum rather than a bool because a bare
trueat the call site would not say which one it is, and picking the wrong one cannot be walked back.Neither scope is a way to reconnect cleanly or recover from an error -- both give the binding up. The header and reference doc say so and point at disconnect/connect instead.
The interface version is 5.0. A first attempt used 3.0 and the cloud answered API_OR_API_VERSION_WRONG on a real device -- an error that does not even say whether the name or the version was at fault. Worth recording because it marks the limit of the mock's version pin: the mock only knows the value this repo told it, so it catches an edit to the constant, not a wrong constant. Only the cloud can confirm the pair, and the test comment now says so.
The mock also requires resetFactory and pins the choice, because nothing in the response can show it: the interface answers with an empty result either way. A devId containing "factory" must arrive with resetFactory=true, any other with false, so a scope that fails to reach the wire is rejected instead of passing silently. Verified by inverting the mapping -- iot_reset_test drops to 2/5. The success envelope still returns an empty result, matching the real interface, which is itself something the suite asserts.
api-activate --release passes IOT_RESET_UNBIND_ONLY so the demo can be re-run; its comment names what a real decommission would pass instead.
Verified: full build clean; ctest --timeout 180 14/14; iot_reset_test 5/5 (1 new); activate_demo builds against the new signature. The api/version pair itself is not verified by any of that -- it needs a real device.