feat(br_server): add REST API discovery endpoint (/.well-known/thread/esp-br-rest) - #216
feat(br_server): add REST API discovery endpoint (/.well-known/thread/esp-br-rest)#216lboue wants to merge 1 commit into
Conversation
| cJSON *response = cJSON_CreateObject(); | ||
| ESP_RETURN_ON_FALSE(response, ESP_FAIL, WEB_TAG, "Failed to allocate well-known response"); | ||
|
|
||
| cJSON *api = cJSON_CreateObject(); |
There was a problem hiding this comment.
Check the api pointer is NULL before we use it.
|
Thanks for the detailed feedback @zwx1995esp! I agree. Starting at On the One related thought for down the line: python-otbr-api recently added Will also fix the NULL check on |
That, or use an entirely different versioning endpoint, e.g. |
Address two review comments on the REST API discovery endpoint: - Serve the discovery endpoint on its own esp-thread-br-specific well-known path (/.well-known/thread/esp-br-rest) instead of ot-br-posix's (/.well-known/thread/br-rest), since the two implementations do not expose the same REST API contract. This lets clients that support both APIs probe each implementation's own path to disambiguate them, per agners' suggestion (espressif#216 (comment)). - Fold ESP_OT_REST_API_VERSION into esp_br_web_api.h and drop the now-unneeded esp_br_web_version.h, per zwx1995esp's suggestion (espressif#216 (comment)). Also add a missing NULL check on the "api" cJSON object in the discovery handler.
…in sync Address zwx1995esp's remaining review comment (espressif#216 (comment)): - esp-thread-br and ot-br-posix do not currently expose the same REST API contract (different base path, and ot-br-posix has endpoints esp-thread-br does not implement), so ESP_OT_REST_API_VERSION should not mirror ot-br-posix's version. Start it, and openapi.yaml's info.version, at 1.0.0 and version esp-thread-br's REST API independently from here on. - Add tools/ci/check_rest_api_version_sync.py, wired into .pre-commit-config.yaml, to fail if ESP_OT_REST_API_VERSION and openapi.yaml's info.version ever drift apart.
Keep the rationale for the esp-br-rest well-known path in the code comments, but drop the link to the specific review comment (espressif#216 (comment)) — it doesn't belong as a permanent reference in the source.
lboue
changed the title
|
Thanks both for the review! Pushed changes addressing each point:
|
Address zwx1995esp's remaining review comment on PR espressif#216 (espressif#216 (comment)): drop implementation details, the br-rest/esp-br-rest naming rationale, and the historical justification from the discovery endpoint's doc comments in esp_br_web.c and esp_br_web_api.h, keeping only what a user of the API needs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zwx1995esp
left a comment
zwx1995esp
left a comment
There was a problem hiding this comment.
Hi, @lboue I think we can simplify the docs and remove some redundant description. And could you please squash the 7 commits into 1? I think this PR will be ready after fixing those.
| ESP Thread Border Router Wer Server REST API | ||
| ----------------------------------------------------------------------*/ | ||
| /* HTTP GET */ | ||
| /* RFC 8615 well-known URI for REST API discovery: reports the running API version and links to its |
There was a problem hiding this comment.
Remove the line 42 and 43. I think we have already explained this at the line 828
| tags: | ||
| - discovery | ||
| summary: Discover the REST API version and entry points | ||
| description: |- |
There was a problem hiding this comment.
Remove this description, since we have already explained the details in the file esp_br_web.c.
| [Semantic Versioning](https://semver.org/): MAJOR for incompatible API changes, MINOR for | ||
| backward-compatible additions, PATCH for backward-compatible fixes. | ||
|
|
||
| esp-thread-br versions its REST API independently from ot-br-posix's `OTBR_REST_API_VERSION`, |
There was a problem hiding this comment.
Let's remove the line from 8 to 10.
…/esp-br-rest) Add a REST API discovery endpoint and a tracked version number for esp-thread-br's own REST API. - GET /.well-known/thread/esp-br-rest: RFC 8615 well-known URI returning the running REST API's version and RFC 8288-style links to its entry points, so clients can discover capabilities at runtime instead of hardcoding/probing endpoint paths. Served on its own esp-thread-br-specific path so clients that need to support both esp-thread-br's and ot-br-posix's REST APIs can probe each implementation's own well-known path to disambiguate them. - ESP_OT_REST_API_VERSION (in esp_br_web_api.h): single source of truth for the REST API's semver version, starting at 1.0.0 and versioned independently from ot-br-posix's OTBR_REST_API_VERSION, since the two implementations' REST surfaces differ. - Bump openapi.yaml's info.version (and ESP_OT_REST_API_VERSION) to 1.1.0, accounting for the ePSKc endpoints (/node/ba-epskc/state, /node/ba-epskc/key) added since the 1.0.0 baseline without a version bump. - tools/ci/check_rest_api_version_sync.py, wired into .pre-commit-config.yaml, fails if ESP_OT_REST_API_VERSION and openapi.yaml's info.version ever drift apart. - components/esp_ot_br_server/CHANGELOG.md tracks the REST API's version history going forward. Closes espressif#215. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
d6cccd4 to
4f1d409
Compare
|
Merged here: ff0d1e3 |
3 participants
Description
Summary
Adds a REST API discovery endpoint and a tracked version number for esp-thread-br's own REST API, inspired by ot-br-posix's discovery endpoint but adapted to the fact that the two implementations do not expose the same REST API contract. Closes #215.
GET /.well-known/thread/esp-br-rest— RFC 8615 well-known URI returning the running REST API's version and RFC 8288-style links to its entry points, so clients can discover capabilities at runtime instead of hardcoding/probing endpoint paths. Inspired by ot-br-posix's endpoint ([rest] add REST API version in /.well-known/thread for API discovery openthread/ot-br-posix#3330), but served on its own esp-thread-br-specific path (rather than ot-br-posix's/.well-known/thread/br-rest) so clients that need to support both APIs can probe each implementation's own well-known path to disambiguate them.ESP_OT_REST_API_VERSION(inesp_br_web_api.h) — new single source of truth for the REST API's semver version, analogous to ot-br-posix'ssrc/rest/version.hpp. Versioned independently from ot-br-posix'sOTBR_REST_API_VERSION, since the two implementations' REST surfaces differ (different base path, and ot-br-posix has endpoints — actions, devices, JSON:API diagnostics, commissioner, coprocessor-version — that esp-thread-br doesn't implement).openapi.yaml'sinfo.version(andESP_OT_REST_API_VERSION) to1.1.0:1.0.0was the pre-existing baseline, and1.1.0accounts for the ePSKc endpoints (/node/ba-epskc/state,/node/ba-epskc/key) added since then without a version bump.tools/ci/check_rest_api_version_sync.py, wired into.pre-commit-config.yaml, fails ifESP_OT_REST_API_VERSIONandopenapi.yaml'sinfo.versionever drift apart.components/esp_ot_br_server/CHANGELOG.mdtracks the REST API's version history going forward.Why
This component reimplements part of the same REST surface as ot-br-posix but had no way for a client to discover the API version or capabilities at runtime, and its OpenAPI
info.versionhad never been bumped even across earlier additive endpoint changes. See #215 for the discussion.Going forward
Bump
ESP_OT_REST_API_VERSIONandopenapi.yaml'sinfo.versiontogether whenever a REST endpoint is added/changed, following semver (MAJOR = breaking, MINOR = backward-compatible addition, PATCH = bug fix), and record the change incomponents/esp_ot_br_server/CHANGELOG.md.check_rest_api_version_sync.pyenforces the two version fields stay in sync.Testing
openapi.yamlvalidated withpython3 -c "import yaml; yaml.safe_load(open(...))".tools/ci/check_rest_api_version_sync.pyrun locally, including a forced-mismatch case to confirm it fails as expected.