Release/4.0.0 - #357
Conversation
Sysint release 3.0.8 version
Co-authored-by: mtirum011 <madhubabu_tirumala@comcast.com>
Co-authored-by: nhanasi <navihansi@gmail.com>
Bringing in changes made for: RDKEMW-9493,RDKEMW-9440,RDKEMW-4148 Signed-off-by: Balaji Punnuru <Balaji_Punnuru@comcast.com> Co-authored-by: Balaji Punnuru <Balaji_Punnuru@comcast.com>
(cherry picked from commit 70f8618) Co-authored-by: Aravindan NC <35158113+AravindanNC@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements NetworkManager-based connectivity checks and migrates from custom network-up.target to the standard systemd network-online.target. The changes remove the custom connectivity check script in favor of NetworkManager's built-in capabilities and add proper WiFi credential migration logic.
Key changes:
- Replace custom connectivity checking with NetworkManager's native network-online.target
- Add WiFi credential migration from wpa_supplicant to NetworkManager
- Update systemd service dependencies from network-up.target to network-online.target
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| notify-network-ready.service | New service to log internet connectivity milestone and create connectivity check marker file |
| minidump-upload.service | Updated network dependency from network-up.target to network-online.target |
| minidump-secure-upload.service | Updated network dependency from network-up.target to network-online.target |
| coredump-upload.service | Updated network dependency from network-up.target to network-online.target |
| coredump-secure-upload.service | Updated network dependency from network-up.target to network-online.target |
| NetworkManager_ecfs.conf | New NetworkManager configuration to ensure it starts after secure mount |
| rebootNow.sh | Added boot_FSR to OPS_TRIGGERED_REASONS array |
| factory-reset.sh | Added cleanup of NetworkManager profile directories during factory reset |
| connectivitycheck.sh | Removed custom connectivity check script (replaced by NetworkManager) |
| NM_Dispatcher.sh | Removed custom connectivity check invocation and connection state validation |
| NM_Bootstrap.sh | Added WiFi credential migration logic from wpa_supplicant to NetworkManager |
| CHANGELOG.md | Updated with version 4.0.0 release notes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sed -i '/network={/,/}/d' /opt/secure/wifi/wpa_supplicant.conf | ||
| fi | ||
|
|
||
| if [ -z $SSID ]; then |
There was a problem hiding this comment.
Variable $SSID should be quoted to properly handle empty values. The current syntax [ -z $SSID ] will fail if SSID is unset or contains whitespace. Use [ -z \"$SSID\" ] instead.
| if [ "$RDK_PROFILE" == "TV" ]; then | ||
| echo "`/bin/timestamp` :$0: Migrating Wifi credentials for TVs from NM_Bootsrtap" >> /opt/logs/NMMonitor.log | ||
| fi | ||
| if [ -z $PSK ]; then |
There was a problem hiding this comment.
Variable $PSK should be quoted to properly handle empty values. The current syntax [ -z $PSK ] will fail if PSK is unset or contains whitespace. Use [ -z \"$PSK\" ] instead.
| if [ -z $PSK ]; then | |
| if [ -z "$PSK" ]; then |
| rm -rf /opt/NetworkManager/system-connections/* | ||
| fi | ||
| if [ "$RDK_PROFILE" == "TV" ]; then | ||
| echo "`/bin/timestamp` :$0: Migrating Wifi credentials for TVs from NM_Bootsrtap" >> /opt/logs/NMMonitor.log |
There was a problem hiding this comment.
Corrected spelling of 'NM_Bootsrtap' to 'NM_Bootstrap'.
| echo "`/bin/timestamp` :$0: Migrating Wifi credentials for TVs from NM_Bootsrtap" >> /opt/logs/NMMonitor.log | |
| echo "`/bin/timestamp` :$0: Migrating Wifi credentials for TVs from NM_Bootstrap" >> /opt/logs/NMMonitor.log |
|
|
||
| if [ "$interfaceStatus" = "up" ]; then | ||
| /usr/bin/nm-online -q -t 60 # If Network manager is not online wait for 60 sec. TODO: Revisit this during connectivity check enable time | ||
|
|
There was a problem hiding this comment.
Remove this empty line that was left behind after removing the nm-online command. The blank line with only whitespace reduces code clarity.
| mkdir -p /opt/secure/NetworkManager/system-connections | ||
| fi | ||
| if [ -d /opt/NetworkManager/system-connections ]; then | ||
| cp /opt/NetworkManager/system-connections/* /opt/secure/NetworkManager/system-connections/ |
There was a problem hiding this comment.
The wildcard expansion * will fail if the directory is empty, causing the script to error. Add a check to verify files exist before copying, or use a safer pattern like cp -r /opt/NetworkManager/system-connections/. /opt/secure/NetworkManager/system-connections/ 2>/dev/null || true.
| cp /opt/NetworkManager/system-connections/* /opt/secure/NetworkManager/system-connections/ | |
| cp -r /opt/NetworkManager/system-connections/. /opt/secure/NetworkManager/system-connections/ 2>/dev/null || true |
No description provided.