Skip to content

Thunder team review #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ManimaranRenganathan wants to merge 10 commits into
base: develop
Choose a base branch
from
+1,632 −10
Open

Thunder team review #212

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3324427
RDKEMW-9659 Implement DeviceSettings client library for FPD in rpc/cli
Jan 27, 2026
b6e760c
Modified the dsFPD-com.cpp code based on latest DeviceSettings plugin…
Jan 30, 2026
fdc4105
Modified the rpc/cliMakefile with conditionally exclude dsFPD.c when …
Feb 2, 2026
034e5db
Added export USE_WPE_THUNDER_PLUGIN=y in cov_build.sh
Feb 4, 2026
3ed996d
Added the DSCliLibStandaloneApp.cpp file for validate the FPD
Feb 6, 2026
d731afd
Install DSApp standalone app into /usr/bin folder
Feb 9, 2026
b18f613
Install the DSApp in install/bin path
Feb 9, 2026
e6d435d
Resolved compilation error
Feb 9, 2026
9c65c2d
Commentout libds.so initialization
Feb 9, 2026
ddf5841
Commentout libds common init
Feb 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ PKG_CHECK_MODULES([DBUS], [dbus-1])

AC_CHECK_LIB(gthread-2.0, g_thread_init)

# Thunder COM-RPC plugin support
AC_ARG_ENABLE([thunder-plugin],
AS_HELP_STRING([--enable-thunder-plugin], [Enable Thunder COM-RPC plugin support (default: no)]),
[enable_thunder_plugin=$enableval],
[enable_thunder_plugin=no])

AM_CONDITIONAL([USE_THUNDER_PLUGIN], [test "x$enable_thunder_plugin" = "xyes"])

AS_IF([test "x$enable_thunder_plugin" = "xyes"],
[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AC_DEFINE([USE_WPE_THUNDER_PLUGIN], ...) places the macro in cfg/config.h, but none of the rpc/cli sources include that header, and the Makefiles don’t -include it either (verified by searching for config.h). As a result, --enable-thunder-plugin may not actually enable the #ifdef USE_WPE_THUNDER_PLUGIN code. Ensure cfg/config.h is included in compilation (e.g., add -include $(top_builddir)/cfg/config.h or explicitly add -DUSE_WPE_THUNDER_PLUGIN when the option is enabled).

Suggested change
[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
CPPFLAGS="$CPPFLAGS -DUSE_WPE_THUNDER_PLUGIN"

Copilot uses AI. Check for mistakes.
AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],
Comment on lines +62 to +63
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--enable-thunder-plugin toggles compilation/linking against WPEFramework (Core/COM), but configure.ac does not validate that the required headers and libraries are present (no PKG_CHECK_MODULES / AC_CHECK_HEADERS / AC_CHECK_LIB for Thunder). Enabling this option will likely fail later during compilation/linking with a less actionable error. Add explicit checks and surface a clear configure-time failure or disable the option when dependencies are missing.

Suggested change
[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],
[PKG_CHECK_MODULES([THUNDER], [WPEFrameworkCore WPEFrameworkCOM],
[AC_DEFINE([USE_WPE_THUNDER_PLUGIN], [1], [Define to 1 to enable Thunder COM-RPC plugin support])
AC_MSG_NOTICE([Thunder COM-RPC plugin support enabled])],
[AC_MSG_ERROR([Thunder COM-RPC plugin support requested, but WPEFramework Core/COM (pkg-config modules WPEFrameworkCore and WPEFrameworkCOM) were not found. Install the appropriate development packages or re-run configure without --enable-thunder-plugin.])])],

Copilot uses AI. Check for mistakes.
[AC_MSG_NOTICE([Thunder COM-RPC plugin support disabled])])

# Checks for typedefs, structures, and compiler characteristics.
AC_TYPE_PID_T
AC_TYPE_SIZE_T
Expand Down
4 changes: 3 additions & 1 deletion cov_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ cd ${RDK_SOURCE_PATH}
export STANDALONE_BUILD_ENABLED=y
export DS_MGRS=$WORKDIR

export USE_WPE_THUNDER_PLUGIN=y

find $WORKDIR -iname "*.o" -exec rm -v {} \;
find $WORKDIR -iname "*.so*" -exec rm -v {} \;

echo "##### Triggering make"
make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"
make CFLAGS+='-fPIC -DDSMGR_LOGGER_ENABLED=ON -DRDK_DSHAL_NAME=\"libdshal.so\" -I${DS_IF_PATH}/include -I${DS_HAL_PATH} -I${DS_MGRS}/stubs -I${IARMBUS_PATH}/core -I${IARMBUS_PATH}/core/include -I${IARM_MGRS}/sysmgr/include -I${DS_MGRS}/ds/include -I${DS_MGRS}/rpc/include -I${POWER_IF_PATH}/include/ -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I${IARM_MGRS}/mfr/include/ -I${IARM_MGRS}/mfr/common -I${DEEPSLEEP_IF_PATH}/include -I${IARM_MGRS}/hal/include -I${IARM_MGRS}/power -I${IARM_MGRS}/power/include' LDFLAGS="-L/usr/lib/x86_64-linux-gnu/ -L/usr/local/include -lglib-2.0 -lIARMBus -lWPEFrameworkPowerController -ldshal"
Comment on lines 66 to 75
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script exports USE_WPE_THUNDER_PLUGIN=y, which will switch builds into Thunder mode, but the script only builds/installs stubs for libIARMBus.so and libWPEFrameworkPowerController.so (not libWPEFrameworkCore/libWPEFrameworkCOM) and doesn’t provide the corresponding headers. This is likely to break the Coverity build in environments without real Thunder libraries. Consider gating this export behind an environment check, or add the needed stubs/deps when enabling Thunder mode here.

Copilot uses AI. Check for mistakes.
24 changes: 21 additions & 3 deletions rpc/cli/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,20 @@ CFLAGS += -g -fPIC -D_REENTRANT -Wall
LIBNAME := dshalcli
LIBNAMEFULL := lib$(LIBNAME).so
INSTALL := $(PWD)/install
OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
OBJS += $(patsubst %.c,%.o,$(wildcard *.c))

# Conditional compilation: Thunder vs IARM
ifdef USE_WPE_THUNDER_PLUGIN
# Thunder mode - use dsFPD-com.cpp, exclude dsFPD.c
OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
OBJS += $(patsubst %.c,%.o,$(filter-out dsFPD.c,$(wildcard *.c)))
else
# IARM mode - use dsFPD.c, exclude dsFPD-com.cpp
OBJS := $(patsubst %.cpp,%.o,$(filter-out dsFPD-com.cpp,$(wildcard *.cpp)))
OBJS += $(patsubst %.c,%.o,$(wildcard *.c))
endif
Comment on lines +25 to +34
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Makefile switches object selection based on the make variable USE_WPE_THUNDER_PLUGIN, but it never defines the C/C++ preprocessor macro USE_WPE_THUNDER_PLUGIN. Since dsFPD-com.cpp is guarded by #ifdef USE_WPE_THUNDER_PLUGIN, enabling the make variable will compile an object with no symbols and likely break linking (because dsFPD.c is filtered out). Ensure the macro is passed in compiler flags when Thunder mode is enabled (or remove the #ifdef guard and rely solely on source selection).

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +34
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Makefile ifdef USE_WPE_THUNDER_PLUGIN is used to pick Thunder objects, but it does not add -DUSE_WPE_THUNDER_PLUGIN to the compiler flags. Because dsFPD-com.cpp is wrapped in #ifdef USE_WPE_THUNDER_PLUGIN, compiling it without the macro will omit all implementations and cause link errors. Ensure the macro is passed via CFLAGS/CPPFLAGS in this branch (or remove the source-level guard).

Copilot uses AI. Check for mistakes.

#OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
#OBJS += $(patsubst %.c,%.o,$(wildcard *.c))
Comment on lines +36 to +37
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commented-out lines should be removed rather than kept in the codebase. The version control system preserves the history, so keeping commented code creates clutter and confusion about what the intended implementation should be.

Suggested change
#OBJS := $(patsubst %.cpp,%.o,$(wildcard *.cpp))
#OBJS += $(patsubst %.c,%.o,$(wildcard *.c))

Copilot uses AI. Check for mistakes.
INCLUDE := -I$(PWD) \
-I$(PWD)/hal/include \
-I$(PWD)/rpc/include
Expand All @@ -31,13 +43,19 @@ INCLUDE += $(HAL_INCLUDE)

CFLAGS += $(INCLUDE)

# Conditional linking flags
ifdef USE_WPE_THUNDER_PLUGIN
LDLIBS := -lWPEFrameworkCore -lWPEFrameworkCOM
else
LDLIBS := -lIARMBus
endif

all: install
@echo "Build Finished...."

library: $(OBJS)
@echo "Building $(LIBNAMEFULL) ...."
$(CXX) $(OBJS) $(CFLAGS) -lIARMBus -shared -o $(LIBNAMEFULL)
$(CXX) $(OBJS) $(CFLAGS) $(LDLIBS) -shared -o $(LIBNAMEFULL)

%.o: %.cpp
@echo "Building $@ ...."
Expand Down
13 changes: 12 additions & 1 deletion rpc/cli/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@ INCLUDE_FILES = -I=$(includedir)/rdk/halif/ds-hal \
lib_LTLIBRARIES = libdshalcli.la
libdshalcli_la_CPPFLAGS = $(INCLUDE_FILES)
libdshalcli_la_CFLAGS = -g -fPIC -D_REENTRANT -Wall
libdshalcli_la_SOURCES = dsAudio.c dsclientlogger.c dsDisplay.c dsFPD.c dsHost.cpp dsVideoDevice.c dsVideoPort.c

# Conditional compilation for Thunder COM-RPC
if USE_THUNDER_PLUGIN
FPD_SOURCE = dsFPD-com.cpp
THUNDER_LIBS = -lWPEFrameworkCore -lWPEFrameworkCOM
else
FPD_SOURCE = dsFPD.c
THUNDER_LIBS =
endif

libdshalcli_la_SOURCES = dsAudio.c dsclientlogger.c dsDisplay.c $(FPD_SOURCE) dsHost.cpp dsVideoDevice.c dsVideoPort.c
libdshalcli_la_LIBADD = $(THUNDER_LIBS)
Comment on lines +32 to +42
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When USE_THUNDER_PLUGIN is enabled, dsFPD-com.cpp is compiled, but there’s no corresponding -DUSE_WPE_THUNDER_PLUGIN added to CPPFLAGS/CXXFLAGS. Because the file is wrapped in #ifdef USE_WPE_THUNDER_PLUGIN, this can result in an empty translation unit and missing dsFPD* symbols at link time. Add the define to the build flags under the conditional (or remove the #ifdef wrapper).

Copilot uses AI. Check for mistakes.
Loading
Loading