RDKEMW-14869: VideoOutput implementation + fix broken unit tests

[brendanobra]

RDKEMW-21295: Add VideoOutput module to Firebolt C++ Client

Summary

Adds the VideoOutput module to the C++ SDK, providing client-side support for all VideoOutput APIs defined in the Firebolt 9 specification (methods 80–88).

What's included

Generated module files (via fb-gen --emit sync-plan-cpp):

• include/firebolt/videooutput.h — interface, enums, structs
• src/videooutput_impl.h / src/videooutput_impl.cpp — JSON-RPC implementation
• src/json_types/videooutput.h — nlohmann_json serialization
• test/unit/videooutputGeneratedTest.cpp — unit tests (4 cases, all passing)
• test/component/videooutputGeneratedTest.cpp — component test stubs

Accessor wiring (manual):

• include/firebolt/firebolt.h — added #include + VideooutputInterface() virtual method
• src/firebolt.cpp — added impl include, initializer, interface override, unsubscribeAll call, member field

Pre-existing test fixture fixes (unrelated to VideoOutput, fixed broken tests on develop):

• docs/openrpc/the-spec/firebolt-open-rpc.json:
  • Added missing Device.dolbyAtmosExperienceAvailable + event method entry
  • Added missing Localization.timeZone + event method entry
  • Renamed *KiB fields → non-suffixed names in Stats.memoryUsage example to match C++ struct

APIs added

Method	Type	Subscribable
resolution	getter → {width, height}	✓ (onResolutionChanged)
hdcp	getter → enum {hdcp1.4, hdcp2.2, none, direct}	✓ (onHdcpChanged)
cecState	getter → enum {active, inactive, unsupported}	✓ (onCecStateChanged)
refreshRate	getter → enum {0, 23.976, 24, 25, 29.97, 30, 50, 59.94, 60}	✓ (onRefreshRateChanged)
colorDepth	getter → enum {0, 8, 10, 12}	—
colorFormat	getter → enum {ycbcr420, ycbcr422, ycbcr444, rgb444, none}	—
colorimetry	getter → enum {bt2020rgb, bt2020ycc, bt709, oprgb, none}	—
dynamicRange	getter → enum {hdr10, hdr10plus, dolbyVision, hlg, sdr, none}	—
quantizationRange	getter → enum {limited, full, none}	—

Testing

• Unit tests: 111/111 pass (including 4 new VideoOutput tests)
• Library builds clean: libFireboltClient.so links without errors
• Pre-existing test failures (3) fixed by OpenRPC fixture corrections

Spec source

Smithy IDL: firebolt-apis/src/smithy/videooutput.smithy (on feat/smithy branch, not part of this PR)

[Copilot]

Pull request overview

Adds a new (auto-generated) VideoOutput-related module implementation to the Firebolt C++ client, wires it into the IFireboltAccessor singleton, and includes some small formatting/linting changes.

Changes:

• Introduces the videooutput public interface, JSON adapters, and JSON-RPC implementation (include/, src/, src/json_types/).
• Wires the new interface into IFireboltAccessor and FireboltAccessorImpl (include/firebolt/firebolt.h, src/firebolt.cpp).
• Adds initial unit/component test stubs for the module and updates lint/clang-tidy configuration.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/unit/videooutputGeneratedTest.cpp	Adds basic generated unit tests (currently minimal coverage).
test/unit/actionsTest.cpp	Formatting-only adjustment to a test call site.
test/component/videooutputGeneratedTest.cpp	Adds generated component-test stubs (compile-time surface checks only).
test/api_test_app/apis/actionsDemo.cpp	Formatting-only output wrapping changes.
src/videooutput_impl.h	Adds VideooutputImpl class declaration and subscription plumbing.
src/videooutput_impl.cpp	Implements Videooutput JSON-RPC calls and subscriptions.
src/json_types/videooutput.h	Adds JSON (de)serialization for Videooutput enums/structs.
src/firebolt.cpp	Wires VideooutputImpl into the accessor singleton and unsubscribeAll flow.
lint.sh	Runs clang-tidy in parallel via run-clang-tidy when available.
include/firebolt/videooutput.h	Adds the new public Videooutput interface/types/method availability helpers.
include/firebolt/firebolt.h	Exposes VideooutputInterface() on IFireboltAccessor.
include/firebolt/actions.h	Formatting-only signature wrapping.
.clang-tidy	Adds clang-tidy configuration.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (6)

include/firebolt/videooutput.h:35

• The new public API uses the module/type spelling Videooutput (e.g., namespace Firebolt::Videooutput, IVideooutput). Existing multi-word modules use UpperCamelCase in the namespace/type names (e.g., namespace Firebolt::TextToSpeech in include/firebolt/texttospeech.h:27). Renaming/regenerating this module to VideoOutput (e.g., Firebolt::VideoOutput::IVideoOutput, VideoOutputImpl, VideoOutputInterface()) would keep the API consistent across modules and align with the PR description’s “VideoOutput”.

namespace Firebolt::Videooutput
{
enum class CecStateValue
{
    Active,

src/videooutput_impl.h:37

• VideooutputImpl deletes copy operations but not move operations. These impl classes hold a IHelper& reference and a SubscriptionManager, so implicit moves can lead to surprising aliasing or invalid subscription state; other modules typically treat impls as non-movable.

    explicit VideooutputImpl(Firebolt::Helpers::IHelper& helper);
    VideooutputImpl(const VideooutputImpl&) = delete;
    VideooutputImpl& operator=(const VideooutputImpl&) = delete;
    ~VideooutputImpl() override = default;

src/videooutput_impl.cpp:26

• <regex> is included but not used in this translation unit, which adds compile-time overhead and unnecessary dependencies.

#include <firebolt/json_types.h>
#include <nlohmann/json.hpp>
#include <regex>

test/component/videooutputGeneratedTest.cpp:36

• The component test only checks that the interface has methods via pointer-to-member, but does not validate any runtime behavior (getter values, subscription delivery, unsubscribe paths). This leaves the new VideoOutput/Videooutput implementation effectively untested at the component level.

TEST(VideooutputGeneratedCTest, InterfaceSurfaceHasresolution)
{
    using Interface = Firebolt::Videooutput::IVideooutput;
    auto ptr = &Interface::resolution;
    (void)ptr;
    SUCCEED();
}

TEST(VideooutputGeneratedCTest, InterfaceSurfaceHascolorDepth)
{
    using Interface = Firebolt::Videooutput::IVideooutput;
    auto ptr = &Interface::colorDepth;
    (void)ptr;
    SUCCEED();
}

include/firebolt/firebolt.h:174

• The new VideooutputInterface() accessor is missing the Doxygen-style comment block that the other interface accessors in this header have, which makes the public API docs inconsistent.

    virtual Videooutput::IVideooutput& VideooutputInterface() = 0;

src/json_types/videooutput.h:103

• The JsonData::*Enum maps are defined but never referenced (e.g., CecStateValueEnum is only defined here). In other modules, these maps are used by a small NL_Json_Basic<Enum> adapter (see DeviceClassJson in src/json_types/device.h:35–39). Either add the corresponding adapter classes and use them from the impl, or remove the unused maps to avoid dead/duplicated enum wiring.

inline const Firebolt::JSON::EnumType<::Firebolt::Videooutput::CecStateValue> CecStateValueEnum({
    {"ACTIVE", ::Firebolt::Videooutput::CecStateValue::Active},
    {"INACTIVE", ::Firebolt::Videooutput::CecStateValue::Inactive},
    {"UNSUPPORTED", ::Firebolt::Videooutput::CecStateValue::Unsupported},
});

[Copilot]

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (10)

test/unit/videooutputGeneratedTest.cpp:1

• The new test fixture and test case names use inconsistent casing (e.g., Videooutput vs VideoOutput, Forwardsresolution vs ForwardsResolution). Please rename them to match the module/type naming (VideoOutput) and standard CamelCase test naming for readability and consistency.
  test/unit/videooutputGeneratedTest.cpp:1
• The new test fixture and test case names use inconsistent casing (e.g., Videooutput vs VideoOutput, Forwardsresolution vs ForwardsResolution). Please rename them to match the module/type naming (VideoOutput) and standard CamelCase test naming for readability and consistency.
  test/unit/videooutputGeneratedTest.cpp:1
• The new test fixture and test case names use inconsistent casing (e.g., Videooutput vs VideoOutput, Forwardsresolution vs ForwardsResolution). Please rename them to match the module/type naming (VideoOutput) and standard CamelCase test naming for readability and consistency.
  test/component/videooutputGeneratedTest.cpp:1
• The test names use inconsistent casing (Videooutput, Hasresolution, HascolorDepth). Please align with existing naming conventions (e.g., VideoOutputGeneratedCTest and InterfaceSurfaceHasResolution / InterfaceSurfaceHasColorDepth) to keep generated tests easy to scan and search.
  test/component/videooutputGeneratedTest.cpp:1
• The test names use inconsistent casing (Videooutput, Hasresolution, HascolorDepth). Please align with existing naming conventions (e.g., VideoOutputGeneratedCTest and InterfaceSurfaceHasResolution / InterfaceSurfaceHasColorDepth) to keep generated tests easy to scan and search.
  src/videooutput_impl.cpp:1
• These includes appear unused in this translation unit (<regex> and <nlohmann/json.hpp>). Please remove unused/redundant includes to reduce compile time and keep dependencies minimal (the JSON header is already pulled in by json_types/videooutput.h).
  src/json_types/videooutput.h:1
• The thrown error message is too generic for debugging (it doesn't indicate which type/fields are missing). Consider including the object name and the required field list (or the specific missing fields) in the exception message so failures are actionable when surfaced from JSON parsing.
  src/videooutput_impl.h:1
• Only resolution(), colorDepth(), and unsubscribe() are covered by the new unit tests, but this PR introduces several additional getters and subscription APIs. Please add unit tests that validate forwarding/error-propagation for hdcp(), cecState(), refreshRate(), colorFormat(), colorimetry(), dynamicRange(), quantizationRange(), plus at least one subscription path to ensure the correct event method name is used.
  include/firebolt/firebolt.h:174
• The newly added VideoOutputInterface() accessor lacks a Doxygen comment block, while neighboring interface accessors are documented. Please add a brief @brief / @return comment for consistency and to keep the public header documentation complete.

    virtual Actions::IActions& ActionsInterface() = 0;

    virtual VideoOutput::IVideoOutput& VideoOutputInterface() = 0;

lint.sh:181

• Running run-clang-tidy with -j $(nproc) can overwhelm resource-constrained CI runners (high memory usage / contention), making lint flaky. Consider capping jobs (e.g., min(NPROC, total_files) and/or a conservative max) or allowing an environment variable override (e.g., CLANG_TIDY_JOBS) so CI can tune parallelism.

  NPROC=$(nproc 2>/dev/null || echo 4)

  if [[ "$APPLY_FIXES" == false ]] && command -v run-clang-tidy >/dev/null 2>&1; then
    echo "[lint][clang-tidy] Running ${total_files} files in parallel (${NPROC} jobs)"
    if ! run-clang-tidy -p "$BUILD_DIR" -j "$NPROC" "${source_files[@]}"; then

[Copilot]

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

src/json_types/videooutput.h:1

• The serialized wire values don’t match the Firebolt 9 spec described in the PR (e.g., dolbyVision, hdcp1.4, hdcp2.2) and also conflict with the JsonData::*Enum tables later in this file. Update these NLOHMANN_JSON_SERIALIZE_ENUM mappings to use the spec strings (and keep them consistent with the JsonData enum tables) to avoid failed (de)serialization at runtime.
  src/json_types/videooutput.h:1
• The JSON enum table contains a typo: rgbb444 does not match the spec/value used elsewhere (rgb444). This will cause parsing failures if this EnumType is used. Replace rgbb444 with rgb444.
  src/videooutput_impl.cpp:1
• <regex> is included but not used anywhere in this new translation unit as shown. Remove the unused include to reduce compile time and avoid suggesting regex usage where none exists.
  test/unit/videooutputGeneratedTest.cpp:1
• The test fixture name uses Videooutput (lowercase 'o') while the module/type name is VideoOutput. Rename the fixture (and associated TEST_F suite name) to VideoOutputGeneratedUTest to keep naming consistent and improve discoverability in test output.
  include/firebolt/metrics.h:74
• These public virtual method signatures changed from const std::optional<AgePolicy>& to std::optional<AgePolicy> by value. Even though calls may still compile, this is an ABI-breaking change for existing binary consumers of the SDK and forces vtable/signature changes. If ABI stability is required, keep the original parameter types (const reference) and apply [[nodiscard]] without altering argument passing.

    [[nodiscard]] virtual Result<void> startContent(const std::optional<std::string>& entityId,
                                      std::optional<Firebolt::AgePolicy> agePolicy) const = 0;

include/firebolt/metrics.h:86

• These public virtual method signatures changed from const std::optional<AgePolicy>& to std::optional<AgePolicy> by value. Even though calls may still compile, this is an ABI-breaking change for existing binary consumers of the SDK and forces vtable/signature changes. If ABI stability is required, keep the original parameter types (const reference) and apply [[nodiscard]] without altering argument passing.

    [[nodiscard]] virtual Result<void> stopContent(const std::optional<std::string>& entityId,
                                     std::optional<Firebolt::AgePolicy> agePolicy) const = 0;

include/firebolt/firebolt.h:174

• VideoOutputInterface() is added without the Doxygen-style comment block used for the adjacent interface accessors. Add a brief doc comment (matching the surrounding style) so the public accessor API remains consistently documented.

    virtual Actions::IActions& ActionsInterface() = 0;

    virtual VideoOutput::IVideoOutput& VideoOutputInterface() = 0;

src/json_types/actions.h:56

• The brace style/indentation in this newly edited block is inconsistent and makes the control flow harder to read (closing brace } is mis-indented and not aligned with the if). Reformat this block to match the project's formatting conventions (brace on its own line or consistently indented) to keep generated/maintained code readable.

            if (json["intent"]["context"].contains("source")) {
                ctx.source = json["intent"]["context"]["source"].get<std::string>();
}
            value_.intent.context = ctx;

[Copilot]

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (14)

src/device_impl.h:38

• This override uses the non-standard POSIX type u_int32_t. The public interface uses uint32_t, and u_int32_t is not available on all platforms/standard libraries. Use uint32_t for portability and consistency.

    [[nodiscard]] Result<u_int32_t> timeInActiveState() const override;

src/firebolt.cpp:103

• Keep unsubscribeAll() consistent with the member rename (videoOutput_).

        videooutput_.unsubscribeAll();

src/firebolt.cpp:119

• Keep the member declaration consistent with the module name and the other members in this class by using videoOutput_ (capital O).

    VideoOutput::VideoOutputImpl videooutput_;

test/component/videooutputGeneratedTest.cpp:36

• This component test file only asserts that the interface methods exist (pointer-to-member checks) and does not validate any runtime behavior (getter results, enum decoding, or subscriptions). The repository’s existing module component tests validate against the OpenRPC fixture and/or event delivery (e.g., test/component/deviceTest.cpp). Add component tests that exercise the VideoOutput getters and subscribable events end-to-end.
  test/unit/videooutputGeneratedTest.cpp:61
• The unit tests here only cover construction, unsubscribe forwarding, and transport-error propagation for 2 getters. There are no happy-path tests verifying JSON decoding for resolution() or enum getters, and no tests for the subscription methods (subscribeOn*Changed) or unsubscribeAll(). Add unit tests similar to other modules that validate the decoded return values and subscription wiring using MockHelper/MockBase.
  src/firebolt.cpp:91
• Keep the VideoOutputInterface() accessor consistent with the member rename (videoOutput_).

    VideoOutput::IVideoOutput& VideoOutputInterface() override { return videooutput_; }

src/json_types/videooutput.h:114

• Typo in the enum wire mapping: "rgbb444" does not match the ColorFormatValue::Rgb444 name (and the nlohmann mapping above uses "rgb444"). This would prevent correct parsing if this EnumType map is used (e.g., by future tests via validate_enum).
  src/json_types/videooutput.h:55
• DynamicRangeValue::DolbyVision is serialized as "dolby_vision" here, but elsewhere in this same file (the DynamicRangeValueEnum map) the wire value is "dolbyVision". This inconsistency will break round-tripping and can cause event/getter deserialization to fail depending on which adapter is used. Use a single canonical wire string.
  src/json_types/videooutput.h:66
• HdcpState is serialized as "hdcp14"/"hdcp22" here, but the same file’s HdcpStateEnum uses "hdcp1.4"/"hdcp2.2" (and the PR description also lists dotted values). These should match the actual wire protocol; otherwise deserialization/serialization will not interoperate correctly.
  src/videooutput_impl.cpp:25
• <regex> is included but not used anywhere in this translation unit, which adds unnecessary compile time and dependencies. Remove the unused include.
  src/firebolt.cpp:56
• Member naming in FireboltAccessorImpl is inconsistent with the established lowerCamelCase-with-internal-capitals pattern used elsewhere in this class (e.g., textToSpeech_). Consider renaming videooutput_ to videoOutput_ for consistency.

This issue also appears in the following locations of the same file:

• line 91
• line 103
• line 119

          videooutput_(Firebolt::Helpers::GetHelperInstance())

include/firebolt/firebolt.h:174

• All other interface accessors in this header have a brief Doxygen comment block, but VideoOutputInterface() was added without one. Add a matching comment so the generated API docs stay consistent.

    virtual VideoOutput::IVideoOutput& VideoOutputInterface() = 0;

include/firebolt/videooutput.h:108

• VideoOutputResolution declares fields as {height, width}, which is inconsistent with the common {width, height} ordering used elsewhere (e.g., Firebolt::Display::DisplaySize in include/firebolt/display.h:27-31) and with the PR description. Because this is an aggregate type, callers using brace-initialization are likely to accidentally swap values. Consider ordering the fields as {width, height}.

struct VideoOutputResolution
{
    uint32_t height;
    uint32_t width;
};

src/json_types/videooutput.h:176

• VideoOutputResolution is an aggregate, so this brace-initialization must match the field order in the public struct. If the struct is {width, height} (consistent with other size structs), this should return {width_, height_} to avoid swapping the values.

[Copilot]

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (9)

src/device_impl.h:38

• timeInActiveState() is still using the non-standard POSIX typedef u_int32_t. The public interface uses uint32_t, and u_int32_t may be unavailable on non-POSIX toolchains, hurting portability. Align this override to uint32_t.

    [[nodiscard]] Result<u_int32_t> timeInActiveState() const override;

test/unit/videooutputGeneratedTest.cpp:47

• The generated unit tests only exercise transport-error forwarding for 2 getters (resolution, colorDepth) plus unsubscribe(). The module exposes 9 getters and 3 subscription APIs, so this leaves most of the new surface untested (including enum deserialization and subscription wiring). Please add at least one happy-path + one transport-error test per getter, and subscribe/unsubscribe coverage for the 3 events.
  test/component/videooutputGeneratedTest.cpp:35
• The component test is currently only a compile-time interface surface check. In this repo, component tests are expected to exercise real JSON-RPC calls via IFireboltAccessor::Instance() and validate against the OpenRPC fixture (including event delivery + negative payload cases where applicable). Please replace/extend these stubs with real component tests for the getters and the 3 subscribable events.
  include/firebolt/firebolt.h:174
• VideoOutputInterface() is the only accessor here without a Doxygen block, which makes the public accessor surface inconsistent and harder to consume in generated docs. Add a brief @brief/@return comment block like the other interfaces.

    virtual VideoOutput::IVideoOutput& VideoOutputInterface() = 0;

src/stats_impl.h:35

• StatsImpl::~StatsImpl() no longer performs any cleanup (it is = default in the .cpp), so it should be defaulted in the header instead. This avoids an unnecessary out-of-line definition and matches the repo convention for trivial destructors.
  src/lifecycle_impl.h:42
• LifecycleImpl::~LifecycleImpl() no longer performs any cleanup (it is = default in the .cpp), so it should be defaulted in the header instead. This avoids an unnecessary out-of-line definition and matches the repo convention for trivial destructors.
  src/videooutput_impl.cpp:38
• The OpenRPC fixture in docs/openrpc/the-spec/firebolt-open-rpc.json currently has no VideoOutput.* methods/events (e.g. VideoOutput.resolution / VideoOutput.onResolutionChanged). That means the new module can't be validated with the repo’s fixture-driven unit/component test patterns, and the RPC/event names can’t be cross-checked against the spec. Please add the VideoOutput module entries (schemas + examples + events) to the fixture as part of this PR.
  src/videooutput_impl.cpp:64
• Enum getters/subscriptions are implemented via Firebolt::JSON::BasicType<...>, while src/json_types/videooutput.h also defines Firebolt::JSON::EnumType maps for the same enums. This duplicates the wire-value mapping and leaves the *Enum tables unused. To match the pattern used in other modules (e.g. Lifecycle/Device), prefer JsonData adapter classes that decode via EnumType::at() and use those adapters in helper_.get<>() / subscriptionManager_.subscribe<>().
  src/json_types/videooutput.h:44
• This JSON adapter header defines each enum mapping twice: once via NLOHMANN_JSON_SERIALIZE_ENUM(...) and again via Firebolt::JSON::EnumType ...Enum. Since the implementation currently uses BasicType<Enum> (nlohmann-based), the ...Enum tables are unused, and the duplicated sources of truth can drift. Prefer a single enum mapping mechanism (consistent with other modules: Firebolt::JSON::EnumType + JsonData adapter classes) and remove the redundant one.

[Copilot]

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (11)

src/device_impl.h:40

• timeInActiveState() is declared with the non-standard u_int32_t type, while the public interface uses uint32_t (and u_int32_t is not guaranteed to exist on non-POSIX toolchains). Use uint32_t here to keep the override portable and consistent with the API surface.

    [[nodiscard]] Result<std::string> chipsetId() const override;
    [[nodiscard]] Result<DeviceClass> deviceClass() const override;
    [[nodiscard]] Result<HDRFormat> hdr() const override;
    [[nodiscard]] Result<u_int32_t> timeInActiveState() const override;
    [[nodiscard]] Result<std::string> uid() const override;
    [[nodiscard]] Result<uint32_t> uptime() const override;

test/unit/videooutputGeneratedTest.cpp:61

• The generated unit tests only cover resolution() and colorDepth() transport-error forwarding plus unsubscribe. The module adds 9 getters and 3 subscription APIs, so the new behavior is largely untested (no happy-path value checks, no event subscription/unsubscribe coverage).
  test/component/videooutputGeneratedTest.cpp:36
• The component tests are currently only compile-time interface checks. This repo’s component tests normally validate real getter values against the OpenRPC fixture and verify subscription callbacks via triggerEvent(...) (see test/component/actionsGeneratedTest.cpp). Without that, the new VideoOutput APIs/events are not validated end-to-end.
  include/firebolt/videooutput.h:108
• VideoOutputResolution defines fields in height, width order, but elsewhere (e.g., Firebolt::Display::DisplaySize) uses width, height, and the PR description also states {width, height}. Keeping the field order consistent avoids accidental swaps in aggregate initialization / structured bindings.

struct VideoOutputResolution
{
    uint32_t height;
    uint32_t width;
};

include/firebolt/firebolt.h:174

• The new accessor method lacks the Doxygen-style comment block used for the other interface accessors in this header, which makes the public API docs inconsistent.

    virtual Actions::IActions& ActionsInterface() = 0;

    virtual VideoOutput::IVideoOutput& VideoOutputInterface() = 0;
};

src/firebolt.cpp:63

• FireboltAccessorImpl deletes copy operations but does not delete move operations. Since it owns member objects that hold references/subscription managers, moving the accessor can lead to dangling references or double-unsubscribe behavior. Explicitly delete the move ctor/assignment.

    FireboltAccessorImpl(const FireboltAccessorImpl&) = delete;
    FireboltAccessorImpl& operator=(const FireboltAccessorImpl&) = delete;

    ~FireboltAccessorImpl() override { unsubscribeAll(); }

src/stats_impl.h:34

• StatsImpl should follow the project pattern for *Impl classes: mark the helper-taking constructor explicit and explicitly delete move operations (it stores a helper reference).
  src/lifecycle_impl.h:41
• LifecycleImpl should follow the project pattern for *Impl classes: mark the helper-taking constructor explicit and explicitly delete move operations (it stores a helper reference / subscription manager).
  test/unit/videooutputGeneratedTest.cpp:36
• Test fixture and test names use Videooutput... / ...Hasresolution (lowercase 'o' / method name), which is inconsistent with the module’s VideoOutput casing used throughout the SDK (e.g., Firebolt::VideoOutput). Renaming improves discoverability and keeps generated tests consistent with other modules (e.g., ActionsGeneratedUTest).
  test/component/videooutputGeneratedTest.cpp:36
• Test suite and test names use Videooutput... / ...Hasresolution casing which is inconsistent with the module’s VideoOutput casing used throughout the SDK. Aligning the casing keeps generated tests consistent with other modules.
  src/videooutput_impl.cpp:57
• VideoOutputImpl uses Firebolt::JSON::BasicType<Enum> for enum getters/subscriptions, while other modules consistently use module-local JSON adapters backed by Firebolt::JSON::EnumType (e.g., DeviceImpl::deviceClass() uses JsonData::DeviceClassJson, and LifecycleImpl::state() uses JsonData::LifecycleState). This inconsistency also leaves the JsonData::*Enum maps in src/json_types/videooutput.h unused.