RDK-61440: Implementation of handling PowerMode Change in NM plugin #308
Open
Anand73-n wants to merge 7 commits into
Open
RDK-61440: Implementation of handling PowerMode Change in NM plugin #308Anand73-n wants to merge 7 commits into
Anand73-n wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
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 intoNetworkManagerImplementationto 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.
Comment on lines
+402
to
+403
| if(getDefaultInterface() != iface.name) | ||
| ReportActiveInterfaceChange(getDefaultInterface(), iface.name); |
Comment on lines
+29
to
+34
| option(ENABLE_POWERMANAGER "Enable PowerManager COMRPC integration for DeepSleep WiFi management" ON) | ||
| if(ENABLE_POWERMANAGER) | ||
| find_package(${NAMESPACE}Definitions REQUIRED) | ||
| add_definitions(-DENABLE_POWERMANAGER) | ||
| message("NetworkManager: PowerManager integration enabled") | ||
| endif() |
Comment on lines
+2226
to
+2229
| // If there are multiple messages backed up, process a bounded number | ||
| // of pending iterations so this path cannot stall indefinitely if the | ||
| // context keeps receiving new work. | ||
| while (g_main_context_iteration(device_context, FALSE)); |
Reason for change: Ctrl ntwrk state based on PowerMode transitions Test procedure: Change the PowerMode state and verify the behavior Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Reason for change: Add log to know cliendId Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Reason for change: fix crash from power worker thread Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Reason for change: offload prechange WiFi disc to dedicated powerthrd Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Reason for change: enabled ConnectToKnownSSID() Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Reason for change: trigger normal WiFi scan on DeepSleep wakeup with Network Standby ON to handle AP channel changes Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Anand73-n
force-pushed
the
topic/RDK-61440
branch
from
0963fb3 to
afbdab5
Compare
Reason for change: check before initiating wifi connect and disconnect Test procedure: Change the PowerMode and verify the log Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com>
Comment on lines
+1221
to
+1224
| if (m_wlanEnabled.load() && !m_lastConnectedSSID.empty()) { | ||
| NMLOG_INFO("OnPowerModePreChange: waking from DeepSleep — reconnecting to '%s'", | ||
| m_lastConnectedSSID.c_str()); | ||
| ConnectToKnownSSID(m_lastConnectedSSID); // fire-and-forget |
Comment on lines
+265
to
+270
| // Enqueue and return immediately — the power thread does the real work. | ||
| { | ||
| std::lock_guard<std::mutex> lock(mClient.mQueueMutex); | ||
| mClient.mEventQueue.push(PowerEvent{PowerEvent::EventType::PRE_CHANGE, | ||
| currentState, newState, standbyMode, transactionId}); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not merge.