Add CSI FW & Driver implementations #7045

dalvinder-s · 2025-11-18T08:53:28Z

No description provided.

pillip8282

LGTM

vibhor-m · 2025-11-24T14:07:37Z

framework/src/csimanager/utc/Makefile

@@ -0,0 +1,17 @@
+UTC_TYPE=CHECK


is this supported in public build?

vibhor-m · 2025-11-24T14:08:45Z

build/configs/rtl8730e/flat_dev_ddr_nand/defconfig

 #
 # CONFIG_EXAMPLES_TAHI is not set
-CONFIG_EXAMPLES_WIFIMANAGER_TEST=y
+# CONFIG_EXAMPLES_WIFIMANAGER_TEST is not set


we can avoid this change

enabling this config is causing build error. So did this change to go ahead and fix the wifi manager issue separately

Let's resolve build error and avoid changes apart from CSI. Please check in all build config files.

vibhor-m · 2025-11-24T14:10:01Z

os/include/tinyara/wifi_csi/wifi_csi.h

@@ -0,0 +1,276 @@
+/****************************************************************************
+ *
+ * Copyright 2024 Samsung Electronics All Rights Reserved.


2024 -> 2025

vibhor-m · 2025-12-26T15:52:36Z

build/configs/rtl8730e/flat_apps/defconfig

+CONFIG_WIFI_CSI_RTL8730E=y
+CONFIG_CSIFW=y
+# CONFIG_CSIFW_LOGS is not set
+# CONFIG_CSIFW_LOGE is not set


let's keep error logs always ON

vibhor-m · 2026-01-02T05:28:05Z

external/netutils/netlib_getipv4addr.c

+                if (sockfd >= 0) {
+                        struct ifreq req;
+                        if (strlen(ifname) >= IFNAMSIZ) {
+                                return ret;


we may require close(sockfd) before return.

vibhor-m · 2026-01-02T08:59:31Z

framework/src/csimanager/CSIFrameworkMain.c

+int start_csi_framework(csifw_context_t *p_ctx)
+{
+
+	if (!p_ctx) {


start_csi_framework is an internal API. The caller of this API has already checked 'csifw_context_t', is it really required to check in every function call.

vibhor-m · 2026-01-02T09:13:41Z

framework/src/csimanager/CSIFrameworkMain.c

+int csifw_task_Main(int argc, char *argv[])
+{
+
+	csifw_context_t *p_csifw_ctx = get_csifw_context();


This is not good.

static csifw_context_t *g_pcsifw_context = NULL;

static means this should not be referenced outside the file of declaration. However, get_csifw_context is a kind of backdoor access to this function.
either remove static from g_pcsifw_context, then you can access it without any function call.
OR
change implementation to not use it outside the file of declaration

vibhor-m · 2026-01-02T09:18:46Z

build/configs/rtl8730e/flat_dev_ddr_nand/defconfig

 #
 # CONFIG_EXAMPLES_TAHI is not set
-CONFIG_EXAMPLES_WIFIMANAGER_TEST=y
+# CONFIG_EXAMPLES_WIFIMANAGER_TEST is not set


Let's resolve build error and avoid changes apart from CSI. Please check in all build config files.

vibhor-m · 2026-01-02T09:37:25Z

framework/include/csimanager/csifw_api.h

+
+#ifdef __cplusplus				/* If this is a C++ compiler, end C linkage */
+}
+#endif


an empty line at end of file is guideline. please check all files

vibhor-m · 2026-01-02T09:47:39Z

framework/src/csimanager/utc/Makefile

@@ -0,0 +1,17 @@
+


We may remove this code. Automated utc is not supported in this repo.

vibhor-m · 2026-01-02T09:52:34Z

framework/src/csimanager/CSIFrameworkMain.c

+
+	if (!p_ctx) {
+		CSIFW_LOGE("Invalid context pointer (NULL)");
+		return 0;


at all other places, you are returning 0 as success and a -ve or +ve number as failure. Please follow same practice

vibhor-m · 2026-01-02T09:53:25Z

framework/src/csimanager/CSIFrameworkMain.c

+{
+
+	csifw_context_t *p_csifw_ctx = get_csifw_context();
+	//p_csifw_ctx is checked for null in start_csi_framework().


no need for such comments

vibhor-m · 2026-01-02T10:22:17Z

framework/src/csimanager/inc/csifw.h

+	struct icmp_echo_hdr *p_iecho;		/* Echo Header */
+	struct sockaddr *socketAddr;		/* Sokcet Address */
+	pthread_t csi_ping_thread;		/* CSI Ping Thread */
+	struct addrinfo *result;		/* Address Information */


'result' what does this mean?
Please use a better name which explain the purpose of this member variable.

vibhor-m · 2026-01-02T10:36:59Z

framework/src/csimanager/PingGenerator.c

+		return CSIFW_ERROR;
+	}
+
+	*socketAddr = rp->ai_addr;


line 316 & 320, Code reached this point considering socket API is successful.
is it possible 316 and 320 can fail

*socketAddr and (*socketAddr)->sa_family
are checked as their validity were not checked earlier.
Although rp is checked for NULL but ai_addr and sa_family can not be left unchecked.

vibhor-m · 2026-01-02T10:38:17Z

framework/src/csimanager/PingGenerator.c

+			break;
+		}
+	}
+	CSIFW_LOGD("Using socket %d for ping operations", *ping_socket);


socket is created, hence in all failure cases in this function it should be closed.

In case of failure/error
CSIFrameworkMain.c -> csifw_task_Main() handles the closing and resource cleaning.

vibhor-m · 2026-01-02T10:39:13Z

framework/src/csimanager/PingGenerator.c

+	tv.tv_sec = 1;
+	tv.tv_usec = 0;
+
+	if (setsockopt(*ping_socket, SOL_SOCKET, SO_RCVTIMEO, (struct timeval *)&tv, sizeof(struct timeval)) != ERR_OK) {


why RCV_TIMEOUT is set to 1 second here?
We are sending ping every few milliseconds.

We used the setsockopt() arguments same as it is used at other places.

vibhor-m · 2026-01-02T10:48:01Z

framework/src/csimanager/inc/csifw.h

+
+	unsigned char *get_data_buffptr;	/* Buffer to get data from driver */
+	unsigned int task_run_success;		/* Track if csifw_task initialization */
+	unsigned int data_receiver_fd;		/* Value from OPEN Driver */


why is it unsigned int, when using you are copying this in an int variable?
Can we not use it as int directly?

vibhor-m · 2026-01-05T04:24:24Z

Please improve commit messages and PR description & mention the actual implementation being added.

This commit establishes the foundational driver support for the CSI (Channel State Information) Framework. Key Changes: - Add CSI Framework configuration menu to os/Kconfig - Enable CONFIG_WIFI_CSI and CONFIG_CSIFW across all RTL8730E build configurations - Add RTL8730E CSI driver (rtl8730e_rtk_csi.c) to board build system - Update netlib headers to support CSI device operations - Configure CSI device path (/dev/wificsi) in defconfig files - Establishes framework for both HT and non-HT CSI data types

This commit implements the core CSI Framework management layer with comprehensive API and functionality: - Complete CSI Framework API (csifw_api.h) with service management functions - Core framework components: CSIManager, CSIPacketReceiver, CSINetworkMonitor, PingGenerator, CSIFrameworkMain - Service lifecycle management (register, start, stop, unregister) - Support for multiple CSI data types (HT_CSI_DATA, NON_HT_CSI_DATA, etc.) - Callback-based data delivery (raw and parsed data) - Network monitoring and automatic connection handling - Thread-safe operations with mutex protection API Features: - Service registration with configurable callbacks (raw/parsed data) - Multi-service support (up to 3 concurrent services) - Configurable data collection intervals - Automatic WiFi connection monitoring and recovery - Thread-safe operations with comprehensive error handling - Support for HT and non-HT CSI data types

This commit adds comprehensive sample applications demonstrating CSI Framework usage: Applications Added: - csifw_sample: Full-featured sample application with CLI interface - csifw_test: Framework validation and testing application csifw_sample Features: - Command-line interface with comprehensive parameter validation - Support for all CSI data types (HT_CSI_DATA, NON_HT_CSI_DATA, etc.) - Configurable callback types (raw data, parsed data, or both) - Adjustable data collection intervals (30ms minimum) - Configurable run duration for automated testing - Detailed usage documentation and examples Build Integration: - Added to apps/examples with complete Kconfig integration - Configurable via CONFIG_EXAMPLES_CSIFW_SAMPLE and CONFIG_EXAMPLES_CSIFW_TEST - Includes comprehensive README with usage examples and technical details

pillip8282 previously approved these changes Nov 18, 2025

View reviewed changes

dalvinder-s dismissed pillip8282’s stale review via d21a817 November 20, 2025 14:04

dalvinder-s force-pushed the master branch from 65c250c to d21a817 Compare November 20, 2025 14:04

vibhor-m reviewed Nov 24, 2025

View reviewed changes

framework/src/csimanager/utc/Makefile Outdated

@@ -0,0 +1,17 @@

UTC_TYPE=CHECK

Copy link

Contributor

vibhor-m Nov 24, 2025

Choose a reason for hiding this comment

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

is this supported in public build?

vibhor-m reviewed Nov 24, 2025

View reviewed changes

rajat-samsung force-pushed the master branch 5 times, most recently from ca01ba7 to 445febd Compare December 22, 2025 11:26

vibhor-m reviewed Jan 2, 2026

View reviewed changes

vibhor-m changed the title ~~Publish CSIFW code to openSource~~ Add CSI FW & Driver implementations Jan 5, 2026

rajat-samsung force-pushed the master branch from 445febd to 73cd915 Compare January 7, 2026 10:57

rajat-samsung force-pushed the master branch 4 times, most recently from 4f402c1 to e5b0ef5 Compare January 8, 2026 10:22

rajat-samsung force-pushed the master branch from e5b0ef5 to c91c6ce Compare January 8, 2026 11:29

Add CSI FW & Driver implementations #7045

Are you sure you want to change the base?

Add CSI FW & Driver implementations #7045

Uh oh!

Conversation

dalvinder-s commented Nov 18, 2025

Uh oh!

pillip8282 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vibhor-m commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants