fix(ble): Make ble uart more useful

epicardium/ble/ble_main.c

@@ -34,6 +34,7 @@
 #include "cccd.h"
 #include "ess.h"
 #include "hid.h"
+#include "uart.h"
 
 #include "ble_api.h"
 #include "epicardium.h"
@@ -234,6 +235,7 @@ static const attsCccSet_t bleCccSet[BLE_NUM_CCC_IDX] =
   {HID_INPUT_REPORT_2_CH_CCC_HDL,       ATT_CLIENT_CFG_NOTIFY,    DM_SEC_LEVEL_NONE},    /* HIDAPP_IN_MOUSE_CCC_HDL */
   {HID_INPUT_REPORT_3_CH_CCC_HDL,       ATT_CLIENT_CFG_NOTIFY,    DM_SEC_LEVEL_NONE},    /* HIDAPP_IN_CONSUMER_CCC_HDL */
   {ESS_IAQ_CH_CCC_HDL,    ATT_CLIENT_CFG_NOTIFY,    DM_SEC_LEVEL_NONE},   /* BLE_ESS_IAQ_CCC_IDX */
+  {UART_TX_CH_CCC_HDL,    ATT_CLIENT_CFG_NOTIFY,    DM_SEC_LEVEL_NONE},   /* BLE_ESS_IAQ_CCC_IDX */
 };
 
 /**************************************************************************************************

epicardium/ble/cccd.h

@@ -13,6 +13,7 @@ enum
   HIDAPP_IN_MOUSE_CCC_HDL,              /*! HID Input Report characteristic for mouse inputs */
   HIDAPP_IN_CONSUMER_CCC_HDL,             /*! HID Input Report characteristic for consumer control inputs */
   BLE_ESS_IAQ_CCC_IDX,                    /*! Environmental sensing service, IAQ characteristic */
+  UART_TX_CH_CCC_IDX,
   BLE_NUM_CCC_IDX
 };
 

epicardium/ble/uart.c

+#include "uart.h"
+
 #include "modules/modules.h"
 
 #include "wsf_types.h"
@@ -11,24 +13,10 @@
 #include <string.h>
 #include <stdbool.h>
 
-#define UART_START_HDL 0x800            /*!< \brief Service start handle. */
-#define UART_END_HDL (UART_MAX_HDL - 1) /*!< \brief Service end handle. */
-
 /**************************************************************************************************
  Handles
 **************************************************************************************************/
 
-/*! \brief UART Service Handles */
-enum { UART_SVC_HDL = UART_START_HDL, /*!< \brief UART service declaration */
-       UART_RX_CH_HDL,                /*!< \brief UART rx characteristic */
-       UART_RX_HDL,                   /*!< \brief UART rx value */
-       UART_TX_CH_HDL,                /*!< \brief UART tx characteristic */
-       UART_TX_HDL,                   /*!< \brief UART tx value */
-       UART_TX_CH_CCC_HDL,            /*!< \brief UART tx CCCD */
-       UART_MAX_HDL                   /*!< \brief Maximum handle. */
-};
-/**@}*/
-
 /* clang-format off */
 static const uint8_t UARTSvc[] = {0x9E,0xCA,0xDC,0x24,0x0E,0xE5,0xA9,0xE0,0x93,0xF3,0xA3,0xB5,0x01,0x00,0x40,0x6E};
 static const uint16_t UARTSvc_len = sizeof(UARTSvc);
@@ -41,7 +29,10 @@ static const uint8_t uartTxCh[] = {ATT_PROP_READ | ATT_PROP_NOTIFY, UINT16_TO_BY
 static const uint16_t uartTxCh_len = sizeof(uartTxCh);
 static const uint8_t attUartTxChUuid[] = {0x9E,0xCA,0xDC,0x24,0x0E,0xE5, 0xA9,0xE0,0x93,0xF3,0xA3,0xB5,0x03,0x00,0x40,0x6E};
 
-static uint8_t ble_uart_tx_buf[128];
+static uint8_t uartValTxChCcc[] = {UINT16_TO_BYTES(0x0000)};
+static const uint16_t uartLenTxChCcc = sizeof(uartValTxChCcc);
+
+static uint8_t ble_uart_tx_buf[20];
 static uint16_t ble_uart_buf_tx_fill = 0;
 /* clang-format on */
 
@@ -96,15 +87,16 @@ static const attsAttr_t uartAttrCfgList[] = {
 	},
 	/* UART tx CCC descriptor */
 	{
-		.pUuid       = attCliChCfgUuid,
-		.pValue      = NULL,
-		.pLen        = NULL,
-		.maxLen      = 0,
-		.settings    = ATTS_SET_CCC,
-		.permissions = ATTS_PERMIT_WRITE | ATTS_PERMIT_WRITE_ENC |
-			       ATTS_PERMIT_WRITE_AUTH | ATTS_PERMIT_READ |
-			       ATTS_PERMIT_READ_ENC | ATTS_PERMIT_READ_AUTH,
+		.pUuid    = attCliChCfgUuid,
+		.pValue   = uartValTxChCcc,
+		.pLen     = (uint16_t *)&uartLenTxChCcc,
+		.maxLen   = sizeof(uartValTxChCcc),
+		.settings = ATTS_SET_CCC,
+		.permissions =
+			(ATTS_PERMIT_READ |
+			 ATTS_PERMIT_WRITE) // How about security?
 	},
+
 };
 
 dmConnId_t active_connection = 0;
@@ -119,63 +111,68 @@ static uint8_t UARTWriteCback(
 	attsAttr_t *pAttr
 ) {
 	active_connection = connId;
+	static bool was_r = false;
 
-	//printf("UARTWriteCback %d: ", len);
 	int i;
 	for (i = 0; i < len; i++) {
-		//printf("%c", pValue[i]);
+		if (pValue[i] == '\n' && !was_r) {
+			serial_enqueue_char('\r');
+		}
+		was_r = pValue[i] == '\r';
 		serial_enqueue_char(pValue[i]);
 	}
-	serial_enqueue_char('\r');
-	//printf("\n");
-
-#if 0
-  AttsSetAttr(UART_TX_HDL, len, pValue);
-  AttsHandleValueNtf(connId, UART_TX_HDL, len, pValue);
-#endif
 
 	return ATT_SUCCESS;
 }
 
 static int ble_uart_lasttick = 0;
 
-void ble_uart_write(uint8_t *pValue, uint8_t len)
+void ble_uart_flush(void)
 {
-	for (int i = 0; i < len; i++) {
-		if (pValue[i] >= 0x20 && pValue[i] < 0x7f) {
-			ble_uart_tx_buf[ble_uart_buf_tx_fill] = pValue[i];
-			ble_uart_buf_tx_fill++;
-		}
-
-		if (ble_uart_buf_tx_fill == 128 || pValue[i] == '\r' ||
-		    pValue[i] == '\n') {
-			if (ble_uart_buf_tx_fill > 0) {
-				if (active_connection) {
-					int x = xTaskGetTickCount() -
-						ble_uart_lasttick;
-					if (x < 100) {
-						/*
-						 * TODO: Ugly hack if we already
-						 *     send something recently.
-						 *     Figure out how fast we
-						 *     can send or use indications.
-						 */
-						vTaskDelay(100 - x);
-					}
-					AttsHandleValueNtf(
-						active_connection,
-						UART_TX_HDL,
-						ble_uart_buf_tx_fill,
-						ble_uart_tx_buf
-					);
-					ble_uart_lasttick = xTaskGetTickCount();
-				}
-				ble_uart_buf_tx_fill = 0;
+	if (ble_uart_buf_tx_fill > 0) {
+		if (active_connection) {
+			int x = xTaskGetTickCount() - ble_uart_lasttick;
+			if (x < 100) {
+				/*
+				 * TODO: Ugly hack if we already
+				 * sent something recently.
+				 * Use ATTS_HANDLE_VALUE_CNF instead.
+				 */
+				vTaskDelay(100 - x);
 			}
+			AttsHandleValueNtf(
+				active_connection,
+				UART_TX_HDL,
+				ble_uart_buf_tx_fill,
+				ble_uart_tx_buf
+			);
+			ble_uart_lasttick = xTaskGetTickCount();
 		}
+		ble_uart_buf_tx_fill = 0;
 	}
 }
 
+void ble_uart_write_char(uint8_t c)
+{
+	ble_uart_tx_buf[ble_uart_buf_tx_fill] = c;
+	ble_uart_buf_tx_fill++;
+
+	// TODO: increase buffer if configured MTU allows it
+	if (ble_uart_buf_tx_fill == sizeof(ble_uart_tx_buf)) {
+		ble_uart_flush();
+	}
+}
+
+void ble_uart_write(uint8_t *pValue, uint8_t len)
+{
+	for (int i = 0; i < len; i++) {
+		ble_uart_write_char(pValue[i]);
+	}
+
+	// TODO schedule timer in a few ms to flush the buffer
+	ble_uart_flush();
+}
+
 static attsGroup_t uartCfgGroup = {
 	.pAttr       = (attsAttr_t *)uartAttrCfgList,
 	.writeCback  = UARTWriteCback,

epicardium/ble/uart.h

+#pragma once
+
+#define UART_START_HDL 0x800            /*!< \brief Service start handle. */
+#define UART_END_HDL (UART_MAX_HDL - 1) /*!< \brief Service end handle. */
+
+/*! \brief UART Service Handles */
+enum { UART_SVC_HDL = UART_START_HDL, /*!< \brief UART service declaration */
+       UART_RX_CH_HDL,                /*!< \brief UART rx characteristic */
+       UART_RX_HDL,                   /*!< \brief UART rx value */
+       UART_TX_CH_HDL,                /*!< \brief UART tx characteristic */
+       UART_TX_HDL,                   /*!< \brief UART tx value */
+       UART_TX_CH_CCC_HDL,            /*!< \brief UART tx CCCD */
+       UART_MAX_HDL                   /*!< \brief Maximum handle. */
+};
+
+

epicardium/modules/serial.c

@@ -50,10 +50,7 @@ void serial_return_to_synchronous()
 	write_stream_buffer = NULL;
 }
 
-/*
- * API-call to write a string.  Output goes to both CDCACM and UART
- */
-void epic_uart_write_str(const char *str, size_t length)
+static void write_str_(const char *str, size_t length)
 {
 	if (length == 0) {
 		return;
@@ -129,6 +126,15 @@ void epic_uart_write_str(const char *str, size_t length)
 	}
 }
 
+/*
+ * API-call to write a string.  Output goes to both CDCACM, UART and BLE
+ */
+void epic_uart_write_str(const char *str, size_t length)
+{
+	ble_uart_write((uint8_t *)str, length);
+	write_str_(str, length);
+}
+
 static void serial_flush_from_isr(void)
 {
 	uint8_t rx_data[32];
@@ -196,7 +202,7 @@ static void serial_flush_from_thread(void)
 		taskEXIT_CRITICAL();
 
 		cdcacm_write((uint8_t *)&rx_data, received_bytes);
-		ble_uart_write((uint8_t *)&rx_data, received_bytes);
+		//ble_uart_write((uint8_t *)&rx_data, received_bytes);
 	} while (received_bytes > 0);
 }
 
@@ -243,6 +249,9 @@ int epic_uart_read_str(char *buf, size_t cnt)
 	return i;
 }
 
+/*
+ * Write a string.  Output goes to both CDCACM and UART
+ */
 long _write_epicardium(int fd, const char *buf, size_t cnt)
 {
 	/*
@@ -252,12 +261,12 @@ long _write_epicardium(int fd, const char *buf, size_t cnt)
 	size_t i, last = 0;
 	for (i = 0; i < cnt; i++) {
 		if (buf[i] == '\n') {
-			epic_uart_write_str(&buf[last], i - last);
-			epic_uart_write_str("\r", 1);
+			write_str_(&buf[last], i - last);
+			write_str_("\r", 1);
 			last = i;
 		}
 	}
-	epic_uart_write_str(&buf[last], cnt - last);
+	write_str_(&buf[last], cnt - last);
 	return cnt;
 }