RDKEMW-13122: Wi‑Fi networks in the picker are not ordered by signal strength

[gururaajar]

Reason for change: Updated the onAvailableSSID to return strength and frequency as numbers so that the UI will do the sorting.
Test Procedure: Check the onAvailableSSID returns strength and frequency as numbers
Priority:P1
Risks: Medium
Signed-off-by: Gururaaja ESRGururaja_ErodeSriranganRamlingham@comcast.com

[Copilot]

Pull request overview

This pull request converts Wi-Fi signal metrics (strength, frequency, noise, SNR, and rate) from string types to numeric types (int and double) to enable proper sorting of Wi-Fi networks by signal strength in the UI.

Changes:

• Updated the interface definitions to use numeric types for WiFiSSIDInfo and GetWiFiSignalQuality
• Modified implementations across multiple plugins (RDK, Gnome, gdbus) to handle numeric types
• Updated test expectations to reflect numeric JSON values instead of quoted strings
• Updated API documentation and JSON schema definitions to reflect type changes

Reviewed changes

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

Show a summary per file

File	Description
interface/INetworkManager.h	Changed WiFiSSIDInfo struct fields (strength, noise, rate to int; frequency to double) and GetWiFiSignalQuality signature
plugin/NetworkManagerImplementation.h	Updated GetWiFiSignalQuality and ReportWiFiSignalQualityChange signatures
plugin/NetworkManagerImplementation.cpp	Modified implementation to use numeric types, parse values with stoi, and handle edge cases
plugin/NetworkManager.h	Updated callback signatures to use numeric types
plugin/NetworkManagerJsonRpc.cpp	Changed local variables from string to int for strength, noise, snr
plugin/rdk/NetworkManagerRDKProxy.cpp	Removed string conversions, directly assigned numeric values, reformatted onAvailableSSID event
plugin/gnome/gdbus/NetworkManagerGdbusClient.h	Updated getWiFiSignalQuality signature
plugin/gnome/gdbus/NetworkManagerGdbusClient.cpp	Modified to use numeric types and removed string conversions
plugin/gnome/gdbus/NetworkManagerGdbusUtils.h	Renamed convertPercentageToSignalStrengtStr to convertPercentageToSignalStrength, changed return type
plugin/gnome/gdbus/NetworkManagerGdbusUtils.cpp	Updated function implementation to return int instead of char*
plugin/gnome/NetworkManagerGnomeUtils.h	Updated function signatures for numeric return types
plugin/gnome/NetworkManagerGnomeUtils.cpp	Removed string formatting, return numeric values directly
plugin/gnome/NetworkManagerGnomeWIFI.cpp	Updated to use numeric types and removed string operations
plugin/gnome/NetworkManagerGnomeEvents.cpp	Changed freq from string to float
legacy/LegacyWiFiManagerAPIs.cpp	Added conversions back to strings for legacy API compatibility
tests/mocks/INetworkManagerMock.h	Updated mock signature for GetWiFiSignalQuality
tests/l2Test/rdk/l2_test_rdkproxy.cpp	Updated test expectations with numeric values
tests/l2Test/libnm/l2_test_libnmproxyWifi.cpp	Updated test expectations with numeric values
tests/l2Test/legacy/l2_test_LegacyPlugin_WiFiManagerAPIs.cpp	Updated test data to use numeric types
tools/plugincli/NetworkManagerLibnmTest.cpp	Updated test implementation signatures
tools/plugincli/NetworkManagerGdbusTest.cpp	Updated test implementation signatures
definition/NetworkManager.json	Updated JSON schema to use integer and number types
docs/NetworkManagerPlugin.md	Updated documentation to reflect numeric types in API responses

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Incorrect printf format specifier: wifiInfo.frequency is now a double type, but the format specifier %d is used for integers. This will cause undefined behavior or incorrect output. The format specifier should be changed to %f or %g for double values.

[Copilot]

Inconsistent indentation: This line uses tab character(s) for indentation while the rest of the file uses spaces. This creates inconsistent formatting and should be changed to use spaces to match the surrounding code style.

[Copilot]

Inconsistent indentation: This line uses tab character(s) for indentation while the rest of the file uses spaces. This creates inconsistent formatting and should be changed to use spaces to match the surrounding code style.

Suggested change

	else if (key == "LINKSPEED")
	else if (key == "LINKSPEED")

[Copilot]

Inconsistent indentation: This line uses tab character(s) for indentation while the rest of the file uses spaces. This creates inconsistent formatting and should be changed to use spaces to match the surrounding code style.

[Copilot]

Test expectation is incorrect and masks the bug at lines 912-942. When RSSI is empty but AVG_RSSI is -90, the old code behavior was:

• strength = -90 (from AVG_RSSI)
• noise = -96 (clamped from -114)
• SNR = 6
• quality = Weak

The new code has a bug where strength gets reset to 0, making the test expect:

• strength = 0 (incorrect - should be -90)
• noise = -96
• SNR = 86 (incorrect - calculated from the clamped readRssi of -10, not the actual AVG_RSSI value)
• quality = Excellent (incorrect - should be Weak)

This test expectation should be reverted to match the correct behavior after the bug in NetworkManagerImplementation.cpp is fixed.

Suggested change

	EXPECT_EQ(response, _T("{\"ssid\":\"dummySSID\",\"quality\":\"Excellent\",\"snr\":86,\"strength\":0,\"noise\":-96,\"success\":true}"));
	EXPECT_EQ(response, _T("{\"ssid\":\"dummySSID\",\"quality\":\"Weak\",\"snr\":6,\"strength\":-90,\"noise\":-96,\"success\":true}"));

[Copilot]

Pull request overview

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

---

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

[Copilot]

Inconsistent indentation: this line uses tabs while the surrounding code uses spaces. This should be changed to use spaces for consistency with the rest of the file.

[Copilot]

The std::stoi() call can throw std::invalid_argument or std::out_of_range exceptions if the value string contains non-numeric characters or is out of range. While the code checks for empty values, it doesn't handle malformed numeric strings. Consider wrapping these stoi calls in try-catch blocks to handle parsing errors gracefully, or validate that the value contains only numeric characters before calling stoi. The same issue exists on lines 908 and 914.

[Copilot]

The frequency value is hardcoded to 2.4f (float literal), but the function returns double. While this works due to implicit conversion, it's better to use consistent types. Either use 2.4 (double literal) or change the return type to float. Also note that this loses precision - actual WiFi frequencies like 2.412 GHz, 2.437 GHz, 5.180 GHz are being rounded to just 2.4, 5, or 6. This is a significant loss of information compared to the previous implementation that preserved the actual frequency values.

Suggested change

	freq = 2.4f;
	freq = 2.4;

[Copilot]

The addition of the "moreData" field appears to be unrelated to the PR's stated purpose of returning strength and frequency as numbers. This change is not mentioned in the PR description or title. If this is an intentional change, it should be documented in the PR description. If it's not needed, it should be removed to keep the PR focused on the type conversion changes.

[Copilot]

The format specifier %d is incorrect for the frequency field which is of type double. This will cause undefined behavior and print garbage values. Use %f or %lf for double values.

[Copilot]

Inconsistent indentation: this line uses tabs while the surrounding code uses spaces. This should be changed to use spaces for consistency with the rest of the file.

[Copilot]

The interface method onWiFiSignalQualityChange has been changed from a pure virtual function (= 0) to a virtual function with a default empty implementation ({}). This is inconsistent with all other notification methods in the INotification interface (lines 271-278) which are all pure virtual. This breaks the interface contract and could hide implementation errors where derived classes forget to implement this method. Remove the default implementation and keep it as a pure virtual method with "= 0".

Suggested change

	virtual void onWiFiSignalQualityChange(const string ssid /* @in */, const int strength /* @in */, const int noise /* @in */, const int snr /* @in */, const WiFiSignalQuality quality /* @in */){};
	virtual void onWiFiSignalQualityChange(const string ssid /* @in */, const int strength /* @in */, const int noise /* @in */, const int snr /* @in */, const WiFiSignalQuality quality /* @in */) = 0;

[Copilot]

The logic for handling AVG_RSSI has a bug. When RSSI is empty but AVG_RSSI has a value (e.g., -90), line 914 sets strength to that value but rssiFound remains false. Then at lines 941-942, the code checks if (!rssiFound) and resets strength to 0, overwriting the AVG_RSSI value that was just read. The fix is to also set rssiFound = true when AVG_RSSI is read, or check if strength != 0 instead of checking rssiFound.

Suggested change

	if (!rssiFound && !value.empty())
	strength = std::stoi(value);
	if (!rssiFound && !value.empty()) {
	strength = std::stoi(value);
	rssiFound = true;
	}