RDKEMW-15248 : Fix L2 issue in systimemgr by sindhu-krishnan · Pull Request #74 · rdkcentral/systemtimemgr

sindhu-krishnan · 2026-05-12T09:07:50Z

No description provided.

- Fix missing WPEFramework includes that caused 'not a namespace-name' and 'JsonObject does not name a type' errors - Move WPEFramework includes exclusively to .cpp to avoid include path issues when header is included from other translation units - Move internal state (controller, thunder_client, m_networkeventsubscribed) to file-scope statics with internal linkage - Convert internal helper functions (subscribeToInternetEvent, unsubscribeFromInternetEvent, plugin_statechange) to static free functions - not part of the class interface - Remove global thunder_ret, use local variable instead - Remove explicit constructor declaration - compiler-generated default is sufficient; destructor handles RAII cleanup on process exit

Two bugs fixed: 1. Use versioned callsign for thunder_client NETWORK_MANAGER_CALLSIGN ('org.rdk.NetworkManager') was used to create the LinkType for subscribing to onInternetStatusChange, but the versioned callsign NETWORK_MANAGER_PLUGIN ('org.rdk.NetworkManager.1') is required for JSONRPC endpoint resolution. Using the unversioned callsign caused ERROR_TIMEDOUT (11) on every subscription attempt. 2. Attempt immediate subscription at startup After subscribing to the controller statechange event, try subscribeToInternetEvent() immediately. If NetworkManager is already active when systimemgr starts or restarts, no statechange event is fired, so the callback-driven subscription never happens. The immediate attempt handles this case; if it fails (plugin not up yet), the statechange callback will retry on the next activation.

…ents Add consistent 'CHRONY:' prefix to all RDK_LOG statements in networkstatussrc.cpp for easy log filtering with: grep CHRONY /opt/logs/systimemgr.log Also remove stale debug log that printed internetStatus before it was populated, which always printed an empty string.

Agent-Logs-Url: https://github.com/rdkcentral/systemtimemgr/sessions/2e4db5f5-1220-4655-94af-a96b15af594a Co-authored-by: sindhu-krishnan <102755514+sindhu-krishnan@users.noreply.github.com>

Copilot

Pull request overview

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

+    ok = inject_internet_status("NO_INTERNET", interface="eth0")
+    assert ok, "Failed to write NO_INTERNET inject file"
+
+    sleep(2)  # Give sysTimeMgr time to process the event if it mistakenly does
+
+    # The code logs "no action needed" for non-connected states
+    no_action_log = "no action needed"
+    assert _wait_for_log(no_action_log, timeout_s=5), (
+        f"Expected 'no action needed' log not found for NO_INTERNET event"
+    )
+
+    # The processing path must NOT have been triggered by this event
+    processing_log = "CHRONY: Processing internet fully_connected event"
+    log_hits_before = grep_sysTimeMgrlogs(processing_log).count(processing_log)
+    sleep(1)
+    log_hits_after = grep_sysTimeMgrlogs(processing_log).count(processing_log)
+    assert log_hits_before == log_hits_after, (


+# --enable-chrony: build networkstatussrc.cpp against the real WPEFramework
+# Thunder API (production builds).
+# When NOT enabled (default for L1/L2 test builds), WPEFRAMEWORKCORE_CFLAGS/LIBS
+# and WPEFRAMEWORKWEBSOCKET_CFLAGS/LIBS are empty strings so Makefile.am simply
+# adds nothing.  networkstatussrc.cpp then uses WPEFrameworkMock.h via the
+# GTEST_ENABLE or __LOCAL_TEST_ guard, requiring no Thunder installation.
+test -n "$WPEFRAMEWORKCORE_CFLAGS" || WPEFRAMEWORKCORE_CFLAGS=""


+          *      Exit non-zero → Case C. */
+         int chronyActive = v_secure_system("/bin/systemctl is-active --quiet chronyd.service");
+         if (chronyActive != 0) {
+            /* Case A */
+            RDK_LOG(RDK_LOG_INFO, LOG_SYSTIME,


+                  RDK_LOG(RDK_LOG_ERROR, LOG_SYSTIME,
+                          "[%s:%d]: CHRONY: chronyc online failed (ret=%d)."
+                          " chronyd will sync on its own schedule.\n",
+                          __FUNCTION__, __LINE__, ret);
+               }
+            }
+      }


rdkcmf-jenkins · 2026-05-12T10:58:31Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

rdkcmf-jenkins · 2026-05-12T11:01:12Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

rdkcmf-jenkins · 2026-05-12T11:03:58Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

rdkcmf-jenkins · 2026-05-12T11:04:14Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

rdkcmf-jenkins · 2026-05-12T11:04:51Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

rdkcmf-jenkins · 2026-05-12T11:06:13Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

github-actions · 2026-05-12T11:57:26Z

Code Coverage Summary

                                        Total:|75.5%   759|85.5%  62|    -    0

github-actions · 2026-05-12T12:24:17Z

Code Coverage Summary

                                        Total:|75.5%   759|85.5%  62|    -    0

rdkcmf-jenkins · 2026-05-12T12:34:22Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

github-actions · 2026-05-13T05:10:40Z

Code Coverage Summary

                                        Total:|75.5%   759|85.5%  62|    -    0

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

test/functional-tests/tests/test_systimemgr_nwstatus.py:360

The file redefines the same tests and imports a second time. In Python the later definitions override the offset-based versions above, so the new stale-log protections and reset logic are never executed and the suite can still false-pass on old log lines. Remove the duplicate block or merge the intended changes into a single set of test functions.

"""
L2 tests for the NWStatusMonitor feature (topic/NWStatusMonitor).

Verifies that sysTimeMgr:
  1. Subscribes to onInternetStatusChange from org.rdk.NetworkManager.
  2. Processes a FULLY_CONNECTED event and triggers the appropriate
     chrony sync logic.
  3. Ignores non-connected status events (no chrony action logged).
  4. Does not re-process a duplicate FULLY_CONNECTED event while
     lastStatus is already fully_connected.

When compiled with -D__LOCAL_TEST_ (the default for test builds) sysTimeMgr
uses WPEFrameworkMock.h instead of the real Thunder library.  Events are
injected by writing JSON to a file that the mock SmartLinkType polls:
  /tmp/thunder_mock_org_rdk_NetworkManager_onInternetStatusChange.inject

No Thunder daemon, WebSocket server, or network-mock.js is required.
This mirrors the enservice entservices-testframework thunder mock approach.
"""

+AC_ARG_ENABLE([chrony],
+        AS_HELP_STRING([--enable-chrony],[enable NWStatus with real WPEFramework Thunder]),
+        [
+          case "${enableval}" in
+           yes) PKG_CHECK_MODULES([WPEFRAMEWORKCORE],[WPEFrameworkCore >= 1.0.0])
+                PKG_CHECK_MODULES([WPEFRAMEWORKWEBSOCKET],[WPEFrameworkWebSocket >= 1.0.0]);;
+           no) ;;
+          *) AC_MSG_ERROR([bad value ${enableval} for --enable-chrony]) ;;
+          esac
+        ],
+        [echo "chrony: WPEFramework disabled (using mocks for GTEST_ENABLE/__LOCAL_TEST_)"])


+    offset = _log_offset()
+    subscribed = wait_for_nw_subscription(timeout_s=20)
+    assert subscribed, (
+        "sysTimeMgr did not call SmartLinkType::Subscribe() within timeout. "
+        "Ensure sysTimeMgr is compiled with -D__LOCAL_TEST_ and that "
+        "networkstatussrc.cpp is included (topic/NWStatusMonitor branch)."
+    )
+
+    expected_log = "Successfully subscribed to onInternetStatusChange"
+    assert _wait_for_new_log(expected_log, after_offset=offset, timeout_s=10), (
+        f"Expected log line not found after test start in {LOG_FILE}: '{expected_log}'"


 lib_LTLIBRARIES = libsystimerfactory.la

-libsystimerfactory_la_SOURCES = timerfactory.cpp pubsubfactory.cpp rdkdefaulttimesync.cpp drmtimersrc.cpp
+libsystimerfactory_la_SOURCES = timerfactory.cpp pubsubfactory.cpp rdkdefaulttimesync.cpp drmtimersrc.cpp networkstatussrc.cpp


rdkcmf-jenkins · 2026-05-13T05:27:09Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

…t body

github-actions · 2026-05-13T05:44:59Z

Code Coverage Summary

                                        Total:|75.5%   759|85.5%  62|    -    0

rdkcmf-jenkins · 2026-05-13T05:53:59Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

github-actions · 2026-05-13T07:30:06Z

Code Coverage Summary

                                        Total:|75.5%   759|85.5%  62|    -    0

Copilot

Pull request overview

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

+# --enable-unittest) it uses WPEFrameworkMock.h instead of the real Thunder
+# library.  Events are injected by writing JSON to a file that the mock
+# SmartLinkType polls — no Thunder daemon or WebSocket server is needed.


+        auto entry = std::unique_ptr<EventEntry>(new EventEntry());
+        EventEntry* raw = entry.get();
+        entries_[eventName] = std::move(entry);
+
+        const std::string injectPath = injectFilePath(eventName);


+      2. The 'Successfully subscribed' line appears in log content written
+         after this test started (offset-based to avoid stale matches).
+    """
+    offset = _log_offset()


+ * L2 functional tests) use the lightweight header stubs from
+ * unittest/mocks/thunder/WPEFrameworkMock.h so no real Thunder/WPEFramework
+ * installation is required.  irdklog.h is NOT included in test builds;
+ * WPEFrameworkMock.h provides its own RDK_LOG stubs.


+    if (normalizedStatus != "fully_connected" || normalizedStatus == prev) {
+        RDK_LOG(RDK_LOG_INFO, LOG_SYSTIME,


+AC_ARG_ENABLE([chrony],
+        AS_HELP_STRING([--enable-chrony],[enable NWStatus with real WPEFramework Thunder]),
+        [
+          case "${enableval}" in
+           yes) PKG_CHECK_MODULES([WPEFRAMEWORKCORE],[WPEFrameworkCore >= 1.0.0])


 lib_LTLIBRARIES = libsystimerfactory.la

-libsystimerfactory_la_SOURCES = timerfactory.cpp pubsubfactory.cpp rdkdefaulttimesync.cpp drmtimersrc.cpp
+libsystimerfactory_la_SOURCES = timerfactory.cpp pubsubfactory.cpp rdkdefaulttimesync.cpp drmtimersrc.cpp networkstatussrc.cpp


+static void processInternetOnline()
+{
+   RDK_LOG(RDK_LOG_INFO,LOG_SYSTIME,"[%s:%d]: CHRONY: Processing internet fully_connected event\n",
+           __FUNCTION__,__LINE__);
+
+   bool firstSyncDone = (access("/tmp/clock-event", F_OK) == 0);


+
+
+    expected_log = "Successfully subscribed to onInternetStatusChange"
+    assert _wait_for_new_log(expected_log, timeout_s=10), (
+        f"Expected log line not found in {LOG_FILE}: '{expected_log}'"
+    )
+


rdkcmf-jenkins · 2026-05-13T07:38:23Z

+        g_internetUpPending = false;  /* clear — we are about to handle it */
+        lock.unlock();
+        processInternetOnline();
+    }


Coverity Issue - Double unlock

"~unique_lock" unlocks "lock" while it is unlocked. [Note: The source code implementation of the function has been overridden by a builtin model.]

Medium Impact, CWE-765
LOCK

sindhu-krishnan and others added 30 commits April 22, 2026 14:01

Update systimemgr.cpp

fd92117

Update systimemgr.h

1bbbdda

Update systimemgr.cpp

1665a0d

Create networkstatussrc.cpp

fd24ee8

Create networkstatussrc.h

98c5242

Update networkstatussrc.h

2db9a58

Update systimemgr.cpp

c4711b9

Update Makefile.am

4e9f9cf

Update networkstatussrc.h

3c242e8

Update networkstatussrc.cpp

8abffc8

Update networkstatussrc.h

15e706d

Update networkstatussrc.cpp

d70d1d0

Update networkstatussrc.h

e1e1186

Update networkstatussrc.h

d7a6e53

Update networkstatussrc.h

b2458fc

Update networkstatussrc.h

8c76eb0

Update networkstatussrc.h

ffaa74f

Update networkstatussrc.cpp

8bfa505

Update networkstatussrc.h

bd4a5ae

Update networkstatussrc.h

ddbf4c8

Update networkstatussrc.h

a0b86ec

Update networkstatussrc.h

57c4ee0

Update networkstatussrc.cpp

171710c

Update networkstatussrc.h

2c2ed41

Update networkstatussrc.cpp

eee93da

Update networkstatussrc.h

e539a6a

Update networkstatussrc.cpp

63d04af

RDKEMW-15246: localize monitor accessor and cleanup errno handling

932284a

Agent-Logs-Url: https://github.com/rdkcentral/systemtimemgr/sessions/2e4db5f5-1220-4655-94af-a96b15af594a Co-authored-by: sindhu-krishnan <102755514+sindhu-krishnan@users.noreply.github.com>

Copilot AI reviewed May 12, 2026

View reviewed changes

rdkcmf-jenkins reviewed May 12, 2026

View reviewed changes

RDKEMW-15246: Use real rdkloggers in L2 builds; printf stubs only for L1

44d9fa8

rdkcmf-jenkins reviewed May 12, 2026

View reviewed changes

RDKEMW-15246: Fix L2 NWStatus tests per reviewer feedback

b02dab7

Copilot AI review requested due to automatic review settings May 13, 2026 05:08

Copilot started reviewing on behalf of sindhu-krishnan May 13, 2026 05:09 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

rdkcmf-jenkins reviewed May 13, 2026

View reviewed changes

RDKEMW-15246: Fix subscription log offset and remove orphaned old tes…

97f68ad

…t body

rdkcmf-jenkins reviewed May 13, 2026

View reviewed changes

Update test_systimemgr_nwstatus.py

7287376

Copilot AI review requested due to automatic review settings May 13, 2026 07:28

Copilot started reviewing on behalf of sindhu-krishnan May 13, 2026 07:28 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

rdkcmf-jenkins reviewed May 13, 2026

View reviewed changes

		if (normalizedStatus != "fully_connected" \|\| normalizedStatus == prev) {
		RDK_LOG(RDK_LOG_INFO, LOG_SYSTIME,

Conversation

sindhu-krishnan commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

github-actions Bot commented May 12, 2026

Code Coverage Summary

Uh oh!

github-actions Bot commented May 12, 2026

Code Coverage Summary

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

github-actions Bot commented May 13, 2026

Code Coverage Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

github-actions Bot commented May 13, 2026

Code Coverage Summary

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

github-actions Bot commented May 13, 2026

Code Coverage Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Double unlock

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants