Skip to content

framework/csimanager: fix/improve error handling and resource management #7174

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
rajat-samsung wants to merge 1 commit into
base: master
Choose a base branch
from
Open

framework/csimanager: fix/improve error handling and resource management #7174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion framework/include/csimanager/csifw_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ typedef enum _CSIFW_RES {
CSIFW_ERROR_ALREADY_INIT_WITH_DIFFERENT_CONFIG = -8, /* ERROR: Service already initialized with different CSI_Configuration */
CSIFW_ERROR_SERVICE_NOT_REGISTERED = -7, /* ERROR: Service not registered. Cannot start/stop/deinit */
CSIFW_ERROR_WIFI_DIS_CONNECTED = -6, /* ERROR: WIFI WIFI_DISCONNECTED */
CSIFW_INVALID_RAWDATA = -5, /* Invalid Raw Data */
CSIFW_ERROR_DATA_NOT_AVAILABLE = -5, /* ERROR: CSI Data Not Available */
CSIFW_INVALID_ARG = -4, /* Invalid argument */
CSIFW_NOT_ENOUGH_SPACE = -3, /* read/write/other buffer has empty space less than required size */
CSIFW_NO_MEM = -2, /* Memory allocation (malloc/calloc) failed */
Expand Down
4 changes: 2 additions & 2 deletions framework/src/csimanager/CSIManager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static void csifw_update_state_and_log(CSI_FRAMEWORK_STATE new_state)
return;
}
const int max_state = sizeof(csi_framework_state_strings) / sizeof(csi_framework_state_strings[0]);
if (new_state < CSI_FRAMEWORK_STATE_UNINITIALIZED || new_state >= max_state) {
if (new_state < CSI_FRAMEWORK_STATE_UNINITIALIZED || new_state >= max_state - 1) {
CSIFW_LOGE("Invalid state value: %d", new_state);
return;
}
Expand Down Expand Up @@ -563,7 +563,7 @@ csifw_service_info_t *add_service(csifw_context_t *p_csifw_ctx, service_callback
}

if (get_service_idx(cid) != -1) {
if (!p_csifw_ctx->parsed_buffptr) {
if (p_csifw_ctx->parsed_buffptr) {
free(p_csifw_ctx->parsed_buffptr);
p_csifw_ctx->parsed_buffptr = NULL;
}
Expand Down
79 changes: 65 additions & 14 deletions framework/src/csimanager/CSIPacketReceiver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <tinyara/wifi_csi/wifi_csi.h>

#define RCVR_TH_NAME "csifw_data_receiver_th"
#define MAX_CONSECUTIVE_FAILURES 50

static inline CSIFW_RES open_driver(int *fd) {
*fd = open(CONFIG_WIFICSI_CUSTOM_DEV_PATH, O_RDONLY);
Expand Down Expand Up @@ -69,15 +70,19 @@ static void *dataReceiverThread(void *vargp)
size_t size;
int csi_data_len = 0;
struct wifi_csi_msg_s msg;
int consecutive_failures = 0;
int fd = p_csifw_ctx->data_receiver_fd;
mqd_t mq_handle = p_csifw_ctx->mq_handle;
unsigned char *get_data_buffptr = p_csifw_ctx->get_data_buffptr;
struct timespec st_time = {0,};
int timeout_count = 0;

while (!p_csifw_ctx->data_receiver_thread_stop) {
size = mq_receive(mq_handle, (char *)&msg, sizeof(msg), &prio);
if (size != sizeof(msg)) {
CSIFW_LOGE("Interrupted while waiting for deque message from kernel %zu, errno: %d (%s)", size, get_errno(), strerror(get_errno()));
} else {
clock_gettime(CLOCK_REALTIME, &st_time);
st_time.tv_sec += 1;
size = mq_timedreceive(mq_handle, (char *)&msg, sizeof(msg), &prio, &st_time);
if (size >= 0) {
CSIFW_LOGD("Message received - msgId: %d, data_len: %d, size: %zu: %d", msg.msgId, msg.data_len, size);
switch (msg.msgId) {
case CSI_MSG_DATA_READY_CB:
if (msg.data_len == 0 || msg.data_len > CSIFW_MAX_RAW_BUFF_LEN) {
Expand All @@ -87,12 +92,20 @@ static void *dataReceiverThread(void *vargp)
csi_data_len = msg.data_len;
len = readCSIData(fd, get_data_buffptr, csi_data_len);
if (len < 0) {
consecutive_failures++;
CSIFW_LOGE("Skipping packet: error: %d", len);
if (consecutive_failures >= MAX_CONSECUTIVE_FAILURES) {
CSIFW_LOGE("CRITICAL: %d consecutive readCSIData failures detected!", consecutive_failures);
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR_DATA_NOT_AVAILABLE, 0, NULL, 0);
break;
}
continue;
}
if (len != csi_data_len - CSIFW_CSI_HEADER_LEN) {
csi_data_len = len + CSIFW_CSI_HEADER_LEN;
}
consecutive_failures = 0;
timeout_count = 0;
p_csifw_ctx->CSI_DataCallback(CSIFW_OK, csi_data_len, get_data_buffptr, len);
break;

Expand All @@ -104,6 +117,26 @@ static void *dataReceiverThread(void *vargp)
CSIFW_LOGE("Received unknown message ID: %d", msg.msgId);
break;
}
} else if (size == -1) {
if (errno == ETIMEDOUT) {
timeout_count++;
if (timeout_count >= CONFIG_CSI_DATA_TIMEOUT_SEC) {
CSIFW_LOGE("Message receive timeout - no data available for [%d] seconds [msg size: %zu, errno: %d (%s)]",
(p_csifw_ctx->data_receiver_thread_stop ? 1 : CONFIG_CSI_DATA_TIMEOUT_SEC), size, errno, strerror(errno));
consecutive_failures = 0;
timeout_count = 0;
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR_DATA_NOT_AVAILABLE, 0, NULL, 0);
}
} else {
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));
consecutive_failures++;
if (consecutive_failures >= MAX_CONSECUTIVE_FAILURES) {
CSIFW_LOGE("CRITICAL: %d consecutive Message received size error", consecutive_failures);
consecutive_failures = 0;
timeout_count = 0;
p_csifw_ctx->CSI_DataCallback(CSIFW_ERROR, 0, NULL, 0);
}
}
}
}
CSIFW_LOGD("[THREAD] : EXIT");
Expand Down Expand Up @@ -207,15 +240,7 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
CSIFW_LOGE("Invalid context pointer (NULL)");
return CSIFW_ERROR_NOT_INITIALIZED;
}
// allocate buffer for receiveing data from driver
if (!p_csifw_ctx->get_data_buffptr) {
p_csifw_ctx->get_data_buffptr = (unsigned char *)malloc(CSIFW_MAX_RAW_BUFF_LEN);
if (!p_csifw_ctx->get_data_buffptr) {
CSIFW_LOGE("Buffer allocation Fail.");
return CSIFW_ERROR;
}
CSIFW_LOGD("Get data buffer allocation done, size: %d", CSIFW_MAX_RAW_BUFF_LEN);
}

if (p_csifw_ctx->disable_required) {
CSIFW_LOGD("WiFi reconnected: Disabling CSI config");
csi_packet_receiver_set_csi_config(CSI_CONFIG_ENABLE);
Expand All @@ -229,6 +254,17 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
}
p_csifw_ctx->data_receiver_thread_stop = false;

// allocate buffer for receiveing data from driver
if (!p_csifw_ctx->get_data_buffptr) {
p_csifw_ctx->get_data_buffptr = (unsigned char *)malloc(CSIFW_MAX_RAW_BUFF_LEN);
if (!p_csifw_ctx->get_data_buffptr) {
csi_packet_receiver_set_csi_config(CSI_CONFIG_DISABLE);
CSIFW_LOGE("Buffer allocation Fail.");
return CSIFW_ERROR;
}
CSIFW_LOGD("Get data buffer allocation done, size: %d", CSIFW_MAX_RAW_BUFF_LEN);
}

pthread_attr_t recv_th_attr;
if (pthread_attr_init(&recv_th_attr) != 0) {
CSIFW_LOGE("Failed to initialize receiver thread attributes - errno: %d (%s)", get_errno(), strerror(get_errno()));
Expand All @@ -245,11 +281,16 @@ CSIFW_RES csi_packet_receiver_start_collect(void)
csi_packet_receiver_set_csi_config(CSI_CONFIG_DISABLE);
free(p_csifw_ctx->get_data_buffptr);
p_csifw_ctx->get_data_buffptr = NULL;
pthread_attr_destroy(&recv_th_attr);
return CSIFW_ERROR;
}
if (pthread_setname_np(p_csifw_ctx->csi_data_receiver_th, RCVR_TH_NAME) != 0) {
CSIFW_LOGE("Failed to set receiver thread name - errno: %d (%s)", get_errno(), strerror(get_errno()));
}
if (pthread_attr_destroy(&recv_th_attr) != 0) {
CSIFW_LOGE("Failed to destroy thread attr - errno: %d (%s)", get_errno(), strerror(get_errno()));
}

CSIFW_LOGD("CSI Data_Receive_Thread created (thread ID: %lu)", (unsigned long)p_csifw_ctx->csi_data_receiver_th);
return res;
}
Expand Down Expand Up @@ -334,7 +375,17 @@ CSIFW_RES csi_packet_receiver_stop_collect(void)
CSIFW_LOGD("Sending dummy message to close blocking mq");
struct wifi_csi_msg_s msg;
msg.msgId = CSI_MSG_ERROR;
mq_send(p_csifw_ctx->mq_handle, (FAR const char *)&msg, sizeof(msg), MQ_PRIO_MAX);
struct timespec timeout;
clock_gettime(CLOCK_REALTIME, &timeout);
timeout.tv_sec += 1;
int mq_send_ret = mq_timedsend(p_csifw_ctx->mq_handle, (FAR const char *)&msg, sizeof(msg), MQ_PRIO_MAX, &timeout);
if (mq_send_ret != 0) {
if (errno == ETIMEDOUT) {
CSIFW_LOGE("Timeout sending dummy message - message queue may be full");
} else {
CSIFW_LOGE("Failed to send dummy message - errno: %d (%s)", errno, strerror(errno));
}
}
} else {
CSIFW_LOGE("Failed to close blocking mq as MQ_Discriptor is Invalid");
}
Expand Down
6 changes: 6 additions & 0 deletions framework/src/csimanager/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ config CSIFW

if CSIFW

config CSI_DATA_TIMEOUT_SEC
int "CSI DATA TIMEOUT"
default 15
---help---
CSI DATA TIMEOUT

menu "CSIFW Debug Logs"

config CSIFW_LOGS
Expand Down
3 changes: 2 additions & 1 deletion os/drivers/wifi_csi/wifi_csi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,9 @@ int wifi_csi_register(FAR const char *name, FAR struct wifi_csi_lowerhalf_s *dev
char* path;
#ifndef CONFIG_WIFICSI_CUSTOM_DEV_PATH
path = "/dev/wificsi";
#endif
#else
path = CONFIG_WIFICSI_CUSTOM_DEV_PATH;
#endif

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