RDKEMW-12974: Fix the rialto Component to support R5.3#439
RDKEMW-12974: Fix the rialto Component to support R5.3#439tabbas651 wants to merge 2 commits intordkcentral:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Rialto wrappers build and Thunder integration to support Thunder R5.3 by conditionally selecting the appropriate Thunder Core/COM dependencies and headers.
Changes:
- Adds conditional inclusion of Thunder error definitions for R5 builds in
ThunderWrapper.cpp. - Switches
wrappersCMake dependency discovery and link targets based onUSE_THUNDER_R5.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| wrappers/source/ThunderWrapper.cpp | Conditionally includes <core/Error.h> for R5 builds. |
| wrappers/CMakeLists.txt | Adds USE_THUNDER_R5 conditionals for package discovery and link libraries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wrappers/CMakeLists.txt
Outdated
| if (USE_THUNDER_R5) | ||
| set(WRAPPER_LIBS | ||
| rdkgstreamerutils | ||
| ocdm::ocdm | ||
| ${NAMESPACE}Core::${NAMESPACE}Core | ||
| ${NAMESPACE}COM::${NAMESPACE}COM | ||
| ${CMAKE_DL_LIBS}) |
There was a problem hiding this comment.
The CMake option USE_THUNDER_R5 is used to switch dependencies, but it is not propagated to the compiler (no add_compile_definitions(USE_THUNDER_R5) / target_compile_definitions(RialtoWrappers PRIVATE USE_THUNDER_R5)). As a result, code guarded by #ifdef USE_THUNDER_R5 won’t match the CMake dependency selection. Add a target-level compile definition when this branch is taken (or rename to a single source of truth).
wrappers/CMakeLists.txt
Outdated
| if (USE_THUNDER_R5) | ||
| find_package( ${NAMESPACE}Core REQUIRED ) | ||
| find_package( ${NAMESPACE}COM REQUIRED ) | ||
| else () | ||
| find_package( WPEFrameworkCore REQUIRED ) | ||
| find_package( WPEFrameworkCOM REQUIRED ) | ||
| endif() |
There was a problem hiding this comment.
Indentation/style for the newly added if (USE_THUNDER_R5) blocks is inconsistent with the surrounding CMake (e.g., if( style and indentation inside the enclosing if(...) block). Reformatting to match the file’s existing conventions will improve readability and reduce the chance of introducing scoping mistakes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Updated the compiler flags and added changes to support R5.3 Test Procedure: please refer the ticket comments Risks: Medium Signed-off-by: Thamim Razith Abbas Ali <tabbas651@cable.comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| find_package( ${NAMESPACE}Core REQUIRED ) | ||
| find_package( ${NAMESPACE}COM REQUIRED ) | ||
|
|
||
| set(WRAPPER_INCLUDES |
There was a problem hiding this comment.
${NAMESPACE} is not defined anywhere in this repo’s CMake files (search shows only this file), so these calls expand to find_package(Core) / find_package(COM) and the imported targets become Core::Core / COM::COM, which will break configuration/linking. Define NAMESPACE (e.g., via a cache variable set in the top-level CMakeLists) or replace this with explicit package/target names with a conditional fallback for Thunder vs WPEFramework.
tabbas651
changed the title
1 participant
Reason for change: Added compile flags to support the Thunder R5.3
Test Procedure: please refer the ticket comments
Risks: Medium