Rebase

[Abhinavpv28]

No description provided.

[Copilot]

Pull request overview

This PR updates TR-069 host interface support across WiFi and Time profiles by expanding the data model and adding new runtime-backed parameter handlers (notably Chrony-related Time Sync settings and additional WiFi radio stats/bandwidth fields), along with some robustness improvements in JSON parsing.

Changes:

• Add Chrony/NTP tuning parameters under Device.Time.* (get/set handlers) and expose them in the data model XMLs.
• Add non-RDKV_NM implementations for selected Device.WiFi.Radio.{i} and Device.WiFi.Radio.{i}.Stats fields using CLI/procfs parsing, and wire them into the WiFi request handler for non-TR69 builds.
• Harden Thunder JSON parsing in Device_WiFi_EndPoint::refreshCache().

Reviewed changes

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

Show a summary per file

File	Description
src/hostif/profiles/wifi/Device_WiFi_Radio_Stats.h	Adjust radio stats types (e.g., NoiseFloor to signed int) and expose props fetch helpers unconditionally.
src/hostif/profiles/wifi/Device_WiFi_Radio_Stats.cpp	Add non-RDKV_NM radio stats collection (procfs + wpa_cli) and reorganize feature guards.
src/hostif/profiles/wifi/Device_WiFi_Radio.h	Expose props fetch helpers unconditionally (removes previous RDKV_NM guard).
src/hostif/profiles/wifi/Device_WiFi_Radio.cpp	Add non-RDKV_NM OperatingChannelBandwidth collection (iw) and reorganize feature guards.
src/hostif/profiles/wifi/Device_WiFi_EndPoint.cpp	Improve JSON validation and boolean parsing for NetworkManager Thunder responses.
src/hostif/profiles/Time/Device_Time.h	Add new Chrony/NTP parameter getter/setter declarations.
src/hostif/profiles/Time/Device_Time.cpp	Implement Chrony enable + NTP poll/directive/maxstep get/set backed by /opt/secure/RFC/chrony/* files.
src/hostif/parodusClient/waldb/data-model/data-model-tv.xml	Add Device.DeviceInfo.ProductClass and Device.Time.* Chrony/NTP params.
src/hostif/parodusClient/waldb/data-model/data-model-stb.xml	Add Device.DeviceInfo.ProductClass, add Device.Time.* Chrony/NTP params, and remove WiFiReset object (moved to generic).
src/hostif/parodusClient/waldb/data-model/data-model-generic.xml	Add WiFi radio bandwidth + selected stats, and add WiFiReset RFC object.
src/hostif/handlers/src/hostIf_WiFi_ReqHandler.cpp	Add non-TR69 handling for new WiFi radio bandwidth/stats parameters.
src/hostif/handlers/src/hostIf_TimeClient_ReqHandler.cpp	Route new Device.Time.* Chrony/NTP parameters to the Time profile.
CHANGELOG.md	Add 1.3.5 section and update 1.3.4 date entry.

---

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

[Copilot]

wifi_getRadioTrafficStats() returns OK even when opening/reading /proc/net/dev fails, so callers can't distinguish real stats from default zeros. Return NOK on failure paths (and consider propagating the error to the caller) so the TR-181 values don't silently degrade.

[Copilot]

Several Radio Stats getters are now compiled only when RDKV_NM is defined, but WiFiReqHandler calls these (e.g., Stats.BytesSent/BytesReceived/Errors*) under the RDKV_TR69 build. If RDKV_TR69 can be built without RDKV_NM, this will cause undefined references at link time; either provide non-RDKV_NM implementations or guard the call sites with the same feature macro.

[Copilot]

The non-RDKV_NM stats path ignores the requested radioIndex (wifi_getRadioTrafficStats is called with 0 and helpers use wlan0). This will report incorrect values on multi-radio devices; map radioIndex to the correct interface and pass it through consistently.

Suggested change

	wifi_getRadioTrafficStats(0, &stats);
	wifi_getRadioTrafficStats(dev_id, &stats);

[Copilot]

These Radio getters are now behind #ifdef RDKV_NM, but hostIf_WiFi_ReqHandler (RDKV_TR69 path) calls ExtensionChannel/GuardInterval/etc. If there are builds where RDKV_TR69 is enabled but RDKV_NM is not, this will break at link time. Consider aligning the feature guards (or providing stubs/non-NM implementations) so the handler and profile compile consistently.

[Copilot]

wifi_getRadioOperatingChannelBandwidth() hard-codes interfaceName="wlan0" and doesn't use radioIndex, so it will return the wrong bandwidth for multi-radio devices. Either remove the unused parameter or use radioIndex to select the proper interface (e.g., wlan0/wlan1).

[Copilot]

set_Device_Time_NTPMinpoll() rejects invalid/out-of-range values but doesn't set stMsgData->faultCode. Setting fcInvalidParameterValue here (and in other new validated setters) would make failures diagnosable by callers.

Suggested change

	__FUNCTION__, __FILE__, __LINE__, minpollStr.c_str());
	__FUNCTION__, __FILE__, __LINE__, minpollStr.c_str());
	if (stMsgData)
	stMsgData->faultCode = fcInvalidParameterValue;

[Copilot]

set_Device_Time_NTPMaxstep() writes under /opt/secure/RFC/chrony but doesn't ensure the directory exists, and it returns NOK on file-open failure without setting faultCode. Consider creating the directory and setting an appropriate faultCode for I/O failures.

[Copilot]

This new code uses popen() directly. Elsewhere in this repo command execution is typically done via v_secure_popen()/v_secure_system() wrappers; using the wrapper here would be more consistent and reduces risk (shell invocation, environment quirks, etc.).

[Copilot]

wifi_getRadioOperatingChannelBandwidth() invokes shell commands via popen(). The codebase generally uses v_secure_popen() for this; please switch to the secure wrapper to match repository conventions and avoid raw shell execution pitfalls.

[Copilot]

The 1.3.5 changelog section doesn’t include a release date line (most other versions include a '> ' line). Add the date for consistency with the rest of CHANGELOG.md.

Suggested change

	> 4 March 2026

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 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]

Most hostIf_WiFi_Radio::* getter implementations are now wrapped in #ifdef RDKV_NM, but the methods remain declared unconditionally in Device_WiFi_Radio.h. This can cause missing-symbol linker errors in non-RDKV_NM builds if any of these getters are referenced. Consider either providing non-RDKV_NM implementations (even if limited) or restoring consistent #ifdef guards in the header/call sites.

Suggested change

	#ifdef RDKV_NM
	#if 1

[Copilot]

res values outside the handled cases (0-13) will leave status unchanged and rc potentially set from a previous operation, which can report an incorrect SSID status. Add a default case to set a safe status (e.g., "ERROR"/"UNKNOWN") and set rc accordingly before the ERR_CHK check.

[Copilot]

set_Device_Time_NTPMaxstep() writes to NTP_MAXSTEP_FILE but doesn't ensure /opt/secure/RFC/chrony exists, and on file open failure it returns NOK without logging or setting a faultCode. Consider creating the directory (consistent with other setters) and returning a meaningful faultCode/log when persistence fails.

Suggested change

	std::ofstream file(NTP_MAXSTEP_FILE, std::ios::trunc);
	if (!file.is_open())
	return NOK;
	// Ensure the chrony configuration directory exists before writing the file
	const char *chronyDir = "/opt/secure/RFC/chrony";
	struct stat st;
	if (stat(chronyDir, &st) != 0)
	{
	if (errno == ENOENT)
	{
	if (mkdir(chronyDir, 0700) != 0)
	{
	stMsgData->faultCode = fcInternalError;
	fprintf(stderr, "set_Device_Time_NTPMaxstep: Failed to create directory %s: %s\n",
	chronyDir, strerror(errno));
	return NOK;
	}
	}
	else
	{
	stMsgData->faultCode = fcInternalError;
	fprintf(stderr, "set_Device_Time_NTPMaxstep: Failed to stat directory %s: %s\n",
	chronyDir, strerror(errno));
	return NOK;
	}
	}
	else if (!S_ISDIR(st.st_mode))
	{
	stMsgData->faultCode = fcInternalError;
	fprintf(stderr, "set_Device_Time_NTPMaxstep: Path %s exists but is not a directory\n",
	chronyDir);
	return NOK;
	}
	std::ofstream file(NTP_MAXSTEP_FILE, std::ios::trunc);
	if (!file.is_open())
	{
	stMsgData->faultCode = fcInternalError;
	fprintf(stderr, "set_Device_Time_NTPMaxstep: Failed to open %s for writing: %s\n",
	NTP_MAXSTEP_FILE, strerror(errno));
	return NOK;
	}

[Copilot]

Typo in log message: "Empty outpu from Thunder call" -> "Empty output from Thunder call".

Suggested change

	RDK_LOG (RDK_LOG_ERROR, LOG_TR69HOSTIF, "[%s] Empty outpu from Thunder call\n", __FUNCTION__);
	RDK_LOG (RDK_LOG_ERROR, LOG_TR69HOSTIF, "[%s] Empty output from Thunder call\n", __FUNCTION__);

[Copilot]

wifi_getRadioTrafficStats() returns OK even when popen() fails, /proc/net/dev can't be read, or parsing fails (only logs an error). This makes callers treat the stats as valid (often all zeros). Consider returning NOK on failure (and/or only returning OK when all expected fields are parsed successfully).

Suggested change

	if (numParams == 0)
	{
	return NOK;
	}

[Copilot]

The comment says the valid NTPMinpoll range is "[4, 17]" but the validation allows up to 24 (if (minpoll < 4 || minpoll > 24)). Please align the comment and the actual allowed range (either tighten the check to 17 or update the comment/spec reference).

Suggested change

	// Validate that minpollStr is a number in a valid range [4, 17] for NTP
	// Validate that minpollStr is a number in a valid range [4, 24] for NTP

[Copilot]

set_Device_Time_NTPServer1Directive() writes to /opt/secure/RFC/chrony/... but doesn't ensure the chrony directory exists (unlike the minpoll/maxpoll/enable setters). If this is the first chrony parameter set, the open will fail. Consider creating the directory (or factoring a shared helper) in all setters that write under /opt/secure/RFC/chrony.

[Copilot]

Typo in log message: "Empty outpu from Thunder call" -> "Empty output from Thunder call".

Suggested change

	RDK_LOG (RDK_LOG_ERROR, LOG_TR69HOSTIF, "[%s] Empty outpu from Thunder call\n", __FUNCTION__);
	RDK_LOG (RDK_LOG_ERROR, LOG_TR69HOSTIF, "[%s] Empty output from Thunder call\n", __FUNCTION__);

[Copilot]

Pull request overview

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

---

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

[Copilot]

Using SIGKILL (-9) in ExecStop prevents the process from handling shutdown signals and running cleanup (e.g., exit_gracefully), which can lead to corrupted state or skipped teardown. Prefer SIGTERM (or omit ExecStop and let systemd send SIGTERM followed by SIGKILL after TimeoutStopSec) so the daemon can exit cleanly.

Suggested change

ExecStop=/bin/kill -9 $MAINPID

[Copilot]

New Chrony/NTP parameter get/set handlers were added (ChronyEnable, NTPMinpoll/Maxpoll, NTPServer*Directive, NTPMaxstep), but there are existing gtests for Device_Time. Adding unit tests for these new handlers (default values, range validation, and file creation/removal behavior) would help prevent regressions.

[Copilot]

New HotelCheckout getters were added, but the repository already has gtests for DeviceInfo. Adding unit tests for get_HotelCheckoutLastResetTime/get_HotelCheckoutStatus (including JSON-RPC parse failures and empty responses) would help validate the new data model behavior.

[Copilot]

exit_gracefully() calls updateHandler::stop() but no longer waits for the update thread to exit. Since updateHandler::run sleeps for up to 60s between polls, the thread can keep running while teardown continues (IARM stop, hash destroys, etc.), risking use-after-free/crashes. Add a shutdown path that wakes the thread and joins it (or make the thread detached and ensure it cannot touch torn-down resources).

Suggested change

	updateHandler::stop();
	updateHandler::stop();
	if (updateHandler_runThread) {
	g_thread_join(updateHandler_runThread);
	updateHandler_runThread = NULL;
	}

[Copilot]

updateHandler::stop() only flips a flag, but updateHandler::run() can remain blocked in sleep(60) for up to a minute. Without a join/wakeup mechanism, shutdown cannot reliably ensure the thread has stopped before other subsystems are torn down. Consider replacing sleep(60) with a condition variable / g_cond wait that can be signaled on stop(), and provide a join (or join internally in stop()).

Suggested change

	stopped = true;
	stopped = true;
	if (thread != NULL)
	{
	g_thread_join(thread);
	thread = NULL;
	}

[Copilot]

getNoise() / wifi_getRadioTrafficStats() are defined unconditionally but only referenced in the non-RDKV_NM branch. With -Wall/-Werror builds, this can fail compilation under RDKV_NM due to unused static functions. Wrap these helpers in #ifndef RDKV_NM (or annotate maybe_unused) so they are only built when needed.

[Copilot]

wifi_getRadioTrafficStats() logs errors when popen()/parsing fails, but still returns OK. Callers will treat the stats as valid (often all zeros or partially parsed). Return NOK on failure (including numParams != expected) and have callers handle/report the error appropriately.

Suggested change

	if (numParams != 8)
	{
	RDK_LOG(RDK_LOG_ERROR, LOG_TR69HOSTIF, "wifi_getRadioTrafficStats: Failed to parse /proc/net/dev for interface %s (numParams=%d)\n", interfaceName, numParams);
	return NOK;
	}

[Copilot]

In the non-RDKV_NM path, wifi_getRadioOperatingChannelBandwidth()'s return value is ignored and get_Device_WiFi_Radio_Props_Fields() still returns OK. If the command fails, OperatingChannelBandwidth may remain empty/stale while callers believe the fetch succeeded. Propagate the failure (return NOK) or set a clear fallback value and log appropriately when the bandwidth cannot be determined.

[Copilot]

The new handling for "Stats.PacketsReceived" returns whatever hostIf_WiFi_Radio_Stats::get_Device_WiFi_Radio_Stats_PacketsReceived provides. Currently that getter reports hostIf_UnsignedIntType with a 4-byte length, but the updated data model defines PacketsReceived as unsignedLong. Update the getter to use an unsigned long type/length to avoid truncation and keep the response consistent with the model.

[Copilot]

The new helper function wifi_getRadioOperatingChannelBandwidth() is defined unconditionally, but only used in the non-RDKV_NM code path. With the project's -Wall/-Werror builds, this will trigger an unused-function warning (error) when RDKV_NM is enabled. Guard the helper with #ifndef RDKV_NM (or mark it maybe_unused) so RDKV_NM builds don't fail.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.