From abc0e58bcdb397e4cbedbebe4a5667ccb3a56097 Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 08:44:01 -0700 Subject: [PATCH 01/11] fix: actions.intent no args/correct return type --- CHANGELOG.md | 2 +- docs/openrpc/the-spec/firebolt-open-rpc.json | 78 ++++++++++ include/firebolt/actions.h | 2 +- run-component-tests-docker.sh | 151 +++++++++++++++++++ src/actions_impl.cpp | 6 +- src/actions_impl.h | 2 +- test/component/actionsGeneratedTest.cpp | 7 +- test/unit/actionsGeneratedTest.cpp | 9 +- test/unit/actionsTest.cpp | 13 +- 9 files changed, 247 insertions(+), 23 deletions(-) create mode 100755 run-component-tests-docker.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 799eb4e..58e532d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Added - New APIs - - `Actions.intent` + - `Actions.intent` (no parameters, returns string) - `Actions.onIntent` event ### Changed diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index 82b8600..89ae940 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -5,6 +5,7 @@ "version": "", "x-module-descriptions": { "Accessibility": "The `Accessibility` module provides access to the user/device settings for closed captioning and voice guidance.\n\nApps **SHOULD** attempt o respect these settings, rather than manage and persist seprate settings, which would be different per-app.", + "Actions": "Methods for setting and observing app intents.", "Advertising": "A module for platform provided advertising settings and functionality.", "Device": "A module for querying about the device and it's capabilities.", "Discovery": "Your App likely wants to integrate with the Platform's discovery capabilities. For example to add a \"Watch Next\" tile that links to your app from the platform's home screen.\n\nGetting access to this information requires to connect to lower level APIs made available by the platform. Since implementations differ between operators and platforms, the Firebolt SDK offers a Discovery module, that exposes a generic, agnostic interface to the developer.\n\nUnder the hood, an underlaying transport layer will then take care of calling the right APIs for the actual platform implementation that your App is running on.\n\nThe Discovery plugin is used to _send_ information to the Platform.\n\n### Localization\nApps should provide all user-facing strings in the device's language, as specified by the Firebolt `Localization.language` property.\n\nApps should provide prices in the same currency presented in the app. If multiple currencies are supported in the app, the app should provide prices in the user's current default currency.", @@ -48,6 +49,83 @@ } ] }, + { + "name": "Actions.intent", + "summary": "Returns the current intent.", + "tags": [ + { + "name": "capabilities", + "x-uses": [ + "xrn:firebolt:capability:actions:intent" + ] + } + ], + "params": [ + ], + "result": { + "name": "intent", + "summary": "The current intent.", + "schema": { + "type": "string" + } + }, + "examples": [ + { + "name": "Get the current intent", + "result": { + "name": "Default Result", + "value": "launch" + } + } + ] + }, + { + "name": "Actions.onIntent", + "tags": [ + { + "name": "event", + "x-notifier": "Actions.onIntent", + "x-subscriber-for": "Actions.intent" + }, + { + "name": "capabilities", + "x-uses": [ + "xrn:firebolt:capability:actions:intent" + ] + } + ], + "summary": "Notifies when the current intent changes.", + "params": [ + { + "name": "listen", + "schema": { + "type": "boolean" + } + } + ], + "result": { + "name": "intent", + "summary": "The current intent.", + "schema": { + "type": "string" + } + }, + "examples": [ + { + "name": "Listen for intent changes", + "params": [ + { + "name": "listen", + "value": true + } + ], + "result": { + "name": "Default Result", + "value": "launch" + } + } + ] + }, { "name": "Accessibility.audioDescription", "summary": "Returns the audio description setting of the device", diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index 9c39198..660abd6 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -38,7 +38,7 @@ class IActions public: virtual ~IActions() = default; - virtual Result intent(const std::string& intent) const = 0; + virtual Result intent() const = 0; virtual Result subscribeOnIntent(std::function&& notification) = 0; virtual Result subscribeOnIntentChanged(std::function&& notification) diff --git a/run-component-tests-docker.sh b/run-component-tests-docker.sh new file mode 100755 index 0000000..aaa13e7 --- /dev/null +++ b/run-component-tests-docker.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +PROTOCOL="rpc_v2" +MOCK_DIR="/tmp/mock-firebolt-firebolt-cpp-client" +IMAGE_TAG="firebolt-client-ci:local" +SKIP_IMAGE_BUILD="false" + +usage() { + cat < RPC protocol (default: rpc_v2) + --mock-dir Host directory for mock-firebolt cache + (default: /tmp/mock-firebolt-firebolt-cpp-client) + --image-tag Docker image tag to use/build + (default: firebolt-client-ci:local) + --skip-image-build Reuse existing image tag; do not run docker build + --help Show this help + +Examples: + ./run-component-tests-docker.sh + ./run-component-tests-docker.sh --protocol legacy + ./run-component-tests-docker.sh --skip-image-build --image-tag firebolt-client-ci:local +EOF +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --protocol) + [[ $# -ge 2 ]] || { echo "Missing value for --protocol" >&2; exit 1; } + PROTOCOL="$2" + shift + ;; + --mock-dir) + [[ $# -ge 2 ]] || { echo "Missing value for --mock-dir" >&2; exit 1; } + MOCK_DIR="$2" + shift + ;; + --image-tag) + [[ $# -ge 2 ]] || { echo "Missing value for --image-tag" >&2; exit 1; } + IMAGE_TAG="$2" + shift + ;; + --skip-image-build) + SKIP_IMAGE_BUILD="true" + ;; + --help|-h) + usage + exit 0 + ;; + *) + echo "Unknown option: $1" >&2 + usage + exit 1 + ;; + esac + shift +done + +if [[ "$PROTOCOL" != "rpc_v2" && "$PROTOCOL" != "legacy" ]]; then + echo "Invalid --protocol '$PROTOCOL'. Expected 'rpc_v2' or 'legacy'." >&2 + exit 1 +fi + +cd "$ROOT_DIR" + +if [[ ! -f .transport.version ]]; then + echo "Missing .transport.version in $ROOT_DIR" >&2 + exit 1 +fi + +TRANSPORT_VERSION="$(cat .transport.version)" +MOCK_SHA1SUM="1fec7b75190e75ac8ea8ebf9f3e00c0a070b2566" +MOCK_BRANCH="topic/changes-for-bidirectional" +NODE_VERSION="24.11.0" + +mkdir -p "$MOCK_DIR" + +echo "[1/4] Docker image: $IMAGE_TAG" +if [[ "$SKIP_IMAGE_BUILD" == "false" ]]; then + docker build \ + -f "$ROOT_DIR/.github/Dockerfile" \ + -t "$IMAGE_TAG" \ + --build-arg "DEPS_TRANSPORT_V=$TRANSPORT_VERSION" \ + --build-arg "DEPS_TRANSPORT_PROTOCOL=$PROTOCOL" \ + "$ROOT_DIR" +else + echo "Skipping image build (--skip-image-build)" +fi + +echo "[2/4] Preparing mock-firebolt in $MOCK_DIR" +docker run --rm --user "$(id -u):$(id -g)" \ + -v "$ROOT_DIR:/workspace" \ + -v "$MOCK_DIR:/mock-host" \ + "$IMAGE_TAG" \ + bash -c ' + set -e + if [ ! -d /mock-host/.git ]; then + git clone --depth 1 --branch '"$MOCK_BRANCH"' \ + https://github.com/rdkcentral/mock-firebolt.git /mock-host + fi + cd /mock-host + git fetch --shallow-since=2026-01-01 + git -c advice.detachedHead=false checkout '"$MOCK_SHA1SUM"' + source /usr/local/nvm/nvm.sh + nvm use --delete-prefix '"$NODE_VERSION"' >/dev/null + cd server + npm ci + ' + +BUILD_SUBDIR="build-docker" + +echo "[3/4] Building ctApp (build dir: $BUILD_SUBDIR)" +docker run --rm --user "$(id -u):$(id -g)" \ + -v "$ROOT_DIR:/workspace" \ + "$IMAGE_TAG" \ + bash -c ' + set -e + cd /workspace + rm -rf '"$BUILD_SUBDIR"' + cmake -B '"$BUILD_SUBDIR"' -S . -DCMAKE_BUILD_TYPE=Debug -DENABLE_TESTS=ON + cp docs/openrpc/the-spec/firebolt-open-rpc.json '"$BUILD_SUBDIR"'/test/ + cmake --build '"$BUILD_SUBDIR"' --parallel + chmod +x '"$BUILD_SUBDIR"'/test/ctApp + ' + +echo "[4/4] Running component tests" +docker run --rm --user "$(id -u):$(id -g)" \ + -v "$ROOT_DIR:/workspace" \ + -v "$MOCK_DIR:/mock" \ + "$IMAGE_TAG" \ + /workspace/.github/scripts/run-component-tests.sh \ + --mock /mock \ + --protocol "$PROTOCOL" \ + --config /workspace/.github/mock-firebolt/config.json \ + --openrpc /workspace/docs/openrpc/the-spec/firebolt-open-rpc.json \ + --app-openrpc /workspace/docs/openrpc/the-spec/firebolt-app-open-rpc.json \ + --test-exe /workspace/"$BUILD_SUBDIR"/test/ctApp + +echo "Done." diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index 431496f..61597de 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -33,11 +33,9 @@ ActionsImpl::ActionsImpl(Firebolt::Helpers::IHelper& helper) { } -Result ActionsImpl::intent(const std::string& intent) const +Result ActionsImpl::intent() const { - nlohmann::json params; - params["intent"] = intent; - return helper_.invoke("Actions.intent", params); + return helper_.get("Actions.intent"); } Result ActionsImpl::subscribeOnIntent(std::function&& notification) diff --git a/src/actions_impl.h b/src/actions_impl.h index e7eaf61..4c8d6ba 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -36,7 +36,7 @@ class ActionsImpl : public IActions ActionsImpl& operator=(const ActionsImpl&) = delete; ~ActionsImpl() override = default; - Result intent(const std::string& intent) const override; + Result intent() const override; Result subscribeOnIntent(std::function&& notification) override; diff --git a/test/component/actionsGeneratedTest.cpp b/test/component/actionsGeneratedTest.cpp index 6ce5e5d..ff1d53e 100644 --- a/test/component/actionsGeneratedTest.cpp +++ b/test/component/actionsGeneratedTest.cpp @@ -32,8 +32,9 @@ class ActionsGeneratedCTest : public ::testing::Test TEST_F(ActionsGeneratedCTest, Intent) { - auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent("launch"); - EXPECT_TRUE(result) << toError(result); + auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); + ASSERT_TRUE(result) << toError(result); + EXPECT_EQ(*result, "launch"); } TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) @@ -52,7 +53,7 @@ TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) ASSERT_TRUE(id) << toError(id); verifyEventSubscription(id); - triggerEvent("Actions.onIntent", R"({"value":"launch"})"); + triggerEvent("Actions.onIntent", R"("launch")"); verifyEventReceived(mtx, cv, eventReceived); auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id.value()); diff --git a/test/unit/actionsGeneratedTest.cpp b/test/unit/actionsGeneratedTest.cpp index 86006f3..cd024d6 100644 --- a/test/unit/actionsGeneratedTest.cpp +++ b/test/unit/actionsGeneratedTest.cpp @@ -42,15 +42,14 @@ TEST_F(ActionsGeneratedUTest, UnsubscribeForwardsToHelper) TEST_F(ActionsGeneratedUTest, ForwardsintentTransportErrors) { - EXPECT_CALL(mockHelper, invoke("Actions.intent", ::testing::_)) + EXPECT_CALL(mockHelper, getJson("Actions.intent", ::testing::_)) .WillOnce(::testing::Invoke( [](const std::string& /*method*/, const nlohmann::json& params) { - EXPECT_TRUE(params.is_object()) << "Expected params object for method call"; - EXPECT_TRUE(params.contains("intent")) << "Missing expected param key: intent"; - return Firebolt::Result{Firebolt::Error::General}; + EXPECT_TRUE(params.is_null() || params.empty()) << "Expected no params for method call"; + return Firebolt::Result{Firebolt::Error::General}; })); - auto result = impl.intent({}); + auto result = impl.intent(); EXPECT_FALSE(result) << "Expected error propagation when helper invoke fails"; } diff --git a/test/unit/actionsTest.cpp b/test/unit/actionsTest.cpp index 7295886..1b87480 100644 --- a/test/unit/actionsTest.cpp +++ b/test/unit/actionsTest.cpp @@ -28,14 +28,11 @@ class ActionsUTest : public ::testing::Test, protected MockBase TEST_F(ActionsUTest, Start) { - nlohmann::json expectedParams; - expectedParams["intent"] = "launch"; - EXPECT_CALL(mockHelper, invoke("Actions.intent", expectedParams)) - .WillOnce(Invoke([&](const std::string& /*methodName*/, const nlohmann::json& /*parameters*/) - { return Firebolt::Result{Firebolt::Error::None}; })); - - auto result = actionsImpl_.intent("launch"); - EXPECT_TRUE(result); + mock_with_response("Actions.intent", "launch"); + + auto result = actionsImpl_.intent(); + ASSERT_TRUE(result) << "ActionsImpl::intent() returned an error"; + EXPECT_EQ(*result, "launch"); } TEST_F(ActionsUTest, SubscribeOnIntent) From e80002249d20be761fbbd95d71b00c48cc018990 Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 09:17:06 -0700 Subject: [PATCH 02/11] feat: use sdk gen to gen fix --- include/firebolt/actions.h | 14 +++--- ...-docker.sh => run-component-tests-local.sh | 0 src/actions_impl.cpp | 23 ++++------ src/actions_impl.h | 8 +--- src/json_types/actions.h | 12 +++-- test/component/actionsGeneratedTest.cpp | 44 +++---------------- test/unit/actionsGeneratedTest.cpp | 18 ++++---- 7 files changed, 36 insertions(+), 83 deletions(-) rename run-component-tests-docker.sh => run-component-tests-local.sh (100%) diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index 660abd6..23bb270 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -22,34 +22,30 @@ #ifndef FIREBOLT_ACTIONS_H #define FIREBOLT_ACTIONS_H -#include #include #include #include #include -#include #include +#include -namespace Firebolt::Actions -{ +namespace Firebolt::Actions { -class IActions -{ +class IActions { public: virtual ~IActions() = default; virtual Result intent() const = 0; virtual Result subscribeOnIntent(std::function&& notification) = 0; - virtual Result subscribeOnIntentChanged(std::function&& notification) - { + virtual Result subscribeOnIntentChanged(std::function&& notification) { return subscribeOnIntent(std::move(notification)); } virtual Result unsubscribe(SubscriptionId id) = 0; virtual void unsubscribeAll() = 0; -}; // class IActions +}; // class IActions } // namespace Firebolt::Actions diff --git a/run-component-tests-docker.sh b/run-component-tests-local.sh similarity index 100% rename from run-component-tests-docker.sh rename to run-component-tests-local.sh diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index 61597de..01dca17 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -23,33 +23,28 @@ #include "json_types/actions.h" #include #include +#include -namespace Firebolt::Actions -{ +namespace Firebolt::Actions { ActionsImpl::ActionsImpl(Firebolt::Helpers::IHelper& helper) - : helper_(helper), - subscriptionManager_(helper, this) -{ -} + : helper_(helper) + , subscriptionManager_(helper, this) +{} -Result ActionsImpl::intent() const -{ +Result ActionsImpl::intent() const { return helper_.get("Actions.intent"); } -Result ActionsImpl::subscribeOnIntent(std::function&& notification) -{ +Result ActionsImpl::subscribeOnIntent(std::function&& notification) { return subscriptionManager_.subscribe("Actions.onIntent", std::move(notification)); } -Result ActionsImpl::unsubscribe(SubscriptionId id) -{ +Result ActionsImpl::unsubscribe(SubscriptionId id) { return subscriptionManager_.unsubscribe(id); } -void ActionsImpl::unsubscribeAll() -{ +void ActionsImpl::unsubscribeAll() { subscriptionManager_.unsubscribeAll(); } diff --git a/src/actions_impl.h b/src/actions_impl.h index 4c8d6ba..8653882 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -25,15 +25,11 @@ #include "firebolt/actions.h" #include -namespace Firebolt::Actions -{ +namespace Firebolt::Actions { -class ActionsImpl : public IActions -{ +class ActionsImpl : public IActions { public: explicit ActionsImpl(Firebolt::Helpers::IHelper& helper); - ActionsImpl(const ActionsImpl&) = delete; - ActionsImpl& operator=(const ActionsImpl&) = delete; ~ActionsImpl() override = default; Result intent() const override; diff --git a/src/json_types/actions.h b/src/json_types/actions.h index 60c76ce..6025350 100644 --- a/src/json_types/actions.h +++ b/src/json_types/actions.h @@ -23,16 +23,14 @@ #define FIREBOLT_ACTIONS_JSON_H #pragma once -#include "firebolt/actions.h" -#include -#include #include +#include +#include +#include "firebolt/actions.h" -namespace Firebolt::Actions -{ +namespace Firebolt::Actions { -namespace JsonData -{ +namespace JsonData { } // namespace JsonData diff --git a/test/component/actionsGeneratedTest.cpp b/test/component/actionsGeneratedTest.cpp index ff1d53e..a08372b 100644 --- a/test/component/actionsGeneratedTest.cpp +++ b/test/component/actionsGeneratedTest.cpp @@ -16,46 +16,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "firebolt/firebolt.h" -#include "utils.h" +#include "firebolt/actions.h" #include -class ActionsGeneratedCTest : public ::testing::Test +TEST(ActionsGeneratedCTest, InterfaceSurfaceHasintent) { -protected: - void SetUp() override { eventReceived = false; } - - std::condition_variable cv; - std::mutex mtx; - bool eventReceived; -}; - -TEST_F(ActionsGeneratedCTest, Intent) -{ - auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); - ASSERT_TRUE(result) << toError(result); - EXPECT_EQ(*result, "launch"); + using Interface = Firebolt::Actions::IActions; + auto ptr = &Interface::intent; + (void)ptr; + SUCCEED(); } -TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) -{ - auto id = Firebolt::IFireboltAccessor::Instance().ActionsInterface().subscribeOnIntent( - [&](const std::string& intent) - { - EXPECT_EQ(intent, "launch"); - { - std::lock_guard lock(mtx); - eventReceived = true; - } - cv.notify_one(); - }); - - ASSERT_TRUE(id) << toError(id); - verifyEventSubscription(id); - - triggerEvent("Actions.onIntent", R"("launch")"); - verifyEventReceived(mtx, cv, eventReceived); - - auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id.value()); - verifyUnsubscribeResult(result); -} diff --git a/test/unit/actionsGeneratedTest.cpp b/test/unit/actionsGeneratedTest.cpp index cd024d6..f1e0129 100644 --- a/test/unit/actionsGeneratedTest.cpp +++ b/test/unit/actionsGeneratedTest.cpp @@ -16,8 +16,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "actions_impl.h" #include "mock_helper.h" +#include "actions_impl.h" #include class ActionsGeneratedUTest : public ::testing::Test @@ -34,22 +34,22 @@ TEST_F(ActionsGeneratedUTest, Constructs) TEST_F(ActionsGeneratedUTest, UnsubscribeForwardsToHelper) { - EXPECT_CALL(mockHelper, unsubscribe(7)).WillOnce(::testing::Return(Firebolt::Result{Firebolt::Error::None})); + EXPECT_CALL(mockHelper, unsubscribe(7)) + .WillOnce(::testing::Return(Firebolt::Result{Firebolt::Error::None})); auto result = impl.unsubscribe(7); ASSERT_TRUE(result) << "unsubscribe should return success when helper succeeds"; } + TEST_F(ActionsGeneratedUTest, ForwardsintentTransportErrors) { EXPECT_CALL(mockHelper, getJson("Actions.intent", ::testing::_)) - .WillOnce(::testing::Invoke( - [](const std::string& /*method*/, const nlohmann::json& params) - { - EXPECT_TRUE(params.is_null() || params.empty()) << "Expected no params for method call"; - return Firebolt::Result{Firebolt::Error::General}; - })); + .WillOnce(::testing::Invoke([](const std::string& /*method*/, const nlohmann::json& /*params*/) { + return Firebolt::Result{Firebolt::Error::General}; + })); auto result = impl.intent(); - EXPECT_FALSE(result) << "Expected error propagation when helper invoke fails"; + EXPECT_FALSE(result) << "Expected error propagation when helper getJson fails"; } + From 08384df5a39a113b1930b0eb23a8b556b6c8cf4b Mon Sep 17 00:00:00 2001 From: Brendan O'Bra Date: Tue, 2 Jun 2026 10:00:10 -0700 Subject: [PATCH 03/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- run-component-tests-local.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/run-component-tests-local.sh b/run-component-tests-local.sh index aaa13e7..9a0d6d1 100755 --- a/run-component-tests-local.sh +++ b/run-component-tests-local.sh @@ -11,7 +11,7 @@ SKIP_IMAGE_BUILD="false" usage() { cat < Date: Tue, 2 Jun 2026 10:06:43 -0700 Subject: [PATCH 04/11] fix: delete copy constructor added --- src/actions_impl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/actions_impl.h b/src/actions_impl.h index 8653882..229b60d 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -30,6 +30,8 @@ namespace Firebolt::Actions { class ActionsImpl : public IActions { public: explicit ActionsImpl(Firebolt::Helpers::IHelper& helper); + ActionsImpl(const ActionsImpl&) = delete; + ActionsImpl& operator=(const ActionsImpl&) = delete; ~ActionsImpl() override = default; Result intent() const override; From 684a7043b3b56c85afed31d046a0488c6ad22848 Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 10:14:12 -0700 Subject: [PATCH 05/11] fix(actions): address include review comments --- include/firebolt/actions.h | 1 + src/actions_impl.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index 23bb270..f76efbd 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index 01dca17..ced5bef 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -23,7 +23,6 @@ #include "json_types/actions.h" #include #include -#include namespace Firebolt::Actions { From 53bf62b4d7c20420f7f7d09feaef0175eca0d9cd Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 10:16:46 -0700 Subject: [PATCH 06/11] fix(actions): address remaining review feedback --- docs/openrpc/the-spec/firebolt-open-rpc.json | 5 ++- test/component/actionsGeneratedTest.cpp | 47 +++++++++++++++++--- test/unit/actionsGeneratedTest.cpp | 2 +- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index 89ae940..b66c62f 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -5,7 +5,7 @@ "version": "", "x-module-descriptions": { "Accessibility": "The `Accessibility` module provides access to the user/device settings for closed captioning and voice guidance.\n\nApps **SHOULD** attempt o respect these settings, rather than manage and persist seprate settings, which would be different per-app.", - "Actions": "Methods for setting and observing app intents.", + "Actions": "Methods for getting and observing app intents.", "Advertising": "A module for platform provided advertising settings and functionality.", "Device": "A module for querying about the device and it's capabilities.", "Discovery": "Your App likely wants to integrate with the Platform's discovery capabilities. For example to add a \"Watch Next\" tile that links to your app from the platform's home screen.\n\nGetting access to this information requires to connect to lower level APIs made available by the platform. Since implementations differ between operators and platforms, the Firebolt SDK offers a Discovery module, that exposes a generic, agnostic interface to the developer.\n\nUnder the hood, an underlaying transport layer will then take care of calling the right APIs for the actual platform implementation that your App is running on.\n\nThe Discovery plugin is used to _send_ information to the Platform.\n\n### Localization\nApps should provide all user-facing strings in the device's language, as specified by the Firebolt `Localization.language` property.\n\nApps should provide prices in the same currency presented in the app. If multiple currencies are supported in the app, the app should provide prices in the user's current default currency.", @@ -25,6 +25,9 @@ "summary": "The OpenRPC schema for this JSON-RPC API", "params": [], "tags": [ + { + "name": "property:readonly" + }, { "name": "capabilities", "x-uses": [ diff --git a/test/component/actionsGeneratedTest.cpp b/test/component/actionsGeneratedTest.cpp index a08372b..e14323e 100644 --- a/test/component/actionsGeneratedTest.cpp +++ b/test/component/actionsGeneratedTest.cpp @@ -16,14 +16,49 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "firebolt/actions.h" +#include "firebolt/firebolt.h" +#include "utils.h" +#include #include +#include -TEST(ActionsGeneratedCTest, InterfaceSurfaceHasintent) +class ActionsGeneratedCTest : public ::testing::Test { - using Interface = Firebolt::Actions::IActions; - auto ptr = &Interface::intent; - (void)ptr; - SUCCEED(); +protected: + void SetUp() override { eventReceived = false; } + + std::condition_variable cv; + std::mutex mtx; + bool eventReceived; +}; + +TEST_F(ActionsGeneratedCTest, Intent) +{ + auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); + ASSERT_TRUE(result) << toError(result); + EXPECT_EQ(*result, "launch"); +} + +TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) +{ + auto id = Firebolt::IFireboltAccessor::Instance().ActionsInterface().subscribeOnIntent( + [&](const std::string& intent) + { + EXPECT_EQ(intent, "launch"); + { + std::lock_guard lock(mtx); + eventReceived = true; + } + cv.notify_one(); + }); + + ASSERT_TRUE(id) << toError(id); + verifyEventSubscription(id); + + triggerEvent("Actions.onIntent", R"("launch")"); + verifyEventReceived(mtx, cv, eventReceived); + + auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id.value()); + verifyUnsubscribeResult(result); } diff --git a/test/unit/actionsGeneratedTest.cpp b/test/unit/actionsGeneratedTest.cpp index f1e0129..0a19b4f 100644 --- a/test/unit/actionsGeneratedTest.cpp +++ b/test/unit/actionsGeneratedTest.cpp @@ -42,7 +42,7 @@ TEST_F(ActionsGeneratedUTest, UnsubscribeForwardsToHelper) } -TEST_F(ActionsGeneratedUTest, ForwardsintentTransportErrors) +TEST_F(ActionsGeneratedUTest, ForwardsIntentTransportErrors) { EXPECT_CALL(mockHelper, getJson("Actions.intent", ::testing::_)) .WillOnce(::testing::Invoke([](const std::string& /*method*/, const nlohmann::json& /*params*/) { From 020afc90e5851b870c8a28d2fd2b1c353ed2a30d Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 10:19:44 -0700 Subject: [PATCH 07/11] docs: add copilot instructions for cpp client conventions --- .github/copilot-instructions.md | 84 +++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..e80616f --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,84 @@ +# firebolt-cpp-client Copilot Instructions + +Scope: This file applies to the firebolt-cpp-client repository. + +## Primary goals + +- Preserve API contract correctness across interface, implementation, tests, and OpenRPC fixtures. +- Keep generated surfaces and bespoke conventions aligned. +- Prefer minimal, targeted changes. + +## High-signal workflow + +1. For API-facing changes, update all of the following in one pass: + - `include/firebolt/*.h` + - `src/*_impl.h` and `src/*_impl.cpp` + - `test/unit/*Test.cpp` and `test/component/*Test.cpp` + - `docs/openrpc/the-spec/firebolt-open-rpc.json` when component tests depend on fixture shape. +2. Run component tests after edits. +3. If behavior is generator-owned, patch generator code in sibling repo and regenerate module artifacts. + +## Test commands + +- Local one-shot (current preferred): + - `./run-component-tests-local.sh` + - `./run-component-tests-local.sh --skip-image-build` +- Legacy wrappers may still exist in conversation history; prefer the local script in this repo. +- Unit-only: + - `./run-unit-tests.sh` + +## Actions module rules (important) + +- `Actions.intent` is getter-only: + - takes no parameters + - returns `Result` +- `Actions.onIntent` callback payload is a string value. +- Component event trigger for `Actions.onIntent` should use a string JSON payload (for example `"launch"`), not an object. + +## Generated-code conventions that must be preserved + +- `*Impl` classes should delete copy constructor and copy assignment: + - `ClassName(const ClassName&) = delete;` + - `ClassName& operator=(const ClassName&) = delete;` +- Unless explicitly justified as safe, `*Impl` classes should also delete move operations: + - `ClassName(ClassName&&) = delete;` + - `ClassName& operator=(ClassName&&) = delete;` +- Keep include hygiene strict: + - include `` when using `std::move` + - remove unused includes such as `` when not used +- Keep test names in consistent CamelCase for filtering. + +## Component test expectations + +- Red schema validation lines in logs can be expected for negative-path tests. +- Negative tests must still verify runtime behavior (callbacks not delivered for invalid payloads), not just compile-time surface checks. + +## OpenRPC fixture expectations + +- Module descriptions must match actual API behavior. +- Getter-style methods should carry property tags consistent with the rest of the file (for example `property:readonly` where applicable). +- Keep notifier/subscriber metadata aligned (`x-notifier`, `x-subscriber-for`). + +## Regeneration notes (sibling repo) + +When a change is generator-owned, use `firebolt-sdk-gen` and apply module-scoped output back into this repo. + +Typical flow: + +- From `../firebolt-sdk-gen`: + - `./sync-plan-checklist.sh --profile core --module actions --apply --no-accessor-touchpoints --target-root ../firebolt-cpp-client` + +This keeps migration incremental and avoids unrelated accessor touchpoint churn. + +## CI parity reminders + +- CI uses Dockerized build/test flow and mock-firebolt integration. +- Keep changes compatible with: + - `.github/workflows/ci.yml` + - `.github/scripts/run-component-tests.sh` + +## PR hygiene + +- If a review asks for include-file fixes, prefer precise header/source edits and re-run component tests. +- Do not relax negative tests just to suppress red validation logs. +- Keep commit messages scoped and explicit (example: `fix(actions): address include review comments`). From 74c8f2b104c2d4e85be30ae4741627f36d4e008b Mon Sep 17 00:00:00 2001 From: Brendan O'Bra Date: Tue, 2 Jun 2026 10:34:07 -0700 Subject: [PATCH 08/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- docs/openrpc/the-spec/firebolt-open-rpc.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index b66c62f..4600d5d 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -56,6 +56,9 @@ "name": "Actions.intent", "summary": "Returns the current intent.", "tags": [ + { + "name": "property:readonly" + }, { "name": "capabilities", "x-uses": [ @@ -63,8 +66,7 @@ ] } ], - "params": [ - ], + "params": [], "result": { "name": "intent", "summary": "The current intent.", From 45d1ffce78f2c8dbe1342ea2a6dc933d81bc7278 Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 10:37:31 -0700 Subject: [PATCH 09/11] fix(actions): finalize PR review follow-ups --- docs/openrpc/the-spec/firebolt-open-rpc.json | 3 --- test/unit/actionsGeneratedTest.cpp | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index 4600d5d..9ad9084 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -25,9 +25,6 @@ "summary": "The OpenRPC schema for this JSON-RPC API", "params": [], "tags": [ - { - "name": "property:readonly" - }, { "name": "capabilities", "x-uses": [ diff --git a/test/unit/actionsGeneratedTest.cpp b/test/unit/actionsGeneratedTest.cpp index 0a19b4f..51b3ee3 100644 --- a/test/unit/actionsGeneratedTest.cpp +++ b/test/unit/actionsGeneratedTest.cpp @@ -45,7 +45,9 @@ TEST_F(ActionsGeneratedUTest, UnsubscribeForwardsToHelper) TEST_F(ActionsGeneratedUTest, ForwardsIntentTransportErrors) { EXPECT_CALL(mockHelper, getJson("Actions.intent", ::testing::_)) - .WillOnce(::testing::Invoke([](const std::string& /*method*/, const nlohmann::json& /*params*/) { + .WillOnce(::testing::Invoke([](const std::string& /*method*/, const nlohmann::json& params) { + EXPECT_TRUE(params.is_null() || (params.is_object() && params.empty())) + << "Actions.intent getter should not send request params"; return Firebolt::Result{Firebolt::Error::General}; })); From e98cce604f57c3c4ec6cf1905ab40e0ed0c1286f Mon Sep 17 00:00:00 2001 From: bobra200 Date: Tue, 2 Jun 2026 10:42:08 -0700 Subject: [PATCH 10/11] fix(actions): drop unused nlohmann include --- src/actions_impl.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index ced5bef..aba7b5b 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -22,7 +22,6 @@ #include "actions_impl.h" #include "json_types/actions.h" #include -#include namespace Firebolt::Actions { From f2fc7a3903ddc3d88b909af515f9f71aac008bb0 Mon Sep 17 00:00:00 2001 From: Brendan O'Bra Date: Tue, 2 Jun 2026 10:59:50 -0700 Subject: [PATCH 11/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- run-component-tests-local.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-component-tests-local.sh b/run-component-tests-local.sh index 9a0d6d1..1752276 100755 --- a/run-component-tests-local.sh +++ b/run-component-tests-local.sh @@ -111,7 +111,7 @@ docker run --rm --user "$(id -u):$(id -g)" \ https://github.com/rdkcentral/mock-firebolt.git /mock-host fi cd /mock-host - git fetch --shallow-since=2026-01-01 + git fetch --depth 1 origin '"$MOCK_SHA1SUM"' git -c advice.detachedHead=false checkout '"$MOCK_SHA1SUM"' source /usr/local/nvm/nvm.sh nvm use --delete-prefix '"$NODE_VERSION"' >/dev/null