Skip to content

Commit e80da5b

Browse files
committed
framework/csimaanger: fix/Improve error handling and resource management
This commit enhances the Channel State Information (CSI) framework with improved error handling, resource management, and robustness: - Renamed CSIFW_INVALID_RAWDATA to CSIFW_ERROR_DATA_NOT_AVAILABLE for clearer error reporting - Fixed logic error in add_service function for proper buffer management - Implemented timeout handling in message queue operations to prevent indefinite blocking - Added consecutive failure detection mechanism to prevent system lockups - Improved resource management with proper pthread attribute cleanup - Added configurable timeout option for CSI data collection - Fixed preprocessor directive logic for custom device path configuration - Enhanced error reporting with more descriptive messages and callback notifications
1 parent c0882aa commit e80da5b

File tree

5 files changed

+75
-17
lines changed

5 files changed

+75
-17
lines changed

framework/include/csimanager/csifw_api.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ typedef enum _CSIFW_RES {
3232
CSIFW_ERROR_ALREADY_INIT_WITH_DIFFERENT_CONFIG = -8, /* ERROR: Service already initialized with different CSI_Configuration */
3333
CSIFW_ERROR_SERVICE_NOT_REGISTERED = -7, /* ERROR: Service not registered. Cannot start/stop/deinit */
3434
CSIFW_ERROR_WIFI_DIS_CONNECTED = -6, /* ERROR: WIFI WIFI_DISCONNECTED */
35-
CSIFW_INVALID_RAWDATA = -5, /* Invalid Raw Data */
35+
CSIFW_ERROR_DATA_NOT_AVAILABLE = -5, /* ERROR: CSI Data Not Available */
3636
CSIFW_INVALID_ARG = -4, /* Invalid argument */
3737
CSIFW_NOT_ENOUGH_SPACE = -3, /* read/write/other buffer has empty space less than required size */
3838
CSIFW_NO_MEM = -2, /* Memory allocation (malloc/calloc) failed */

framework/src/csimanager/CSIManager.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ csifw_service_info_t *add_service(csifw_context_t *p_csifw_ctx, service_callback
563563
}
564564

565565
if (get_service_idx(cid) != -1) {
566-
if (!p_csifw_ctx->parsed_buffptr) {
566+
if (p_csifw_ctx->parsed_buffptr) {
567567
free(p_csifw_ctx->parsed_buffptr);
568568
p_csifw_ctx->parsed_buffptr = NULL;
569569
}

framework/src/csimanager/CSIPacketReceiver.c

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <tinyara/wifi_csi/wifi_csi.h>
2626

2727
#define RCVR_TH_NAME "csifw_data_receiver_th"
28+
#define MAX_CONSECUTIVE_FAILURES 50
2829

2930
static inline CSIFW_RES open_driver(int *fd) {
3031
*fd = open(CONFIG_WIFICSI_CUSTOM_DEV_PATH, O_RDONLY);
@@ -69,15 +70,19 @@ static void *dataReceiverThread(void *vargp)
6970
size_t size;
7071
int csi_data_len = 0;
7172
struct wifi_csi_msg_s msg;
73+
int consecutive_failures = 0;
7274
int fd = p_csifw_ctx->data_receiver_fd;
7375
mqd_t mq_handle = p_csifw_ctx->mq_handle;
7476
unsigned char *get_data_buffptr = p_csifw_ctx->get_data_buffptr;
77+
struct timespec st_time = {0,};
78+
int timeout_count = 0;
7579

7680
while (!p_csifw_ctx->data_receiver_thread_stop) {
77-
size = mq_receive(mq_handle, (char *)&msg, sizeof(msg), &prio);
78-
if (size != sizeof(msg)) {
79-
CSIFW_LOGE("Interrupted while waiting for deque message from kernel %zu, errno: %d (%s)", size, get_errno(), strerror(get_errno()));
80-
} else {
81+
clock_gettime(CLOCK_REALTIME, &st_time);
82+
st_time.tv_sec += 1;
83+
size = mq_timedreceive(mq_handle, (char *)&msg, sizeof(msg), &prio, &st_time);
84+
if (size >= 0) {
85+
CSIFW_LOGD("Message received - msgId: %d, data_len: %d, size: %zu: %d", msg.msgId, msg.data_len, size);
8186
switch (msg.msgId) {
8287
case CSI_MSG_DATA_READY_CB:
8388
if (msg.data_len == 0 || msg.data_len > CSIFW_MAX_RAW_BUFF_LEN) {
@@ -87,12 +92,20 @@ static void *dataReceiverThread(void *vargp)
8792
csi_data_len = msg.data_len;
8893
len = readCSIData(fd, get_data_buffptr, csi_data_len);
8994
if (len < 0) {
95+
consecutive_failures++;
9096
CSIFW_LOGE("Skipping packet: error: %d", len);
97+
if (consecutive_failures >= MAX_CONSECUTIVE_FAILURES) {
98+
CSIFW_LOGE("CRITICAL: %d consecutive readCSIData failures detected!", consecutive_failures);
99+
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR_DATA_NOT_AVAILABLE, 0, NULL, 0);
100+
break;
101+
}
91102
continue;
92103
}
93104
if (len != csi_data_len - CSIFW_CSI_HEADER_LEN) {
94105
csi_data_len = len + CSIFW_CSI_HEADER_LEN;
95106
}
107+
consecutive_failures = 0;
108+
timeout_count = 0;
96109
p_csifw_ctx->CSI_DataCallback(CSIFW_OK, csi_data_len, get_data_buffptr, len);
97110
break;
98111

@@ -104,6 +117,26 @@ static void *dataReceiverThread(void *vargp)
104117
CSIFW_LOGE("Received unknown message ID: %d", msg.msgId);
105118
break;
106119
}
120+
} else if (size == -1) {
121+
if (errno == ETIMEDOUT) {
122+
timeout_count++;
123+
if (timeout_count >= CONFIG_CSI_DATA_TIMEOUT_SEC) {
124+
CSIFW_LOGE("Message receive timeout - no data available for [%d] seconds [msg size: %zu, errno: %d (%s)]",
125+
(p_csifw_ctx->data_receiver_thread_stop ? 1 : CONFIG_CSI_DATA_TIMEOUT_SEC), size, errno, strerror(errno));
126+
consecutive_failures = 0;
127+
timeout_count = 0;
128+
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR_DATA_NOT_AVAILABLE, 0, NULL, 0);
129+
}
130+
} else {
131+
CSIFW_LOGE("Message received size error - msgId: %d, data_len: %d, size: %zu, errno: %d (%s)", msg.msgId, msg.data_len, size, errno, strerror(errno));
132+
consecutive_failures++;
133+
if (consecutive_failures >= MAX_CONSECUTIVE_FAILURES) {
134+
CSIFW_LOGE("CRITICAL: %d consecutive Message received size error", consecutive_failures);
135+
consecutive_failures = 0;
136+
timeout_count = 0;
137+
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR, 0, NULL, 0);
138+
}
139+
}
107140
}
108141
}
109142
CSIFW_LOGD("[THREAD] : EXIT");
@@ -207,15 +240,7 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
207240
CSIFW_LOGE("Invalid context pointer (NULL)");
208241
return CSIFW_ERROR_NOT_INITIALIZED;
209242
}
210-
// allocate buffer for receiveing data from driver
211-
if (!p_csifw_ctx->get_data_buffptr) {
212-
p_csifw_ctx->get_data_buffptr = (unsigned char *)malloc(CSIFW_MAX_RAW_BUFF_LEN);
213-
if (!p_csifw_ctx->get_data_buffptr) {
214-
CSIFW_LOGE("Buffer allocation Fail.");
215-
return CSIFW_ERROR;
216-
}
217-
CSIFW_LOGD("Get data buffer allocation done, size: %d", CSIFW_MAX_RAW_BUFF_LEN);
218-
}
243+
219244
if (p_csifw_ctx->disable_required) {
220245
CSIFW_LOGD("WiFi reconnected: Disabling CSI config");
221246
csi_packet_receiver_set_csi_config(CSI_CONFIG_ENABLE);
@@ -229,6 +254,17 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
229254
}
230255
p_csifw_ctx->data_receiver_thread_stop = false;
231256

257+
// allocate buffer for receiveing data from driver
258+
if (!p_csifw_ctx->get_data_buffptr) {
259+
p_csifw_ctx->get_data_buffptr = (unsigned char *)malloc(CSIFW_MAX_RAW_BUFF_LEN);
260+
if (!p_csifw_ctx->get_data_buffptr) {
261+
csi_packet_receiver_set_csi_config(CSI_CONFIG_DISABLE);
262+
CSIFW_LOGE("Buffer allocation Fail.");
263+
return CSIFW_ERROR;
264+
}
265+
CSIFW_LOGD("Get data buffer allocation done, size: %d", CSIFW_MAX_RAW_BUFF_LEN);
266+
}
267+
232268
pthread_attr_t recv_th_attr;
233269
if (pthread_attr_init(&recv_th_attr) != 0) {
234270
CSIFW_LOGE("Failed to initialize receiver thread attributes - errno: %d (%s)", get_errno(), strerror(get_errno()));
@@ -245,11 +281,16 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
245281
csi_packet_receiver_set_csi_config(CSI_CONFIG_DISABLE);
246282
free(p_csifw_ctx->get_data_buffptr);
247283
p_csifw_ctx->get_data_buffptr = NULL;
284+
pthread_attr_destroy(&recv_th_attr);
248285
return CSIFW_ERROR;
249286
}
250287
if (pthread_setname_np(p_csifw_ctx->csi_data_receiver_th, RCVR_TH_NAME) != 0) {
251288
CSIFW_LOGE("Failed to set receiver thread name - errno: %d (%s)", get_errno(), strerror(get_errno()));
252289
}
290+
if (pthread_attr_destroy(&recv_th_attr) != 0) {
291+
CSIFW_LOGE("Failed to destroy thread attr - errno: %d (%s)", get_errno(), strerror(get_errno()));
292+
}
293+
253294
CSIFW_LOGD("CSI Data_Receive_Thread created (thread ID: %lu)", (unsigned long)p_csifw_ctx->csi_data_receiver_th);
254295
return res;
255296
}
@@ -334,7 +375,17 @@ CSIFW_RES csi_packet_receiver_stop_collect(void)
334375
CSIFW_LOGD("Sending dummy message to close blocking mq");
335376
struct wifi_csi_msg_s msg;
336377
msg.msgId = CSI_MSG_ERROR;
337-
mq_send(p_csifw_ctx->mq_handle, (FAR const char *)&msg, sizeof(msg), MQ_PRIO_MAX);
378+
struct timespec timeout;
379+
clock_gettime(CLOCK_REALTIME, &timeout);
380+
timeout.tv_sec += 1;
381+
int mq_send_ret = mq_timedsend(p_csifw_ctx->mq_handle, (FAR const char *)&msg, sizeof(msg), MQ_PRIO_MAX, &timeout);
382+
if (mq_send_ret != 0) {
383+
if (errno == ETIMEDOUT) {
384+
CSIFW_LOGE("Timeout sending dummy message - message queue may be full");
385+
} else {
386+
CSIFW_LOGE("Failed to send dummy message - errno: %d (%s)", errno, strerror(errno));
387+
}
388+
}
338389
} else {
339390
CSIFW_LOGE("Failed to close blocking mq as MQ_Discriptor is Invalid");
340391
}

framework/src/csimanager/Kconfig

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ config CSIFW
1313

1414
if CSIFW
1515

16+
config CSI_DATA_TIMEOUT_SEC
17+
int "CSI DATA TIMEOUT"
18+
default 15
19+
---help---
20+
CSI DATA TIMEOUT
21+
1622
menu "CSIFW Debug Logs"
1723

1824
config CSIFW_LOGS

os/drivers/wifi_csi/wifi_csi.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,9 @@ int wifi_csi_register(FAR const char *name, FAR struct wifi_csi_lowerhalf_s *dev
535535
char* path;
536536
#ifndef CONFIG_WIFICSI_CUSTOM_DEV_PATH
537537
path = "/dev/wificsi";
538-
#endif
538+
#else
539539
path = CONFIG_WIFICSI_CUSTOM_DEV_PATH;
540+
#endif
540541

541542
/* Allocate the upper-half data structure */
542543
upper = (FAR struct wifi_csi_upperhalf_s *)kmm_zalloc(sizeof(struct wifi_csi_upperhalf_s));

0 commit comments

Comments
 (0)