bristlemouth / bm_protocol

Primary Bristlemouth firmware repository
https://www.bristlemouth.org/
Apache License 2.0
11 stars 8 forks source link

DevKit Payload UART wrapper (PLUART) should implement a queue for sensor lines, rather than a single buffer #16

Open evanShap opened 11 months ago

evanShap commented 11 months ago

Issue / request

Background / why

The PLUART namespace (defined here) adds some very convenient wrappers to the DevKit's Payload UART interface, which is the data interface for many of the serial connections the DevKit supports (UART, RS232, RS485, RS422, SDI-12).

There are (2) thread-safe methods exposed for users to access Rx data from their apps - PLUART::readLine and PLUART::readByte.

readLine can be used when the attached device is generating frames of serial data with reserved delimiting characters (a common example is ASCII text data with \ and/or \ being the frame delimiter). This is a very common pattern for legacy marine sensors so this is a very convenient wrapper to have.

The issue is that most of these sensors will have some multi-line data or command responses, the the potential for overwriting with the existing implementation results hairy-edge brittleness and a poor developer experience - the line buffer works most of the time. Works often enough to make it non-obvious what exactly is going wrong, but fails more than often enough to make it something a developer can just ignore. It's also susceptible to small timing changes in the app causing things to break, which can result in pernicious Heisenbugs.

related PR from legacy private repo for reference: https://github.com/wavespotter/bristlemouth/pull/465

SimenKH commented 10 months ago

As of the FreeRTOSConfig.h files I have read, it does seem like the protocol uses the CMSIS API for FreeRTOS.

So it could feasibly be solved by redefining the buffer in https://github.com/bristlemouth/bm_protocol/blob/3c10ebdedcb73a3b012b9545f5171ab0ba3a9ca8/src/lib/common/payload_uart.cpp to be a CMSIS message queue.

https://www.keil.com/pack/doc/CMSIS/RTOS/html/group__CMSIS__RTOS__Message.html https://www.keil.com/pack/doc/CMSIS/RTOS2/html/group__CMSIS__RTOS__Message.html

The message queues are thread safe, but the messages in the queue are consume-on-read.