RDKTV-39792:[RDKV] Xumo TV devices with older builds failing to upgrade due to image verification failure error

[mkadinti]

No description provided.

[Copilot]

Pull request overview

This PR addresses upgrade failures on older Xumo TV builds by introducing explicit upgrade-library error codes (instead of exiting), improving error reporting, and enhancing D-Bus download behavior via cached XConf metadata. It also updates unit/functional test scaffolding and adds initial build/public-header wiring for a client library.

Changes:

• Add upgrade-library error codes + rdkv_upgrade_strerror() and propagate fatal conditions via return codes instead of exit(1).
• Update daemon/CLI flows to handle negative “library error” returns and improve DownloadFirmware URL resolution using in-memory/file XConf cache.
• Adjust test mocks/build scripts and add an initial client library header + build target wiring.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
unittest/test_dbus_fake.cpp	Updates copyright year in unit test file header.
unittest/rdkfwupdatemgr_main_flow_gtest.cpp	Switches g_DeviceUtilsMock global handling to shared global header/pointer.
unittest/rdkFwupdateMgr_handlers_gtest.cpp	Removes/adjusts tests and updates mock lambda signatures to const RdkUpgradeContext_t*.
unittest/mocks/rdkFwupdateMgr_mock.h	Updates mock interface signature for rdkv_upgrade_request to match const-correct production API.
unittest/mocks/rdkFwupdateMgr_mock.cpp	Moves device utils stubs out; adds handler dependency stubs (incl. rdkv_upgrade_strerror, getOPTOUTValue).
unittest/mocks/deviceutils_mock.cpp	Defines g_DeviceUtilsMock global pointer in one place for all test binaries.
unittest/mocks/dbus_handlers_gmock.cpp	Adds additional stubs for handler tests (DownloadFirmware/opt-out/strerror).
unittest/miscellaneous_mock.cpp	Aligns mock signatures (e.g., checkAndEnterStateRed now returns int) and adds strerror/handler stubs under test flags.
unittest/deviceutils_mock_global.h	New shared header declaring extern DeviceUtilsMock *g_DeviceUtilsMock.
unittest/deviceutils/deviceutils_gtest.cpp	Stops defining g_DeviceUtilsMock locally; relies on shared global.
unittest/basic_rdkv_main_gtest.cpp	Switches to shared g_DeviceUtilsMock declaration and includes new global header.
unittest/Makefile.am	Updates unit test source lists and mock linkage (notably adds deviceutils_mock.cpp to some targets).
test/functional-tests/tests/test_dbus_UpdateFirmware.py	Minor cleanup of unused variables/imports in UpdateFirmware functional test.
test/functional-tests/tests/test_dbus_DownloadFirmware.py	Changes helper import style and adjusts some local variable usage/exception handling.
test/functional-tests/tests/test_dbus_CheckForUpdate.py	Changes helper import style and removes an unused import.
src/rdkv_upgrade.c	Adds rdkv_upgrade_strerror, converts fatal exits into return codes, and checks checkAndEnterStateRed() return.
src/rdkv_main.c	Handles negative library errors from rdkv_upgrade_request (CLI exits on fatal library errors).
src/rdkFwupdateMgr.c	Handles negative library errors from rdkv_upgrade_request (daemon logs and continues).
src/include/rdkv_upgrade.h	Introduces negative-valued library error enum and declares rdkv_upgrade_strerror(int).
src/include/device_status_helper.h	Changes checkAndEnterStateRed signature to return int.
src/device_status_helper.c	Implements checkAndEnterStateRed as non-exiting and returns status to caller.
src/dbus/rdkv_dbus_server.c	Updates DownloadFirmware validation and resolves effective download URL via in-memory/file XConf cache.
src/dbus/rdkFwupdateMgr_handlers.h	Updates DownloadFirmware API signature and exposes cache helper APIs for daemon/tests.
src/dbus/rdkFwupdateMgr_handlers.c	Adds global in-memory XConf cache + opt-out response helper and adjusts cache saving behavior.
src/chunk.c	Converts “force exit” behavior into negative library return code and includes upgrade error header.
run_l2.sh	Removes --enable-extended-logger from configure invocation.
librdkFwupdateMgr/include/rdkFwupdateMgr_client.h	Adds initial public client API header for D-Bus clients.
cov_build.sh	Adjusts build steps and removes --enable-extended-logger.
Makefile.am	Adds librdkFwupdateMgr.la target and installs its public header.
CHANGELOG.md	Updates changelog with new release notes entries.

Comments suppressed due to low confidence (2)

test/functional-tests/tests/test_dbus_DownloadFirmware.py:33

• The import was changed to import rdkfw_test_helper, but the module’s helpers are still used unqualified (e.g., initial_rdkfw_setup(), remove_file()). This will raise NameError when running the tests. Please either switch back to from rdkfw_test_helper import ... or prefix all helper calls with rdkfw_test_helper..

import rdkfw_test_helper

# D-Bus Configuration
DBUS_SERVICE_NAME = "org.rdkfwupdater.Service"
DBUS_OBJECT_PATH = "/org/rdkfwupdater/Service"
DBUS_INTERFACE = "org.rdkfwupdater.Interface"
DAEMON_BINARY = "/usr/local/bin/rdkFwupdateMgr"

test/functional-tests/tests/test_dbus_CheckForUpdate.py:32

• The import was changed to import rdkfw_test_helper, but helper functions are still called unqualified (e.g., rename_file(), write_on_file(), remove_file()). This will raise NameError when running the tests. Please either revert to from rdkfw_test_helper import ... or prefix calls with rdkfw_test_helper..

import json

import rdkfw_test_helper

# D-Bus Configuration
DBUS_SERVICE_NAME = "org.rdkfwupdater.Service"
DBUS_OBJECT_PATH = "/org/rdkfwupdater/Service"
DBUS_INTERFACE = "org.rdkfwupdater.Interface"
DAEMON_BINARY = "/usr/local/bin/rdkFwupdateMgr"

---

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

[Copilot]

librdkFwupdateMgr_la_SOURCES currently lists a directory (${top_srcdir}/librdkFwupdateMgr/src/) instead of source files. Automake will treat this as an invalid source entry and fail the build. Please replace it with the concrete .c/.cpp files (or remove the library target until sources are added).

[Copilot]

The test stub rdkFwupdateMgr_downloadFirmware(...) is declared as variadic and returns int, but the production header declares it as returning DownloadFirmwareResult with a fixed parameter list. If this symbol is ever referenced from C code compiled against the header, the return-type/ABI mismatch can cause crashes. Please change the stub to match the real signature and return type (or remove it if unused).

[Copilot]

getOPTOUTValue is stubbed here with no parameters, but production code calls getOPTOUTValue(const char *path) (see handlers.c forward declaration). In C++ this mismatch defeats type checking and can be undefined behavior on some ABIs. Please update the stub signature to accept const char * (and keep it consistent with other mocks that also define this function).

[Copilot]

The rdkv_upgrade_strerror stub uses -1/-2 for the new library error cases, but the updated public enum in rdkv_upgrade.h defines RDKV_UPGRADE_ERROR_THROTTLE_ZERO=-100 and RDKV_UPGRADE_ERROR_FORCE_EXIT=-101. This will produce incorrect messages/branching in tests that validate error handling. Please update the stub to use the new constants (or include rdkv_upgrade.h and switch on the enum values).

[Copilot]

This rdkv_upgrade_strerror stub also maps the throttle/force-exit errors to -1/-2, but production now uses RDKV_UPGRADE_ERROR_THROTTLE_ZERO=-100 and RDKV_UPGRADE_ERROR_FORCE_EXIT=-101 (see src/include/rdkv_upgrade.h). Please update the cases to match the new error codes so unit tests reflect real behavior.

[Copilot]

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 11 comments.

---

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

[Copilot]

The added stubs introduce ABI/signature mismatches that can cause crashes or undefined behavior at link/runtime. rdkFwupdateMgr_downloadFirmware is defined here as extern "C" int rdkFwupdateMgr_downloadFirmware(...), but the real symbol now returns DownloadFirmwareResult (a struct). Also getOPTOUTValue is defined with no parameters, but other code calls getOPTOUTValue(const char*). Because these are extern "C", the linker will happily bind the symbol and the call/return ABI will be wrong. Fix by matching the exact production signatures (including return type and parameters) or remove these stubs and rely on the existing ones in the appropriate mock files.

[Copilot]

*_SOURCES references ../librdkFwupdateMgr/src/rdkFwupdateMgr_async_api.c, but that file does not exist in the tree (the file present is rdkFwupdateMgr_api.c). As written, these new test targets will fail to build. Update the source list to the correct filename(s) and ensure any required test-only helpers are actually compiled into the binaries.

Suggested change

	../librdkFwupdateMgr/src/rdkFwupdateMgr_async_api.c
	../librdkFwupdateMgr/src/rdkFwupdateMgr_api.c

[Copilot]

This test file declares/uses several async test helper APIs and entry points (rdkFwupdateMgr_async_init_for_test, *_cleanup_for_test, *_get_pending_count_for_test, etc.) plus rdkFwupdateMgr_checkForUpdate_async, but none of these symbols exist in the current codebase (the public client API is checkForUpdate(...) in rdkFwupdateMgr_client.h). As-is, this file will not compile/link. Either rename the calls to match the actual public API, or add the required test-only exports (guarded by a macro) and compile them into this test target.

[Copilot]

This file depends on rdkFwupdateMgr_async_init_for_test/cleanup_for_test/simulate_signal_for_test and calls rdkFwupdateMgr_checkForUpdate_async(...), but those symbols are not defined anywhere in the repository (and rdkFwupdateMgr_client.h does not declare them). This will fail to compile/link. Please align the tests with the actual client API (checkForUpdate, signal subscription) or add the missing test hooks behind a compile-time flag and include them in the test target sources.

[Copilot]

This file declares/uses async test helper APIs (rdkFwupdateMgr_async_*_for_test) and calls rdkFwupdateMgr_checkForUpdate_async/_cancel, but these symbols are not present in the current codebase (the public API in rdkFwupdateMgr_client.h is checkForUpdate(...)). As-is, the test target will not compile/link. Either refactor the tests to use the actual API, or add/export the required async test hooks and include them in the build.

[Copilot]

This refcount test appears to target a different/internal async implementation than the one currently in librdkFwupdateMgr: it includes rdkFwupdateMgr_async_internal.h but then uses AsyncCallbackContext, AsyncCallbackRegistry, and context_ref/context_unref, none of which are declared/defined in that header or anywhere else in the repo. This will not compile/link. Suggest either (a) updating the test to use the actual internal types/functions that exist in rdkFwupdateMgr_async.c, or (b) adding explicit test-only exports for the refcount helpers and the associated types behind a macro, and including them in the test target sources.

[Copilot]

rdkv_upgrade_strerror() in this mock uses -1 / -2 for the new library error codes, but production now defines RDKV_UPGRADE_ERROR_THROTTLE_ZERO = -100 and RDKV_UPGRADE_ERROR_FORCE_EXIT = -101 (see src/include/rdkv_upgrade.h). As a result, tests that log/interpret these errors will produce misleading messages and may not exercise the intended branches. Update the switch cases to match the new negative values (or use the enum constants).

[Copilot]

The import was changed from from rdkfw_test_helper import * to import rdkfw_test_helper, but the rest of the file still calls helper functions (e.g., initial_rdkfw_setup(), remove_file(), rename_file(), etc.) without the rdkfw_test_helper. prefix. This will raise NameError at runtime for most tests. Either restore the star import, or update all helper references to be fully qualified.

Suggested change

	import rdkfw_test_helper
	from rdkfw_test_helper import *

[Copilot]

The import was changed from from rdkfw_test_helper import * to import rdkfw_test_helper, but the rest of the test module still calls helper functions (e.g., initial_rdkfw_setup(), write_on_file(), remove_file(), etc.) without the rdkfw_test_helper. prefix. This will cause NameError at runtime. Either revert to the star import or qualify all helper calls.

Suggested change

	import rdkfw_test_helper
	from rdkfw_test_helper import *

[Copilot]

start_daemon() return value is no longer captured, and this test no longer stops the daemon process or performs cleanup in a finally block. That leaves the daemon running across tests and can cause flakiness/port conflicts and resource leaks. Suggest restoring proc = start_daemon() and ensuring stop_daemon(proc) (and any temp file cleanup) always runs in finally:.

[github-actions]

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

5 out of 6 committers have signed the CLA.
✅ satya200
✅ mkadinti
✅ Sidsohail
✅ KTirumalaSrihari
✅ Ananth916
❌ root
root seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[Copilot]

Pull request overview

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

---

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

[Copilot]

The stubs added here don't match the real function signatures used by production code linked into dbus_handlers_gtest. In particular, getOPTOUTValue() is defined with no parameters, but rdkFwupdateMgr_handlers.c declares/calls int getOPTOUTValue(const char *file_name). This is undefined behavior and can break on some ABIs. Define the stub with the exact signature (and do the same for rdkFwupdateMgr_downloadFirmware, which should match the struct-returning declaration).

[Copilot]

This test shares callback_invoked and the callback_* strings between the test thread and the async callback thread without any synchronization. The callback writes these globals while the test reads them, which is a data race (and will be flagged by TSAN / can flake). Protect the shared state with a mutex/condition_variable (or make the fields atomic where applicable) and wait for the callback deterministically instead of relying on sleeps.

[Copilot]

ConcurrentRegisterAndCancel mutates and reads handler_ids from multiple threads without synchronization (writes in the register path, reads in the cancel path). Even if indices differ most of the time, this is still a data race on the underlying int elements and makes the test itself non-thread-safe. Use an std::vector<std::atomic<int>> (or guard access with a mutex) when sharing IDs across threads.

[Copilot]

This test includes an internal header via a hard-coded relative path (../librdkFwupdateMgr/src/...). This is brittle (depends on build working directory) and duplicates include-path logic already present in the Makefile. Prefer including the header by name (e.g., rdkFwupdateMgr_async_internal.h) and relying on the -I include paths to make builds more portable.

[Copilot]

bin_PROGRAMS lists rdkFwupdateMgr_async_main_flow_gtest and rdkFwupdateMgr_async_handlers_gtest, but this Makefile does not define corresponding *_SOURCES / build rules for them. Automake will fail with “no rule to make target …” or missing source errors. Either add the missing build stanzas or remove these binaries from bin_PROGRAMS until they exist.

[Copilot]

These async gtest targets reference ../librdkFwupdateMgr/src/rdkFwupdateMgr_async_api.c, but that file is not present in the tree (the async/public API implementation appears to be rdkFwupdateMgr_api.c). This will break the unit-test build; update the *_SOURCES lists to point at the correct source file(s).

[Copilot]

CleanupDuringCallbackInvocation can hang indefinitely: cleanup_thread busy-waits for callback_running to become true, but the test never triggers a signal/callback (the signal_thread only sleeps and does nothing). This will deadlock the test run. Trigger the callback via the existing test helper (e.g., simulate a signal) or add a bounded wait/timeout and fail the test if the callback never starts.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

test/functional-tests/tests/test_dbus_DownloadFirmware.py:31

• This file switched from from rdkfw_test_helper import * to import rdkfw_test_helper, but the rest of the module still calls helper functions like remove_file() and initial_rdkfw_setup() unqualified. As written, this will raise NameError at runtime. Either revert to the star import or update all helper calls to use the module prefix (e.g., rdkfw_test_helper.remove_file(...)).

---

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

[Copilot]

bin_PROGRAMS includes rdkFwupdateMgr_async_main_flow_gtest and rdkFwupdateMgr_async_handlers_gtest, but this Makefile does not define corresponding *_SOURCES entries for them. Automake will fail (or build empty targets) unless you add the source lists or remove these programs from bin_PROGRAMS.

[Copilot]

getOPTOUTValue is declared/used elsewhere with a const char* parameter (e.g., extern int getOPTOUTValue(const char *file_name); in src/dbus/rdkFwupdateMgr_handlers.c), but this mock defines it with no parameters. With C linkage this relies on ABI quirks and is not portable; update the stub signature to accept const char* file_name to match the real declaration.

[Copilot]

rdkFwupdateMgr_downloadFirmware prototype was changed to take handler_id as the first argument. There are existing unit tests (e.g., unittest/dbus_handlers.cpp) that still call the old argument order (firmwareName, downloadUrl, type, localFilePath, ...). Because all parameters are const gchar*, this won’t fail to compile but will pass the wrong values at runtime. Please update all callers/tests to the new order (or consider a backward-compatible wrapper if the old signature is still expected).

[Copilot]

The test declares and calls rdkFwupdateMgr_async_init_for_test, rdkFwupdateMgr_async_cleanup_for_test, and registry inspection helpers, but these symbols don’t exist anywhere in the current codebase (no matches in librdkFwupdateMgr/src). As a result this test won’t link. Either add these test-only entry points to the async implementation (guarded by #ifdef ASYNC_TEST), or refactor the test to drive the public API and observe behavior via existing hooks.

[Copilot]

This file switched from from rdkfw_test_helper import * to import rdkfw_test_helper, but the module still uses helper functions (rename_file, write_on_file, remove_file, etc.) as bare names. This will break at runtime with NameError unless all such calls are qualified (e.g., rdkfw_test_helper.rename_file(...)) or the original import style is restored.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 57 changed files in this pull request and generated 5 comments.

---

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

[Copilot]

Pull request overview

Copilot reviewed 60 out of 61 changed files in this pull request and generated 5 comments.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.