Feat/atop generic call - #19
Merged
Merged
Conversation
A well-formed envelope carrying success=false was reported as success. atop_response_result_parse_cjson() logged errorCode / errorMsg, dropped them, and returned OPRT_OK for every code except GATEWAY_NOT_EXISTS. Detecting the failure required inspecting atop_base_response_t.success separately, and only three of the eight named wrappers did. Worst case was tuya.device.schema.newest.get: a rejection arrived as OPRT_OK with result == NULL, which atop_schema_newest_get() reports as "no newer schema". A permission error and an up-to-date schema were indistinguishable from the return value alone. The envelope now copies both strings into new error_code / error_msg fields on atop_base_response_t and returns the new OPRT_ATOP_BUSINESS_ERROR (-0x000E) -- for every rejection uniformly. GATEWAY_NOT_EXISTS loses its historical OPRT_COMMUNICATION_ERROR mapping: nothing in the repo branches on that mapping, and it presented a permanent "device removed from the cloud" verdict as a retryable transport failure. A caller that needs per-code policy now branches on error_code, which is carried on the response for exactly that purpose. Fallout handled here rather than left to drift: - iot_on_boarding.c's error switch names the new code, so a cloud- rejected activation (expired/used pairing token) logs "Rejected by the cloud" instead of falling to "Unknown error". - The .success re-checks in atop_version_update / atop_upgrade_status_ update are unreachable under the new contract and were removed; the header now states callers never need to inspect .success. - A rejection whose envelope lacks errorCode still logs the server's errorMsg (previously dropped on that path). - atop_base_response_free() is NULL-safe and frees result whenever set instead of gating on .success, which only ever added a way to leak. Behavioral note: named wrappers now return -0x000E where a cloud rejection previously surfaced as OPRT_OK-with-empty-result (or, in two wrappers, an internal OPRT_COMMUNICATION_ERROR mapping). Two tests in atop_test.c pin the contract: a rejected call must return OPRT_ATOP_BUSINESS_ERROR with errorCode attached, and an empty result must still be OPRT_OK -- the pair that keeps the two cases distinguishable. iot_atop_test 20/20, full suite 13/13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK wraps eight ATOP interfaces by name; the cloud exposes many more.
Business layers that need one of the others had to wait for an SDK
release, or reach into src/atop_base.h -- which the build already makes
possible (src/ is on the PUBLIC include path and the library is static)
but which is undocumented, untested, and free to change.
New public header iot_atop.h turns that capability into a supported API:
supply an api name, its version and a JSON body, get the envelope's
result back as a JSON string plus the cloud's errorCode / errorMsg and
server time. Signing, AES-GCM body encryption, TLS, host resolution and
envelope parsing stay inside the SDK.
Four deliberate choices:
- Takes iot_client_t rather than devid/key, following iot_ota.c. The
caller never handles the device secret key and cannot forget the CA
bundle. This is the main gain over exposing atop_base_request() itself.
- Returns result as a JSON string, not cJSON *. Keeps cJSON out of the
public ABI, so applications are not pinned to the SDK's cJSON version,
and memory ownership stays on one side. The printed string is returned
directly -- cJSON's hooks are the pal allocator, and no successful
response can precede iot_init(): the AES-GCM nonce needs rng_bytes(),
which fails closed until rng_init() runs inside that same function.
A {"result":null} envelope comes back as NULL, not the string "null".
- errorCode / errorMsg reach the caller. The return code separates
transport from verdict: OPRT_OK means the cloud accepted, and
OPRT_ATOP_BUSINESS_ERROR means it rejected and said why. For an
interface the SDK does not know, the cloud's own code is the only
reliable signal, so it cannot stay in the log.
- Activated devices only -- it signs with devid + secret_key and returns
OPRT_UNINITIALIZED otherwise. Activation signs with uuid + authkey and
stays behind iot_client_init_on_boarding(); the generic path gives no
way to hand-assemble an activation request.
The response is zeroed before any other validation, so the documented
"zeroed on entry / free on every path" contract holds on the argument-
guard early returns too. Request bodies are forwarded verbatim (callers
supply what the interface needs, including the `t` field most ATOP
interfaces want in the body) and validated as "parses as a JSON object
with nothing trailing" -- cJSON_ParseWithOpts with require_null_
terminated, so a "{}garbage" snprintf slip fails locally instead of
spending an HTTPS round trip.
The mock's key selection is keyed purely off the URL identity param --
devId means sec_key, otherwise authkey -- instead of a per-API list.
That is what lets a test reach an interface the mock has no handler for,
which is exactly the case the generic entry exists to serve. Two
test-only vectors (result:null, GATEWAY_NOT_EXISTS) pin the null-result
and uniform-verdict contracts.
The ESP-IDF component's source list gains iot_atop.c alongside the root
CMakeLists, so the API is linkable everywhere its header is visible.
The guide lists the existing named wrappers so they are not
reimplemented, and states the three criteria for promoting an interface
to a named wrapper: used by more than one product, non-trivial protocol
semantics, or needs SDK-internal state. None met is a legitimate resting
place, not debt. CONTEXT.md gains the ATOP vocabulary it lacked
entirely -- ATOP interface, envelope, named wrapper, generic call -- and
flags 封装/wrap as the ambiguity that kept the design discussion circling.
iot_atop_call_test 13/13, full suite 13/13, posix examples build clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
include(CTest) only calls enable_testing() while BUILD_TESTING is ON, but we force it OFF to silence third-party tests. First configure registered tests; every re-configure after that silently stopped regenerating CTestTestfile.cmake, so tests added since (iot_ota_verify_test, iot_atop_call_test) built but never ran under ctest. Call enable_testing() explicitly for our own tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
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
2 participants
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.
Summary
The SDK wraps eight ATOP interfaces by name; the cloud has many more. Reaching one of the
others meant waiting for an SDK release, or reaching into
src/atop_base.h— which thebuild permits but which is undocumented, untested, and free to change. This adds a
supported entry point for them, plus the envelope fix it depends on.
iot_atop_call()New public header
iot_atop.h: anapiname, itsversionand a JSON body in; theenvelope's
resultout as a JSON string, plus the cloud'serrorCode/errorMsgandserver time. Signing, AES-GCM body encryption, TLS, host resolution and envelope parsing
stay inside the SDK.
iot_client_t(followingiot_ota.c), so the caller never handles the devicesecret key and cannot forget the CA bundle.
resultis a JSON string, notcJSON *— keeps cJSON out of the public ABI and memoryownership on one side (
iot_atop_response_free).{"result":null}comes back asNULL,not
"null".OPRT_UNINITIALIZEDotherwise. Activation signs withuuid+authkeyand stays behindiot_client_init_on_boarding().tfield most interfaces expect —validated only as "parses as a JSON object with nothing trailing", so a typo fails
locally instead of costing an HTTPS round trip.
Breaking: rejections now have their own return code
The envelope used to log
errorCode/errorMsg, drop them, and returnOPRT_OK.Detecting a rejection meant inspecting
atop_base_response_t.successseparately, and onlythree of the eight wrappers did. Worst case: a rejected
tuya.device.schema.newest.getarrived as
OPRT_OKwithresult == NULL, whichatop_schema_newest_get()reports as "nonewer schema" — a permission error was indistinguishable from an up-to-date schema.
Rejections now carry
error_code/error_msgon the response and return a newOPRT_ATOP_BUSINESS_ERROR(-0x000E), uniformly. Two consequences for existing code:-0x000Ewhere a rejection previously surfaced asOPRT_OK-with-empty-result, or asOPRT_COMMUNICATION_ERRORin two of them.GATEWAY_NOT_EXISTSloses itsOPRT_COMMUNICATION_ERRORmapping — it presented apermanent "device removed from the cloud" verdict as a retryable transport failure.
Branch on
error_codefor per-code policy.Testing
Full suite 13/13, POSIX examples build clean.
It was 12 before this branch:
include(CTest)only callsenable_testing()whileBUILD_TESTINGis ON, which we FORCE off to silence the vendored suites, so re-configureshad silently stopped regenerating the ctest list —
iot_ota_verify_test(from #17) andthis branch's
iot_atop_call_testcompiled without ever running. Fixed here.iot_atop_call_testis 13 cases, with two mock vectors pinning theresult: nullandrejection paths; two cases in
atop_test.cpin the envelope contract itself.Docs: new guide
atop-generic-call.md, including the criteria for promoting an interfaceto a named wrapper; ATOP vocabulary added to
modules/iot-client/CONTEXT.md; CHANGELOG andsidebars.tsupdated.