From c4e0e049e27622f01e93a4ac6beaec60d72d2364 Mon Sep 17 00:00:00 2001 From: Sharvil Nanavati Date: Sat, 26 Apr 2014 10:40:30 -0700 Subject: [PATCH] Clean up the public functions in userial.c. Change-Id: I6513296a8c606c49a4cadf196d89573948026e4a --- hci/include/userial.h | 86 ++++---------- hci/src/bt_hci_bdroid.c | 20 +++- hci/src/userial.c | 306 ++++++++++++++---------------------------------- hci/src/userial_mct.c | 33 ++---- 4 files changed, 137 insertions(+), 308 deletions(-) diff --git a/hci/include/userial.h b/hci/include/userial.h index f04111bbf..86984f6f8 100644 --- a/hci/include/userial.h +++ b/hci/include/userial.h @@ -16,13 +16,8 @@ * ******************************************************************************/ -/****************************************************************************** - * - * Filename: userial.h - * - * Description: Contains definitions used for serial port controls - * - ******************************************************************************/ +// This module manages the serial port over which HCI commands +// and data are sent/received. #pragma once @@ -50,67 +45,34 @@ typedef enum { USERIAL_PORT_18, } userial_port_t; -typedef enum { - USERIAL_OP_RXFLOW_ON, - USERIAL_OP_RXFLOW_OFF, -} userial_ioctl_op_t; - -/******************************************************************************* -** -** Function userial_init -** -** Description Initializes the userial driver -** -*******************************************************************************/ +// Initializes the userial module. This function should only ever be called once. +// It returns true if the module could be initialized, false if there was an error. bool userial_init(void); -/******************************************************************************* -** -** Function userial_open -** -** Description Open Bluetooth device with the port ID -** -*******************************************************************************/ +// Opens the given serial port. Returns true if successful, false otherwise. +// Once this function is called, the userial module will begin producing +// buffers from data read off the serial port. If you wish to pause the +// production of buffers, call |userial_pause_reading|. You can then resume +// by calling |userial_resume_reading|. This function returns true if the +// serial port was successfully opened and buffer production has started. It +// returns false if there was an error. bool userial_open(userial_port_t port); +void userial_close(void); -/******************************************************************************* -** -** Function userial_read -** -** Description Read data from the userial port -** -** Returns Number of bytes actually read from the userial port and -** copied into p_data. This may be less than len. -** -*******************************************************************************/ +// Reads a maximum of |len| bytes from the serial port into |p_buffer|. +// This function returns the number of bytes actually read, which may be +// less than |len|. This function will not block. uint16_t userial_read(uint16_t msg_id, uint8_t *p_buffer, uint16_t len); -/******************************************************************************* -** -** Function userial_write -** -** Description Write data to the userial port -** -** Returns Number of bytes actually written to the userial port. This -** may be less than len. -** -*******************************************************************************/ +// Writes a maximum of |len| bytes from |p_data| to the serial port. +// This function returns the number of bytes actually written, which may be +// less than |len|. This function may block. uint16_t userial_write(uint16_t msg_id, const uint8_t *p_data, uint16_t len); -/******************************************************************************* -** -** Function userial_close -** -** Description Close the userial port -** -*******************************************************************************/ -void userial_close(void); +// Pauses reading data from the serial port. No new buffers will be produced +// until |userial_resume_reading| is called. This function is idempotent. +void userial_pause_reading(void); -/******************************************************************************* -** -** Function userial_ioctl -** -** Description ioctl inteface -** -*******************************************************************************/ -void userial_ioctl(userial_ioctl_op_t op); +// Resumes reading data from the serial port if reads were previously paused. +// This function is idempotent. +void userial_resume_reading(void); diff --git a/hci/src/bt_hci_bdroid.c b/hci/src/bt_hci_bdroid.c index c5360adff..7cd422e83 100644 --- a/hci/src/bt_hci_bdroid.c +++ b/hci/src/bt_hci_bdroid.c @@ -27,6 +27,7 @@ #define LOG_TAG "bt_hci_bdroid" +#include #include #include #include "bt_hci_bdroid.h" @@ -332,12 +333,23 @@ static int transmit_buf(TRANSAC transac, char * p_buf, int len) /** Controls receive flow */ -static int set_rxflow(bt_rx_flow_state_t state) -{ +static int set_rxflow(bt_rx_flow_state_t state) { BTHCDBG("set_rxflow %d", state); - userial_ioctl(\ - ((state == BT_RXFLOW_ON) ? USERIAL_OP_RXFLOW_ON : USERIAL_OP_RXFLOW_OFF)); + switch (state) { + case BT_RXFLOW_ON: + userial_resume_reading(); + break; + + case BT_RXFLOW_OFF: + userial_pause_reading(); + break; + + default: + assert(false); + ALOGE("%s unexpected flow state: %d", __func__, state); + return BT_HC_STATUS_FAIL; + } return BT_HC_STATUS_SUCCESS; } diff --git a/hci/src/userial.c b/hci/src/userial.c index 00ddef954..ecf53be67 100644 --- a/hci/src/userial.c +++ b/hci/src/userial.c @@ -26,17 +26,19 @@ #define LOG_TAG "bt_userial" +#include #include #include #include #include #include +#include #include + #include "bt_hci_bdroid.h" #include "userial.h" #include "utils.h" #include "bt_vendor_lib.h" -#include #include "bt_utils.h" /****************************************************************************** @@ -53,10 +55,6 @@ #define USERIALDBG(param, ...) {} #endif -#ifndef ENABLE_USERIAL_TIMING_LOGS -#define ENABLE_USERIAL_TIMING_LOGS FALSE -#endif - #define MAX_SERIAL_PORT (USERIAL_PORT_3 + 1) #define READ_LIMIT (BTHC_USERIAL_READ_MEM_SIZE - BT_HC_HDR_SIZE) @@ -92,33 +90,6 @@ typedef struct static tUSERIAL_CB userial_cb; static volatile uint8_t userial_running = 0; -/****************************************************************************** -** Static functions -******************************************************************************/ - -#if defined(ENABLE_USERIAL_TIMING_LOGS) && (ENABLE_USERIAL_TIMING_LOGS==TRUE) - -static void log_userial_tx_timing(int len) -{ - #define USEC_PER_SEC 1000000L - static struct timespec prev = {0, 0}; - struct timespec now, diff; - unsigned int diff_us = 0; - unsigned int now_us = 0; - - clock_gettime(CLOCK_MONOTONIC, &now); - now_us = now.tv_sec*USEC_PER_SEC + now.tv_nsec/1000; - diff_us = (now.tv_sec - prev.tv_sec) * USEC_PER_SEC + (now.tv_nsec - prev.tv_nsec)/1000; - - ALOGW("[userial] ts %08d diff : %08d len %d", now_us, diff_us, - len); - - prev = now; -} - -#endif - - /***************************************************************************** ** Socket signal functions to wake up userial_read_thread for termination ** @@ -126,34 +97,40 @@ static void log_userial_tx_timing(int len) ** - signal_fds[0]: join fd_set in select call of userial_read_thread ** - signal_fds[1]: trigger from userial_close *****************************************************************************/ -static int signal_fds[2]={0,1}; +static int signal_fds[2] = { -1, -1 }; static uint8_t rx_flow_on = TRUE; -static inline int create_signal_fds(fd_set* set) -{ - if(signal_fds[0]==0 && socketpair(AF_UNIX, SOCK_STREAM, 0, signal_fds)<0) - { - ALOGE("create_signal_sockets:socketpair failed, errno: %d", errno); + +static inline int create_signal_fds(fd_set *set) { + if (signal_fds[0] == -1 && socketpair(AF_UNIX, SOCK_STREAM, 0, signal_fds) == -1) { + ALOGE("%s socketpair failed: %s", __func__, strerror(errno)); return -1; } + FD_SET(signal_fds[0], set); return signal_fds[0]; } -static inline int send_wakeup_signal(char sig_cmd) -{ + +static inline int send_wakeup_signal(char sig_cmd) { + assert(signal_fds[0] != -1); + assert(signal_fds[1] != -1); return send(signal_fds[1], &sig_cmd, sizeof(sig_cmd), 0); } -static inline char reset_signal() -{ + +static inline char reset_signal() { + assert(signal_fds[0] != -1); + assert(signal_fds[1] != -1); + char sig_recv = -1; recv(signal_fds[0], &sig_recv, sizeof(sig_recv), MSG_WAITALL); return sig_recv; } -static inline int is_signaled(fd_set* set) -{ + +static inline int is_signaled(fd_set *set) { + assert(signal_fds[0] != -1); + assert(signal_fds[1] != -1); return FD_ISSET(signal_fds[0], set); } - /******************************************************************************* ** ** Function select_read @@ -227,15 +204,6 @@ static int select_read(int fd, uint8_t *pbuf, int len) return ret; } -/******************************************************************************* -** -** Function userial_read_thread -** -** Description -** -** Returns void * -** -*******************************************************************************/ static void *userial_read_thread(void *arg) { int rx_length = 0; @@ -307,122 +275,60 @@ static void *userial_read_thread(void *arg) ** Userial API Functions *****************************************************************************/ -/******************************************************************************* -** -** Function userial_init -** -** Description Initializes the userial driver -** -*******************************************************************************/ bool userial_init(void) { USERIALDBG("userial_init"); memset(&userial_cb, 0, sizeof(tUSERIAL_CB)); userial_cb.fd = -1; - utils_queue_init(&(userial_cb.rx_q)); + utils_queue_init(&userial_cb.rx_q); return true; } -/******************************************************************************* -** -** Function userial_open -** -** Description Open Bluetooth device with the port ID -** -*******************************************************************************/ -bool userial_open(userial_port_t port) -{ - struct sched_param param; - int policy, result; - pthread_attr_t thread_attr; - int fd_array[CH_MAX]; - - USERIALDBG("userial_open(port:%d)", port); - - if (userial_running) - { - /* Userial is open; close it first */ - userial_close(); - utils_delay(50); +bool userial_open(userial_port_t port) { + if (port >= MAX_SERIAL_PORT) { + ALOGE("%s serial port %d > %d (max).", __func__, port, MAX_SERIAL_PORT); + return false; } - if (port >= MAX_SERIAL_PORT) - { - ALOGE("Port > MAX_SERIAL_PORT"); + if (!bt_vnd_if) { + ALOGE("%s no vendor interface, cannot open serial port.", __func__); return false; } - /* Calling vendor-specific part */ - if (bt_vnd_if) - { - result = bt_vnd_if->op(BT_VND_OP_USERIAL_OPEN, &fd_array); + if (userial_running) { + userial_close(); + utils_delay(50); + } - if (result != 1) - { - ALOGE("userial_open: wrong numbers of open fd in vendor lib [%d]!", - result); - ALOGE("userial_open: HCI UART expects only one open fd"); - bt_vnd_if->op(BT_VND_OP_USERIAL_CLOSE, NULL); - return false; - } + // Call in to the vendor-specific library to open the serial port. + int fd_array[CH_MAX]; + int num_ports = bt_vnd_if->op(BT_VND_OP_USERIAL_OPEN, &fd_array); - userial_cb.fd = fd_array[0]; - } - else - { - ALOGE("userial_open: missing vendor lib interface !!!"); - ALOGE("userial_open: unable to open UART port"); - return false; + if (num_ports > 1) { + ALOGE("%s opened wrong number of ports: got %d, expected 1.", __func__, num_ports); + goto error; } - if (userial_cb.fd == -1) - { - ALOGE("userial_open: failed to open UART port"); - return false; + userial_cb.fd = fd_array[0]; + if (userial_cb.fd == -1) { + ALOGE("%s unable to open serial port.", __func__); + goto error; } - USERIALDBG( "fd = %d", userial_cb.fd); - userial_cb.port = port; - pthread_attr_init(&thread_attr); - - if (pthread_create(&(userial_cb.read_thread), &thread_attr, \ - userial_read_thread, NULL) != 0 ) - { - ALOGE("pthread_create failed!"); - return false; - } - - if(pthread_getschedparam(userial_cb.read_thread, &policy, ¶m)==0) - { - policy = BTHC_LINUX_BASE_POLICY; -#if (BTHC_LINUX_BASE_POLICY != SCHED_NORMAL) - param.sched_priority = BTHC_USERIAL_READ_THREAD_PRIORITY; -#else - param.sched_priority = 0; -#endif - result = pthread_setschedparam(userial_cb.read_thread, policy, ¶m); - if (result != 0) - { - ALOGW("userial_open: pthread_setschedparam failed (%s)", \ - strerror(result)); - } + if (pthread_create(&userial_cb.read_thread, NULL, userial_read_thread, NULL)) { + ALOGE("%s unable to spawn read thread.", __func__); + goto error; } return true; + +error: + bt_vnd_if->op(BT_VND_OP_USERIAL_CLOSE, NULL); + return false; } -/******************************************************************************* -** -** Function userial_read -** -** Description Read data from the userial port -** -** Returns Number of bytes actually read from the userial port and -** copied into p_data. This may be less than len. -** -*******************************************************************************/ uint16_t userial_read(uint16_t msg_id, uint8_t *p_buffer, uint16_t len) { uint16_t total_len = 0; @@ -468,92 +374,58 @@ uint16_t userial_read(uint16_t msg_id, uint8_t *p_buffer, uint16_t len) return total_len; } -/******************************************************************************* -** -** Function userial_write -** -** Description Write data to the userial port -** -** Returns Number of bytes actually written to the userial port. This -** may be less than len. -** -*******************************************************************************/ -uint16_t userial_write(uint16_t msg_id, const uint8_t *p_data, uint16_t len) -{ - int ret, total = 0; +uint16_t userial_write(uint16_t msg_id, const uint8_t *p_data, uint16_t len) { UNUSED(msg_id); - while(len != 0) - { -#if defined(ENABLE_USERIAL_TIMING_LOGS) && (ENABLE_USERIAL_TIMING_LOGS==TRUE) - log_userial_tx_timing(len); -#endif - ret = write(userial_cb.fd, p_data+total, len); - total += ret; - len -= ret; + uint16_t total = 0; + while (len) { + ssize_t ret = write(userial_cb.fd, p_data + total, len); + switch (ret) { + case -1: + ALOGE("%s error writing to serial port: %s", __func__, strerror(errno)); + return total; + case 0: // don't loop forever in case write returns 0. + return total; + default: + total += ret; + len -= ret; + break; + } } - return ((uint16_t)total); + return total; } -/******************************************************************************* -** -** Function userial_close -** -** Description Close the userial port -** -*******************************************************************************/ -void userial_close(void) -{ - int result; - TRANSAC p_buf; - - USERIALDBG("userial_close(fd:%d)", userial_cb.fd); +void userial_close(void) { + assert(bt_vnd_if != NULL); + assert(bt_hc_cbacks != NULL); - if (userial_running) + // Join the reader thread if it's still running. + if (userial_running) { send_wakeup_signal(USERIAL_RX_EXIT); + int result = pthread_join(userial_cb.read_thread, NULL); + if (result) + ALOGE("%s failed to join reader thread: %d", __func__, result); + } - if ((result=pthread_join(userial_cb.read_thread, NULL)) < 0) - ALOGE( "pthread_join() FAILED result:%d", result); + // Ask the vendor-specific library to close the serial port. + bt_vnd_if->op(BT_VND_OP_USERIAL_CLOSE, NULL); - /* Calling vendor-specific part */ - if (bt_vnd_if) - bt_vnd_if->op(BT_VND_OP_USERIAL_CLOSE, NULL); + // Free all buffers still waiting in the RX queue. + // TODO: use list data structure and clean this up. + void *buf; + while ((buf = utils_dequeue(&userial_cb.rx_q)) != NULL) + bt_hc_cbacks->dealloc(buf, (char *) ((HC_BT_HDR *)buf + 1)); userial_cb.fd = -1; - - if (bt_hc_cbacks) - { - while ((p_buf = utils_dequeue (&(userial_cb.rx_q))) != NULL) - { - bt_hc_cbacks->dealloc(p_buf, (char *) ((HC_BT_HDR *)p_buf+1)); - } - } } -/******************************************************************************* -** -** Function userial_ioctl -** -** Description ioctl inteface -** -*******************************************************************************/ -void userial_ioctl(userial_ioctl_op_t op) -{ - switch(op) - { - case USERIAL_OP_RXFLOW_ON: - if (userial_running) - send_wakeup_signal(USERIAL_RX_FLOW_ON); - break; - - case USERIAL_OP_RXFLOW_OFF: - if (userial_running) - send_wakeup_signal(USERIAL_RX_FLOW_OFF); - break; +void userial_pause_reading(void) { + if (userial_running) + send_wakeup_signal(USERIAL_RX_FLOW_OFF); +} - default: - ALOGE("%s invalid operation: %d", __func__, op); - break; - } +void userial_resume_reading(void) { + if (userial_running) + send_wakeup_signal(USERIAL_RX_FLOW_ON); } diff --git a/hci/src/userial_mct.c b/hci/src/userial_mct.c index 3c4f09f1f..ecb25e3c4 100644 --- a/hci/src/userial_mct.c +++ b/hci/src/userial_mct.c @@ -407,29 +407,12 @@ void userial_close(void) userial_cb.fd[idx] = -1; } -/******************************************************************************* -** -** Function userial_ioctl -** -** Description ioctl inteface -** -*******************************************************************************/ -void userial_ioctl(userial_ioctl_op_t op) -{ - switch(op) - { - case USERIAL_OP_RXFLOW_ON: - if (userial_running) - send_wakeup_signal(USERIAL_RX_FLOW_ON); - break; - - case USERIAL_OP_RXFLOW_OFF: - if (userial_running) - send_wakeup_signal(USERIAL_RX_FLOW_OFF); - break; - - default: - ALOGE("%s invalid operation: %d", __func__, op); - break; - } +void userial_pause_reading(void) { + if (userial_running) + send_wakeup_signal(USERIAL_RX_FLOW_OFF); +} + +void userial_resume_reading(void) { + if (userial_running) + send_wakeup_signal(USERIAL_RX_FLOW_ON); } -- 2.11.0