RDKEMW-13334: Bluetooth plug-in enhancements

[pahearn73]

version: minor

• Implement new APIs
• Add power state monitoring
• Add client notification support
• Add re/connect logic for system boot and power state changes
• Provide means of backward compatibility such that above changes do not affect existing behavior if new APIs are not called.

[Copilot]

Pull request overview

Enhances the Bluetooth Thunder plugin with new JSON-RPC APIs and device-state persistence, and adds power-state monitoring logic intended to manage reconnect/disconnect behavior across system power transitions.

Changes:

• Added BluetoothDeviceManager to persist per-device autoconnect state and last-connect timestamps via PersistentStore.
• Added PowerManager integration and new JSON-RPC methods (setAutoConnect, getAutoConnectStatus) plus corresponding status notifications.
• Added an initial L1 test file for Bluetooth and introduced an L1 tests authoring guide under .github/instructions/.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
Bluetooth/CMakeLists.txt	Adds BluetoothDeviceManager.cpp to the plugin build.
Bluetooth/BluetoothDeviceManager.h	Declares a cache + PersistentStore-backed manager for per-device info.
Bluetooth/BluetoothDeviceManager.cpp	Implements PersistentStore read/write and autoconnect/last-connect-time tracking.
Bluetooth/Bluetooth.h	Adds PowerManager notification plumbing and device manager member.
Bluetooth/Bluetooth.cpp	Registers new JSON-RPC methods, adds PowerManager-based behavior, and augments device lists with new fields.
Tests/L1Tests/tests/test_Bluetooth.cpp	Introduces L1 tests for existing Bluetooth methods (but currently has structural issues).
.github/instructions/l1_tests.instructions.md	Adds a repository guideline for generating L1 tests.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 17 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 16 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

[Copilot]

Pull request overview

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

[Copilot]

@pahearn73 I've opened a new pull request, #36, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 14 comments.

[Copilot]

@pahearn73 I've opened a new pull request, #37, to work on those changes. Once the pull request is ready, I'll request review from you.

[rdkcmf-jenkins]

Coverity Issue - Large stack use

Local variable "" uses 72728 bytes of stack space, which exceeds the maximum single use of 10000 bytes.

Low Impact, CWE-400
STACK_USE

[Copilot]

Pull request overview

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

[rdkcmf-jenkins]

Coverity Issue - Large stack use

Local variable "" uses 72728 bytes of stack space, which exceeds the maximum single use of 10000 bytes.

Low Impact, CWE-400
STACK_USE

[rdkcmf-jenkins]

Coverity Issue - Large stack use

Local variable "" uses 72728 bytes of stack space, which exceeds the maximum single use of 10000 bytes.

Low Impact, CWE-400
STACK_USE

[Copilot]

Pull request overview

Copilot reviewed 21 out of 23 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

Tests/README.md:5

• Spelling: "commited" should be "committed" (appears twice in this sentence).

Hence, any modifications/additions related to mocks should be commited to entservices-testframework repo @ rdkcentral and any modifications/additions related to test case should be commited to Test directory of corresponding entservices repo.

Bluetooth/CMakeLists.txt:41

• The option guard uses RDK_SERVICE_L2_TEST, but elsewhere in this repo the naming pattern is RDK_SERVICES_* (e.g., RDK_SERVICES_L1_TEST). If the intended flag is RDK_SERVICES_L2_TEST, this block will never execute and L2 mock linking will be skipped.

if (RDK_SERVICE_L2_TEST)
    find_library(TESTMOCKLIB_LIBRARIES NAMES TestMocklib)
    if (TESTMOCKLIB_LIBRARIES)
        message ("linking mock libraries ${TESTMOCKLIB_LIBRARIES} library")
        target_link_libraries(${MODULE_NAME} PRIVATE ${TESTMOCKLIB_LIBRARIES})

[Copilot]

Pull request overview

Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

Bluetooth/CMakeLists.txt:41

• The build flag is checked as RDK_SERVICE_L2_TEST, but this repository consistently uses the RDK_SERVICES_* prefix (e.g., RDK_SERVICES_L1_TEST). As-is, this block likely never runs and mock linking for L2 tests won't work. Use the correct/consistent CMake option name (or define/propagate RDK_SERVICE_L2_TEST).

if (RDK_SERVICE_L2_TEST)
    find_library(TESTMOCKLIB_LIBRARIES NAMES TestMocklib)
    if (TESTMOCKLIB_LIBRARIES)
        message ("linking mock libraries ${TESTMOCKLIB_LIBRARIES} library")
        target_link_libraries(${MODULE_NAME} PRIVATE ${TESTMOCKLIB_LIBRARIES})

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

helpers/frontpanel.cpp:167

• ASSERT(_powerManagerPlugin); will fire if PowerManager is unavailable, even though the code immediately handles the null case with an if (_powerManagerPlugin) check. Since the interface acquisition now includes retries and can still legitimately fail, remove/relax this assert to avoid crashing in debug builds.

                        Core::hresult res = Core::ERROR_GENERAL;
                        PowerState pwrStateCur = WPEFramework::Exchange::IPowerManager::POWER_STATE_UNKNOWN;
                        PowerState pwrStatePrev = WPEFramework::Exchange::IPowerManager::POWER_STATE_UNKNOWN;
                        ASSERT (_powerManagerPlugin);
                        if (_powerManagerPlugin) {

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

CMakeLists.txt:24

• Leftover debug scaffolding (# <pca> debug with commented-out project(...)) should be removed to avoid confusing future maintainers and to keep the top-level CMake clean.

# <pca> debug
#project(Connectivity)
# </pca>

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

helpers/PluginInterfaceBuilder.h:75

• PluginInterfaceRef now stores _service, but it is never used or released, and the move constructor explicitly drops it (_service(nullptr)). This is confusing (and the comment in createInterface() suggests ownership is transferred). Either remove _service from PluginInterfaceRef, or define clear ownership semantics (e.g., AddRef/Release it consistently and move it correctly).

    class PluginInterfaceRef {
        INTERFACE* _interface;
        PluginHost::IShell* _service;

    public:
        PluginInterfaceRef()
            : _interface(nullptr)
            , _service(nullptr)
        {
        }

        PluginInterfaceRef(INTERFACE* interface, PluginHost::IShell* controller)
            : _interface(interface)
            , _service(controller)
        {
        }

        ~PluginInterfaceRef()
        {
            Reset();
        }

        // avoid copies
        PluginInterfaceRef(const PluginInterfaceRef&) = delete;
        PluginInterfaceRef& operator=(const PluginInterfaceRef&) = delete;

        // use move
        PluginInterfaceRef(PluginInterfaceRef&& other)
            : _interface(other._interface)
            , _service(nullptr)
        {
            other._interface = nullptr;
        }

        PluginInterfaceRef& operator=(PluginInterfaceRef&& other)
        {
            if (this != &other) {
                _interface = other._interface;
                other._interface = nullptr;
            }
            return *this;

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

helpers/PluginInterfaceBuilder.h:76

• PluginInterfaceRef now stores a _service pointer, but it is never used, never released, and it is not moved/assigned (move ctor sets it to nullptr). This is misleading (comment says ownership is passed) and can cause accidental dangling usage later; either remove _service entirely or manage its lifetime consistently (AddRef/Release + proper move semantics).

        PluginInterfaceRef(INTERFACE* interface, PluginHost::IShell* controller)
            : _interface(interface)
            , _service(controller)
        {
        }

        ~PluginInterfaceRef()
        {
            Reset();
        }

        // avoid copies
        PluginInterfaceRef(const PluginInterfaceRef&) = delete;
        PluginInterfaceRef& operator=(const PluginInterfaceRef&) = delete;

        // use move
        PluginInterfaceRef(PluginInterfaceRef&& other)
            : _interface(other._interface)
            , _service(nullptr)
        {
            other._interface = nullptr;
        }

        PluginInterfaceRef& operator=(PluginInterfaceRef&& other)
        {
            if (this != &other) {
                _interface = other._interface;
                other._interface = nullptr;
            }
            return *this;
        }

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

helpers/PluginInterfaceBuilder.h:75

• PluginInterfaceRef now stores _service, but it is never used/released and it is not moved in the move ctor/assignment. This is misleading (the comment says ownership is passed) and risks dangling pointers if it’s later used; either remove _service or implement clear ownership semantics (AddRef/Release + proper move/reset).

    class PluginInterfaceRef {
        INTERFACE* _interface;
        PluginHost::IShell* _service;

    public:
        PluginInterfaceRef()
            : _interface(nullptr)
            , _service(nullptr)
        {
        }

        PluginInterfaceRef(INTERFACE* interface, PluginHost::IShell* controller)
            : _interface(interface)
            , _service(controller)
        {
        }

        ~PluginInterfaceRef()
        {
            Reset();
        }

        // avoid copies
        PluginInterfaceRef(const PluginInterfaceRef&) = delete;
        PluginInterfaceRef& operator=(const PluginInterfaceRef&) = delete;

        // use move
        PluginInterfaceRef(PluginInterfaceRef&& other)
            : _interface(other._interface)
            , _service(nullptr)
        {
            other._interface = nullptr;
        }

        PluginInterfaceRef& operator=(PluginInterfaceRef&& other)
        {
            if (this != &other) {
                _interface = other._interface;
                other._interface = nullptr;
            }
            return *this;

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

helpers/PluginInterfaceBuilder.h:76

• PluginInterfaceRef now stores _service, but it is never used or released in Reset()/destructor, and the move ctor/assignment do not move it. This is confusing (and the comment in createInterface() says ownership is passed). Either remove _service or implement consistent ownership semantics (e.g., AddRef/Release and correct move behavior).

    class PluginInterfaceRef {
        INTERFACE* _interface;
        PluginHost::IShell* _service;

    public:
        PluginInterfaceRef()
            : _interface(nullptr)
            , _service(nullptr)
        {
        }

        PluginInterfaceRef(INTERFACE* interface, PluginHost::IShell* controller)
            : _interface(interface)
            , _service(controller)
        {
        }

        ~PluginInterfaceRef()
        {
            Reset();
        }

        // avoid copies
        PluginInterfaceRef(const PluginInterfaceRef&) = delete;
        PluginInterfaceRef& operator=(const PluginInterfaceRef&) = delete;

        // use move
        PluginInterfaceRef(PluginInterfaceRef&& other)
            : _interface(other._interface)
            , _service(nullptr)
        {
            other._interface = nullptr;
        }

        PluginInterfaceRef& operator=(PluginInterfaceRef&& other)
        {
            if (this != &other) {
                _interface = other._interface;
                other._interface = nullptr;
            }
            return *this;
        }