Sync develop with latest changes - #95
Conversation
Patch Release 0.6.1
Release 0.6.2
* RDKEMW-20911 : Return full JSON document from Actions.intent/onIntent * RDKEMW-20911 : Address copilot comments * RDKEMW-20911: Update changelog for v0.6.3
There was a problem hiding this comment.
Pull request overview
This PR updates the Firebolt Actions surface to support an intent payload represented as a structured JSON object (instead of a plain string), updates the OpenRPC spec accordingly, adjusts unit/component tests, and adds an API test-app demo for Actions.
Changes:
- Change
Actions.intent/Actions.onIntentfrom string payloads to a structuredIntentobject, and addActions.startto the C++ interface/impl. - Update OpenRPC schema/examples and tests to reflect the new object payload shape.
- Add
ActionsDemoto the API test app; improve Lifecycle demo unsubscribe UX by tracking the last subscription id.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/actionsTest.cpp | Updates unit tests to validate structured intent payload and adds a unit test for Actions.start. |
| test/component/actionsGeneratedTest.cpp | Updates component tests for structured payload + adds component coverage for Actions.start. |
| test/api_test_app/main.cpp | Registers the new ActionsDemo in the API test app. |
| test/api_test_app/apis/lifecycleDemo.h | Tracks last Lifecycle subscription id for later unsubscribe convenience. |
| test/api_test_app/apis/lifecycleDemo.cpp | Uses the tracked subscription id as the default for unsubscribe input. |
| test/api_test_app/apis/actionsDemo.h | Introduces an Actions demo interface for the API test app. |
| test/api_test_app/apis/actionsDemo.cpp | Implements interactive calls for Actions.intent, Actions.start, and Actions.onIntent subscription handling. |
| src/json_types/actions.h | Adds JSON deserialization for Firebolt::Actions::Intent wire payload. |
| src/actions_impl.h | Updates Actions interface to return/emit Intent and declares start. |
| src/actions_impl.cpp | Implements intent deserialization, subscription payload deserialization, and Actions.start invocation. |
| include/firebolt/actions.h | Introduces IntentContext / IntentData / Intent and changes public API signatures accordingly. |
| docs/openrpc/the-spec/firebolt-open-rpc.json | Updates Actions.intent / Actions.onIntent schemas and adds Actions.start definition. |
| CHANGELOG.md | Adds release note for the Actions intent payload change (needs alignment with actual payload shape/API impact). |
| virtual Result<Intent> intent() const = 0; | ||
|
|
||
| virtual Result<SubscriptionId> subscribeOnIntent(std::function<void(const std::string&)>&& notification) = 0; | ||
| virtual Result<SubscriptionId> subscribeOnIntentChanged(std::function<void(const std::string&)>&& notification) | ||
| virtual Result<SubscriptionId> subscribeOnIntent(std::function<void(const Intent&)>&& notification) = 0; | ||
| virtual Result<SubscriptionId> subscribeOnIntentChanged(std::function<void(const Intent&)>&& notification) |
| ## [0.6.3](https://github.com/rdkcentral/firebolt-cpp-client/compare/0.6.2...v0.6.3) | ||
|
|
||
| ### Fixed | ||
| - `Actions.intent` and `Actions.onIntent` now correctly handle a JSON object payload (`{"intent":"...","intentId":N}`) sent by the Firebolt backend. Previously the client failed to parse the response because it expected a plain string. |
| #include "actionsDemo.h" | ||
| #include <firebolt/firebolt.h> | ||
| #include <iostream> | ||
| #include <string> | ||
| #include <utility> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Suppressed comments (7)
include/firebolt/actions.h:58
- This file is explicitly marked "AUTO-GENERATED by fb-gen — DO NOT EDIT", but the PR changes the public Actions API (e.g., intent() now returns Result and the subscription payload type changed). These edits should be made in the generator and the file re-generated; otherwise it’s easy for future syncs to overwrite this change and it’s a breaking change for any existing consumers expecting the prior signature.
virtual Result<Intent> intent() const = 0;
virtual Result<SubscriptionId> subscribeOnIntent(std::function<void(const Intent&)>&& notification) = 0;
docs/openrpc/the-spec/firebolt-open-rpc.json:890
- Same as above: this watchedV2 example result should be
null(nottrue) to match the C++ API’s Result signature.
"result": {
"name": "result",
"value": true
}
docs/openrpc/the-spec/firebolt-open-rpc.json:3783
- The MemoryUsage schema was renamed to
*KiBproperties, but the client implementation still deserializesuserMemoryUsed,userMemoryLimit,gpuMemoryUsed,gpuMemoryLimit(src/json_types/stats.h:32-44) into Firebolt::Stats::MemoryInfo. Either the schema/spec needs to match the client, or the client + tests need updating in the same PR; otherwise schema validation and fixture-based tests will break.
"MemoryUsage": {
"title": "MemoryUsage",
"type": "object",
"description": "Describes current and maximum memory usage of the container.",
"properties": {
"userMemoryUsedKiB": {
"type": "integer",
"description": "User memory currently used in 1024 bytes."
},
src/actions_impl.h:43
- This implementation header is marked auto-generated, but the PR directly changes the interface/override signatures. These changes should come from the generator so that future re-generation doesn’t revert them and the generated unit/component tests stay consistent.
Result<Intent> intent() const override;
Result<SubscriptionId> subscribeOnIntent(std::function<void(const Intent&)>&& notification) override;
Result<void> start(const IntentData& intent, std::optional<std::string> handlerAppId = std::nullopt) const override;
src/json_types/actions.h:44
- This json_types header is marked auto-generated; changing the wire-to-native mapping here by hand is likely to be lost on the next generation/sync. Please update the generator to emit the desired JSON adapter and re-generate the Actions module outputs.
// Deserialises the wire object {"intent":{"action":"...","context":{"source":"..."}},"intentId":N}
// into Firebolt::Actions::Intent. nlohmann stays hidden in this impl-layer header.
class JsonValue : public Firebolt::JSON::NL_Json_Basic<Intent>
{
public:
void fromJson(const nlohmann::json& json) override
{
value_ = {};
docs/openrpc/the-spec/firebolt-open-rpc.json:835
- The OpenRPC spec now documents Discovery.watchedV2 as returning a boolean, but the C++ API returns Result (see include/firebolt/discovery.h:65-67). This mismatch can cause schema validation failures and makes the spec misleading for consumers of this repo.
"name": "result",
"summary": "Whether the platform accepted the watched notification",
"schema": {
"type": "boolean"
}
docs/openrpc/the-spec/firebolt-open-rpc.json:861
- Discovery.watchedV2 example result is
true, but the C++ client API treats watchedV2 as Result. The example should usenullto match the client’s invoke-style contract and avoid schema/fixture mismatches.
This issue also appears on line 887 of the same file.
"result": {
"name": "result",
"value": true
}
| "value": { | ||
| "userMemoryUsed": 126418944, | ||
| "userMemoryLimit": 807948288, | ||
| "gpuMemoryUsed": 353974272, | ||
| "gpuMemoryLimit": 922863616 | ||
| "userMemoryUsedKiB": 123456, | ||
| "userMemoryLimitKiB": 789012, | ||
| "gpuMemoryUsedKiB": 345678, | ||
| "gpuMemoryLimitKiB": 901234 |
| } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Localization.timeZone", | ||
| "tags": [ | ||
| { | ||
| "name": "property:readonly" | ||
| }, | ||
| { | ||
| "name": "capabilities", | ||
| "x-uses": [ | ||
| "xrn:firebolt:capability:localization:time-zone" | ||
| ] | ||
| } | ||
| ], | ||
| "summary": "Get the IANA timezone of the device.", | ||
| "params": [], | ||
| "result": { | ||
| "name": "timeZone", | ||
| "summary": "The device timezone.", | ||
| "schema": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "examples": [ | ||
| { | ||
| "name": "Default example", | ||
| "params": [], | ||
| "result": { | ||
| "name": "Default Result", | ||
| "value": "America/New_York" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Metrics.ready", | ||
| "tags": [ |
| } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Device.dolbyAtmosExperienceAvailable", | ||
| "summary": "Returns whether Dolby Atmos experience is available on the device", | ||
| "params": [], | ||
| "tags": [ | ||
| { | ||
| "name": "property:readonly" | ||
| }, | ||
| { | ||
| "name": "capabilities", | ||
| "x-uses": [ | ||
| "xrn:firebolt:capability:device:info" | ||
| ] | ||
| } | ||
| ], | ||
| "result": { | ||
| "name": "dolbyAtmosExperienceAvailable", | ||
| "summary": "Whether Dolby Atmos experience is available on the device", | ||
| "schema": { | ||
| "type": "boolean" | ||
| } | ||
| }, | ||
| "examples": [ | ||
| { | ||
| "name": "Getting Dolby Atmos experience availability", | ||
| "params": [], | ||
| "result": { | ||
| "name": "Default Result", | ||
| "value": true | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "Discovery.watched", | ||
| "summary": "Notify the platform that content was partially or completely watched", |
No description provided.