RDK-61440: Implementation of handling PowerMode Change in NM plugin

[Anand73-n]

Reason for change: Control network state based on power mode changes.
Test procedure: Change the PowerMode and verify the logs.
Risks: low
Priority: P1

Signed-off-by: Anand N Anand_N@comcast.com

[Copilot]

Pull request overview

This PR introduces PowerManager integration to handle DeepSleep-related power mode transitions in the NetworkManager plugin, while also improving thread-safety around default-interface handling and making several robustness fixes (iterator null checks, GLib/libnm context isolation, and deinit hooks). It also bumps the plugin/interface version to 2.2.0 and updates documentation/changelog accordingly.

Changes:

• Add a new NetworkManagerPowerClient (COMRPC client) and hook it into NetworkManagerImplementation to act on DeepSleep pre-change/changed events.
• Make default-interface access thread-safe (getDefaultInterface/setDefaultInterface) and update call sites across proxies/connectivity monitor.
• Improve crash resilience/robustness (null checks on iterator creation, platform deinit functions, GNOME wifi operation serialization, minimal ethernet profile support) and update versioning/docs.

Reviewed changes

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

Show a summary per file

File	Description
tests/l2Test/libnm/l2_test_libnmproxy.cpp	Minor unit test expectation cleanup / formatting fix.
plugin/rdk/NetworkManagerRDKProxy.cpp	Use thread-safe default-interface getters/setters; add iterator null checks; add platform_deinit().
plugin/NetworkManagerPowerClient.h	New PowerManager client interface + callback contract for DeepSleep handling.
plugin/NetworkManagerPowerClient.cpp	New COMRPC client implementation with an internal worker thread and pre-change ack handling.
plugin/NetworkManagerJsonRpc.cpp	Add null checks for iterator creation before invoking implementation APIs.
plugin/NetworkManagerImplementation.h	Introduce thread-safe default-interface accessors, PowerManager callback integration, and platform deinit declaration.
plugin/NetworkManagerImplementation.cpp	Instantiate PowerManager client; call platform deinit in destructor; use thread-safe default-interface; implement power callbacks.
plugin/NetworkManagerConnectivity.cpp	Use thread-safe default-interface access in connectivity monitoring logic.
plugin/gnome/NetworkManagerGnomeWIFI.h	Add ethernet-minimal-profile helper; add mutex to serialize wifi operations.
plugin/gnome/NetworkManagerGnomeWIFI.cpp	Context push/pop moved to per-operation; serialize operations; add minimal ethernet profile; improve cleanup/error handling.
plugin/gnome/NetworkManagerGnomeProxy.cpp	Replace global NMClient with per-instance client/context; add deinit; add migration cleanup + minimal ethernet profile on eth enable.
plugin/gnome/gdbus/NetworkManagerGdbusProxy.cpp	Use thread-safe default-interface and add platform_deinit().
plugin/CMakeLists.txt	Add ENABLE_POWERMANAGER option, build/link new power client when enabled.
legacy/LegacyWiFiManagerAPIs.cpp	Add null check after iterator creation.
legacy/LegacyNetworkAPIs.cpp	Add null check after iterator creation.
docs/NetworkManagerPlugin.md	Bump documented version to 2.2.0.
definition/NetworkManager.json	Bump interface version to 2.2.0.
CMakeLists.txt	Bump project version minor to 2 (2.2.0).
CHANGELOG.md	Add entries for 2.2.0 and 2.1.0.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

plugin/NetworkManagerPowerClient.cpp:115

• Operational(false) stops and joins the power-event thread before unregistering from PowerManager. There is a race window where the COM-RPC dispatcher can still deliver an OnPowerModePreChange after the thread has exited, causing the notification handler to enqueue a PRE_CHANGE event that will never be processed/acked, potentially stalling the power-mode transition. Consider unregistering (or otherwise preventing further notifications / sending an immediate ack) before joining the thread, or making the notification path fast-ack when mStopThread is set.

    } else {
        // Stop the power-event thread before unregistering so any in-flight
        // event that was already enqueued is drained with a fast ack.
        mStopThread = true;
        mQueueCv.notify_one();
        if (mPowerThread.joinable()) {
            mPowerThread.join();
        }
        unregisterEventsAndDeactivate();
    }

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

plugin/NetworkManagerImplementation.cpp:1264

• m_wlanDisconnectedForSleep is cleared before attempting ConnectToKnownSSID(). If the reconnect fails, the flag remains false and subsequent wakeups (or retries) will be skipped even though WiFi was disconnected for sleep. Consider only clearing the flag after a successful reconnect, or restoring it when ConnectToKnownSSID() fails.

                        NMLOG_INFO("OnPowerModePreChange: waking from DeepSleep — reconnecting to '%s'",
                               m_lastConnectedSSID.c_str());
                        m_wlanDisconnectedForSleep.store(false);
                        uint32_t rcWifiUp = ConnectToKnownSSID(m_lastConnectedSSID);
                        if (rcWifiUp != Core::ERROR_NONE)
                            NMLOG_WARNING("OnPowerModePreChange: ConnectToKnownSSID failed (rc=%u)", rcWifiUp);

plugin/NetworkManagerImplementation.cpp:1282

• m_ethDisconnectedForSleep is cleared before attempting EthernetConnect(). If the reconnect fails, the flag remains false and later logic will assume Ethernet was not disconnected for sleep. Consider clearing this flag only after a successful EthernetConnect(), or restoring it on failure.

                if (m_ethDisconnectedForSleep.load())
                {
                    m_ethDisconnectedForSleep.store(false);
                    NMLOG_INFO("OnPowerModePreChange: waking from DeepSleep — reconnecting Ethernet");
                    uint32_t rcEthUp = EthernetConnect();
                    if (rcEthUp != Core::ERROR_NONE)
                        NMLOG_WARNING("OnPowerModePreChange: EthernetConnect failed (rc=%u)", rcEthUp);

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (4)

plugin/NetworkManagerImplementation.cpp:1272

• OnPowerModePreChange calls EthernetConnect(), but there is no corresponding implementation for the RDK/netsrvmgr backend. As-is, non-GNOME builds will fail to link. Either guard this reconnect logic behind the GNOME backend build flag or add a non-GNOME implementation (e.g., returning ERROR_UNAVAILABLE).

                if (m_ethDisconnectedForSleep.load())
                {
                    m_ethDisconnectedForSleep.store(false);
                    NMLOG_INFO("OnPowerModePreChange: waking from DeepSleep — reconnecting Ethernet");
                    uint32_t rcEthUp = EthernetConnect();
                    if (rcEthUp != Core::ERROR_NONE)
                        NMLOG_WARNING("OnPowerModePreChange: EthernetConnect failed (rc=%u)", rcEthUp);

plugin/NetworkManagerImplementation.cpp:1299

• OnPowerModeChanged calls ReapplyWifiSettings(), which is only implemented in the GNOME/libnm proxy today. This will break non-GNOME builds at link time. Please either guard this call behind the GNOME backend macro or provide an implementation/stub for other backends.

                    NMLOG_INFO("OnPowerModeChanged: waking from DeepSleep, triggering active WiFi scan");
                    StartWiFiScan("", nullptr);

                    NMLOG_INFO("OnPowerModeChanged: waking from DeepSleep, toggling auto-route-ext-gw on wlan0");
                    ReapplyWifiSettings();
                }

plugin/gnome/NetworkManagerGnomeWIFI.cpp:538

• toggleAutoRouteExtGw passes nm_connection_to_dbus(conn, ...) directly into nm_remote_connection_update2 without unref-ing the returned GVariant. Elsewhere in this file (e.g., around line ~1455) the variant is stored and g_variant_unref()'d after the call, suggesting update2 does not take ownership. Please mirror that pattern here to avoid leaking the settings GVariant on each wakeup.

            /* Save to disk */
            m_isSuccess = false;
            nm_remote_connection_update2(remoteConn,
                                         nm_connection_to_dbus(conn, NM_CONNECTION_SERIALIZE_ALL),
                                         NM_SETTINGS_UPDATE2_FLAG_TO_DISK,
                                         NULL, m_cancellable,
                                         onUpdate2Done, this);

plugin/gnome/NetworkManagerGnomeWIFI.cpp:538

• toggleAutoRouteExtGw intentionally toggles NM_SETTING_IP_CONFIG_AUTO_ROUTE_EXT_GW and persists the change to disk (NM_SETTINGS_UPDATE2_FLAG_TO_DISK). Because this is called on every wake, the on-disk connection profile will flip between DEFAULT and TRUE across sleep cycles, which can permanently alter routing behavior and survive reboots. If the goal is only to force a runtime reapply, avoid TO_DISK and/or avoid changing the stored value (e.g., reapply current settings, update in-memory only, or restore the original value after reapply).

            /* Read current value and toggle */
            NMTernary currentVal = NM_TERNARY_DEFAULT;
            g_object_get(s_ip4, NM_SETTING_IP_CONFIG_AUTO_ROUTE_EXT_GW, &currentVal, NULL);

            NMTernary newVal = (currentVal == NM_TERNARY_DEFAULT) ? NM_TERNARY_TRUE : NM_TERNARY_DEFAULT;
            NMLOG_INFO("toggleAutoRouteExtGw: '%s' auto-route-ext-gw %d -> %d",
                       iface.c_str(), static_cast<int>(currentVal), static_cast<int>(newVal));
            g_object_set(s_ip4, NM_SETTING_IP_CONFIG_AUTO_ROUTE_EXT_GW, newVal, NULL);

            /* Save to disk */
            m_isSuccess = false;
            nm_remote_connection_update2(remoteConn,
                                         nm_connection_to_dbus(conn, NM_CONNECTION_SERIALIZE_ALL),
                                         NM_SETTINGS_UPDATE2_FLAG_TO_DISK,
                                         NULL, m_cancellable,
                                         onUpdate2Done, this);

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.