RDKEMW-17873: HDMI-CEC Polaris HAL Migration by IshvarKR · Pull Request #57 · rdkcentral/hdmicec

IshvarKR · 2026-06-02T12:23:15Z

No description provided.

apatel859

Can you cimpile this code in normal build and vdevice build and see if you face any compilation issue?

github-actions · 2026-06-08T17:29:50Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Copilot

Pull request overview

This PR introduces a HAL abstraction + factory to switch the CCEC driver between a legacy C HAL backend (vHAL) and an Android Binder/AIDL backend (AidlHAL), and updates the build to produce/link the new backend libraries.

Changes:

Added HDMICecHal interface and HDMICecHalFactory to select AIDL vs legacy backend at runtime.
Implemented vHAL (legacy C driver wrapper) and AidlHAL (binder/AIDL implementation).
Updated DriverImpl to route all HAL operations through the new interface; updated Makefiles to build/link backend shared libraries.

Reviewed changes

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

Show a summary per file

File	Description
ccec/src/Makefile.am	Builds `libvHAL.la` + `libAidlHAL.la` and links them into `libRCEC.la`.
ccec/src/Makefile	Builds `libvHAL.so` + `libAidlHAL.so` and links them into `libRCEC.so`; adds binder/AIDL include/link plumbing.
ccec/src/factoryImpl/HDMICecHal.h	New abstract HAL interface used by `DriverImpl`.
ccec/src/factoryImpl/HDMICecHalFactory.h	Declares factory for backend selection.
ccec/src/factoryImpl/HDMICecHalFactory.cpp	Implements backend selection via binder service discovery.
ccec/src/factoryImpl/vHAL.h	Declares legacy backend wrapper.
ccec/src/factoryImpl/vHAL.cpp	Implements legacy backend wrapper via `HdmiCec*` C APIs.
ccec/src/factoryImpl/AidlHAL.h	Declares AIDL/binder backend.
ccec/src/factoryImpl/AidlHAL.cpp	Implements AIDL/binder backend + poll ACK emulation + frame length filtering.
ccec/src/DriverImpl.hpp	Stores `std::unique_ptr<HDMICecHal>` for the selected backend.
ccec/src/DriverImpl.cpp	Uses `HDMICecHal` for open/close/read/write operations and AIDL-specific behaviors.

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

Copilot

Pull request overview

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

Copilot

Pull request overview

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

Copilot

Pull request overview

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

Copilot

Pull request overview

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

+    if(emulateAckForPollFrames(buf, length)) {
+        *result = 0;  // HDMI_CEC_IO_SUCCESS
+        return 0;
+    }


+    android::binder::Status onMessageSent(const std::vector<uint8_t>& message, SendMessageStatus status) override {
+        if (mAidlHal) {
+            int result = (status == SendMessageStatus::ACK_STATE_0) ? 1 :
+                         (status == SendMessageStatus::ACK_STATE_1) ? 2 : 3; 
+            mAidlHal->dispatchTx(result);
+        }
+        return android::binder::Status::ok();
+    }


+    // Map AIDL SendMessageStatus to HAL error codes
+    *result = 0;  // HDMI_CEC_IO_SUCCESS
+    if (sendStatus == SendMessageStatus::ACK_STATE_0) {
+        *result = 1; // HDMI_CEC_IO_SENT_AND_ACKD
+    } else if (sendStatus == SendMessageStatus::ACK_STATE_1) {
+        *result = 2; // HDMI_CEC_IO_SENT_BUT_NOT_ACKD
+    } else if (sendStatus == SendMessageStatus::BUSY){
+        *result = 3; // HDMI_CEC_IO_SENT_FAILED
+        throw IOException();
+    }


+        CCEC_LOG(LOG_WARN,
+            "DriverImpl::getLogicalAddress no allocated LA from AIDL (statusOk=%d, count=%zu). Trying fallback allocation.\r\n",
+            status.isOk() ? 1 : 0,
+            addresses.size());


+// Macro definitions for internal use
+#define BINDER_VERSION       _IOWR('b', 9, struct binder_version)
+#define BINDER_WRITE_READ_V7 _IOWR('b', 1, struct binder_write_read_v7)
+#define BC_TRANSACTION_V7    _IOW('c', 0, struct binder_transaction_data_v7)
+


+    const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8;
+    void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0);
+    if (mapped_mem == MAP_FAILED) {


+# Binder libraries - link if they exist
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/libbinder.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -lbinder
+endif
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/libutils.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -lutils
+endif
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/liblog.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -llog
+endif


+# Calculate AIDL include path relative to workspace
+AIDL_GEN_DIR := $(shell cd ../../.. && pwd)/aidl/rdk-halif-aidl/gen/hdmicec/current
+AIDL_H_DIR := $(AIDL_GEN_DIR)/h
+BINDER_IDL_DIR := $(shell cd ../../.. && pwd)/aidl/rdk-halif-aidl/build-tools/linux_binder_idl
+BINDER_INCLUDE := $(BINDER_IDL_DIR)/android/native/libs/binder/include
+BINDER_NDK_INCLUDE := $(BINDER_IDL_DIR)/android/native/libs/binder/ndk/include_cpp
+BINDER_UTILS_INCLUDE := $(BINDER_IDL_DIR)/android/core/libutils/include
+BINDER_CUTILS_INCLUDE := $(BINDER_IDL_DIR)/android/core/libcutils/include
+BINDER_LOG_INCLUDE := $(BINDER_IDL_DIR)/android/logging/liblog/include
+BINDER_BASE_INCLUDE := $(BINDER_IDL_DIR)/android/libbase/include
+BINDER_BUILD_DIR := $(BINDER_IDL_DIR)/aidl-generator/out


 DriverImpl::DriverImpl() : status(CLOSED), nativeHandle(0)
 {
+	mHal = HDMICecHalFactory::Create();
 	CCEC_LOG( LOG_DEBUG, "Creating DriverImpl done\r\n");


apatel859 · 2026-06-19T12:30:25Z

+ * -------------------------------------------------------------------- */
+int HDMICecRdkVHAL::txAsync(int handle, const unsigned char *buf, int len)
+{
+    CCEC_LOG(LOG_INFO, "HDMICecRdkVHAL::txAsync handle=%d len=%d\r\n", handle, len);


We can combine two logs to one for all the APIs

apatel859 · 2026-06-19T13:15:44Z

+            status.isOk() ? 1 : 0,
+            addresses.size());
+        if (mAidlController != nullptr) {
+            const std::vector<int32_t> preferred = preferredLogicalAddressesForDeviceType(*logicalAddress);


pass correct device type

Copilot

Pull request overview

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

yuvaramachandran-gurusamy · 2026-06-22T16:09:14Z

+    CCEC_LOG(LOG_DEBUG, "HDMICecRdkVHAL::getPhysicalAddress handle=%d ret=%d addr=0x%x\r\n",
+             ret, (physicalAddress ? *physicalAddress : 0));


Fix Copilot comments

+    android::binder::Status onMessageSent(const std::vector<uint8_t>& message, SendMessageStatus status) override {
+        if (mAidlHal) {
+            int result = (status == SendMessageStatus::ACK_STATE_0) ? 1 :
+                         (status == SendMessageStatus::ACK_STATE_1) ? 2 : 3; 
+            mAidlHal->dispatchTx(result);
+        }
+        return android::binder::Status::ok();
+    }


+    *result = 0;  // HDMI_CEC_IO_SUCCESS
+    if (sendStatus == SendMessageStatus::ACK_STATE_0) {
+        *result = 1; // HDMI_CEC_IO_SENT_AND_ACKD
+    } else if (sendStatus == SendMessageStatus::ACK_STATE_1) {
+        *result = 2; // HDMI_CEC_IO_SENT_BUT_NOT_ACKD
+    } else if (sendStatus == SendMessageStatus::BUSY){
+        *result = 3; // HDMI_CEC_IO_SENT_FAILED
+        throw IOException();
+    }


+        if (mAidlController != nullptr) {
+            const std::vector<int32_t> preferred = preferredLogicalAddressesForDeviceType(*logicalAddress);
+            for (std::vector<int32_t>::const_iterator it = preferred.begin(); it != preferred.end(); ++it) {


+// Macro definitions for internal use
+#define BINDER_VERSION       _IOWR('b', 9, struct binder_version)
+#define BINDER_WRITE_READ_V7 _IOWR('b', 1, struct binder_write_read_v7)
+#define BC_TRANSACTION_V7    _IOW('c', 0, struct binder_transaction_data_v7)


+    CCEC_LOG(LOG_INFO, "[+] Binder protocol version detected: %d\n", version.protocol_version);
+
+    const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8;
+    void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0);


+                     factoryImpl/HDMICecHalFactory.cpp \
+                     factoryImpl/HDMICecRdkVHAL.cpp \
+                     factoryImpl/HDMICecAidlHAL.cpp \
+                     factoryImpl/ServiceManagerCheck.cpp


+# Binder libraries - link if they exist
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/libbinder.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -lbinder
+endif
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/libutils.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -lutils
+endif
+ifneq ($(wildcard $(BINDER_BUILD_DIR)/liblog.a),)
+LDFLAGS += -L$(BINDER_BUILD_DIR) -llog
+endif


 DriverImpl::DriverImpl() : status(CLOSED), nativeHandle(0)
 {
+	mHal = HDMICecHalFactory::Create();
 	CCEC_LOG( LOG_DEBUG, "Creating DriverImpl done\r\n");


yuvaramachandran-gurusamy · 2026-06-22T16:06:20Z

 		}

-		int err = HdmiCecOpen(&nativeHandle);
+		int err = mHal->open(&nativeHandle);


Add NULL check for mHal wherever it is used

yuvaramachandran-gurusamy · 2026-06-22T16:09:14Z

+    CCEC_LOG(LOG_DEBUG, "HDMICecRdkVHAL::getPhysicalAddress handle=%d ret=%d addr=0x%x\r\n",
+             ret, (physicalAddress ? *physicalAddress : 0));


Fix Copilot comments

Copilot

Pull request overview

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

+    class HalFactoryUtility {
+        enum class BackendType {
+            UNKNOWN,
+            LEGACY,
+            AIDL
+        };
+
+        static BackendType mBackendType;
+
+        bool isAidlServiceAvailable(const android::String16 &expectedServiceName)
+        {
+            CCEC_LOG(LOG_INFO, "isAidlServiceAvailable invoked\r\n");


+{
+    CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n");
+
+    if (HalFactoryUtility::isAidlServiceAvailable(IHdmiCec::serviceName().c_str())) {


+    const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8;
+    void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0);
+    if (mapped_mem == MAP_FAILED) {
+        CCEC_LOG(LOG_ERROR, "[-] Shared address space context instantiation failed\n");


+    CCEC_LOG(LOG_DEBUG, "HDMICecRdkVHAL::getPhysicalAddress handle=%d ret=%d addr=0x%x\r\n",
+             ret, (physicalAddress ? *physicalAddress : 0));


+int HDMICecAidlHAL::getPhysicalAddress(int handle, unsigned int *physicalAddress)
+{
+    if (physicalAddress != nullptr) {
+        *physicalAddress = 0;
+    }
+
+    CCEC_LOG( LOG_DEBUG, "HDMICecAidlHAL::getPhysicalAddress completed\r\n");
+
+    return 0;
+}


+                     factoryImpl/HDMICecHalFactory.cpp \
+                     factoryImpl/HDMICecRdkVHAL.cpp \
+                     factoryImpl/HDMICecAidlHAL.cpp \
+                     factoryImpl/ServiceManagerCheck.cpp

 libRCEC_la_LDFLAGS = -lpthread


Copilot

Pull request overview

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

+                     factoryImpl/HDMICecHalFactory.cpp \
+                     factoryImpl/HDMICecRdkVHAL.cpp \
+                     factoryImpl/HDMICecAidlHAL.cpp \
+                     factoryImpl/ServiceManagerCheck.cpp


+    CCEC_LOG(LOG_INFO, "[+] Binder protocol version detected: %d\n", version.protocol_version);
+
+    const size_t binder_map_size = (version.protocol_version == 7) ? BINDER_MMAP_SIZE_V7 : BINDER_MMAP_SIZE_V8;
+    void* const mapped_mem = mmap(nullptr, binder_map_size, PROT_READ, MAP_PRIVATE, binder_fd, 0);


+// Macro definitions for internal use
+#define BINDER_VERSION       _IOWR('b', 9, struct binder_version)
+#define BINDER_WRITE_READ_V7 _IOWR('b', 1, struct binder_write_read_v7)
+#define BC_TRANSACTION_V7    _IOW('c', 0, struct binder_transaction_data_v7)


+        bwr.write_size = write_size;
+        bwr.write_consumed = 0;
+        bwr.write_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.write_payload.data()));
+        bwr.read_size = read_size;
+        bwr.read_consumed = 0;
+        bwr.read_buffer = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(tx.read_payload.data()));


+            /* Probe with a 2-byte directed frame (GiveDevicePowerStatus) */
+            {
+                AutoLock lock_(mAidlMutex);
+                std::vector<uint8_t> probe;
+                probe.reserve(2);
+                probe.push_back(buf ? buf[0] : 0);
+                probe.push_back(0x8F); // GiveDevicePowerStatus
+
+                SendMessageStatus probeStatus = SendMessageStatus::BUSY;
+                android::binder::Status aidlStatus = mAidlController->sendMessage(probe, &probeStatus);
+                if (aidlStatus.isOk() && probeStatus == SendMessageStatus::ACK_STATE_0) {


+    /**
+     * @brief Get the physical address of the device.
+     *
+     * Legacy: calls HdmiCecGetPhysicalAddress().
+     * AIDL:   queries the physical address via binder.
+     *


+std::unique_ptr<IHDMICecHal> HDMICecHalFactory::Create()
+{
+    CCEC_LOG(LOG_INFO, "HDMICecHalFactory::Create invoked\r\n");
+
+    try {
+        if (HalFactoryUtility::isAidlServiceAvailable(android::String16(IHdmiCec::serviceName().c_str()))) {
+            CCEC_LOG(LOG_INFO, "HDMICecHalFactory: Aidl Service is available — using HDMICecAidlHAL\r\n");
+            return std::make_unique<HDMICecAidlHAL>();
+        }


vinodtel · 2026-06-25T05:23:42Z

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

vinodtel requested review from Kishoredarmaradje, Ranjeetha07, Sid2001-maker, anand-ky, apatel859 and vinodtel June 2, 2026 12:51

apatel859 reviewed Jun 2, 2026

View reviewed changes

Comment thread ccec/src/factoryImpl/HDMICecHalFactory.h Outdated

apatel859 reviewed Jun 2, 2026

View reviewed changes

Comment thread ccec/src/factoryImpl/HDMICecHalFactory.cpp Outdated

apatel859 requested changes Jun 2, 2026

View reviewed changes

Sid2001-maker force-pushed the feature/factoryImpl branch 2 times, most recently from 876022c to 1c009ec Compare June 3, 2026 09:34

IshvarKR force-pushed the feature/factoryImpl branch from 02757ea to ba51587 Compare June 9, 2026 10:51

vinodtel marked this pull request as ready for review June 9, 2026 16:55

vinodtel requested a review from a team as a code owner June 9, 2026 16:55

Copilot AI review requested due to automatic review settings June 9, 2026 16:55

Copilot started reviewing on behalf of vinodtel June 9, 2026 16:55 View session

vinodtel changed the title ~~Feature/factory impl~~ VDevice : HDMI-CEC Middleware component Jun 9, 2026

Copilot AI reviewed Jun 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 11, 2026 09:34

Copilot started reviewing on behalf of vinodtel June 11, 2026 09:34 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Sid2001-maker force-pushed the feature/factoryImpl branch from 174c2b5 to 1c06faf Compare June 11, 2026 10:17

Copilot AI review requested due to automatic review settings June 11, 2026 11:13

Copilot started reviewing on behalf of Sid2001-maker June 11, 2026 11:13 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 15, 2026 10:23

Copilot started reviewing on behalf of vinodtel June 15, 2026 10:23 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Sid2001-maker force-pushed the feature/factoryImpl branch from 32ad93b to 50d97a2 Compare June 15, 2026 10:51

vinodtel force-pushed the feature/factoryImpl branch from fe826af to 0982342 Compare June 18, 2026 18:36

Copilot AI review requested due to automatic review settings June 18, 2026 18:47

vinodtel force-pushed the feature/factoryImpl branch from 0982342 to f4eeb3a Compare June 18, 2026 18:47

Copilot started reviewing on behalf of vinodtel June 18, 2026 18:47 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Handle AIDL poll fallback for CEC device discovery

9e492e8

IshvarKR force-pushed the feature/factoryImpl branch from 93e20d3 to 9e492e8 Compare June 19, 2026 06:09

apatel859 reviewed Jun 19, 2026

View reviewed changes

Removed unwanted logs.

3521d79

Copilot AI review requested due to automatic review settings June 22, 2026 05:18

Copilot started reviewing on behalf of vinodtel June 22, 2026 05:18 View session

github-advanced-security AI found potential problems Jun 22, 2026

View reviewed changes

Comment thread ccec/src/factoryImpl/HDMICecRdkVHAL.cpp Fixed

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Pass devType through HAL getLogicalAddress for AIDL fallback

1e17370

yuvaramachandran-gurusamy requested changes Jun 22, 2026

View reviewed changes

vinodtel added 3 commits June 23, 2026 11:11

Changed year to 2026

0ef127c

Refactored the method isAidlServiceAvailable.

fe8d48b

Fixed review comments.

2cebd5d

Copilot AI review requested due to automatic review settings June 23, 2026 08:19

Copilot started reviewing on behalf of vinodtel June 23, 2026 08:19 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

vinodtel added 2 commits June 23, 2026 15:11

Added exception handling around AIDL HAL creation.

8f01cb6

Fixed build issue

e36b490

Copilot AI review requested due to automatic review settings June 23, 2026 11:30

Copilot started reviewing on behalf of vinodtel June 23, 2026 11:30 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

anand-ky changed the title ~~VDevice : HDMI-CEC Middleware component~~ RDKEMW-17873: HDMI-CEC Polaris HAL Migration Jun 24, 2026

vinodtel changed the base branch from develop to support/AIDL June 26, 2026 16:39

		CCEC_LOG(LOG_DEBUG, "HDMICecRdkVHAL::getPhysicalAddress handle=%d ret=%d addr=0x%x\r\n",
		ret, (physicalAddress ? *physicalAddress : 0));

Uh oh!

Conversation

IshvarKR commented Jun 2, 2026

Uh oh!

Uh oh!

Uh oh!

apatel859 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

apatel859 Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

apatel859 Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

yuvaramachandran-gurusamy Jun 22, 2026

github-actions Bot commented Jun 8, 2026 •

edited

Loading