adafruit / Adafruit_Mynewt

Apache Mynewt documentation and test project for the nRF5x family of BLE SoCs
MIT License
8 stars 3 forks source link

Convert apps/bleuart/src/fifo to Mynewt library/module #7

Closed microbuilder closed 8 years ago

microbuilder commented 8 years ago

The BLE UART helper currently uses the FIFO code from previous projects.

The code itself is well written and useful, but it would probably be better to stick to the buffers already available in Mynewt for statistic collection and debugging purposes, and just to improve our chances of getting future pull requests integrated into the core codebase.

Some of the buffer and queue options defined as part of Mynewt are:

These should be evaluated to see if there isn't something that matches the FIFO/buffering needs for BLE UART and other services.

Some related buffer elements worth exploring in the future are:

microbuilder commented 8 years ago

Alternatively, we can convert the FIFO.* code into a Mynewt library so that it can be maintained and reused, and improve our chances of having pull requests accepted.

microbuilder commented 8 years ago

Unfortunately, the minimum size for mempool is a word, so 4 bytes, which makes it very inefficient for UART style FIFOs that deal with one byte at a time. Adapting FIFO to be a library seems like the best approach.

microbuilder commented 8 years ago

This isn't documented, but might be a valid 'standard' alternative to the custom FIFO code: https://github.com/apache/incubator-mynewt-core/blob/master/libs/util/src/cbmem.c

hathach commented 8 years ago

cbmem is a variable-length FIFO, each time an item is pushed into the buffer, it will create an header (4 bytes) for each item. It still has an huge overhead considering the item size of bleuart is probably 1 or 2 most of the time.

microbuilder commented 8 years ago

Lets plan on creating a custom FIFO module later then, but leave this as is for now. I'll keep the issue open until the FIFO module is implemented though (in Mynewt library/module form).

hathach commented 8 years ago

OK, let's leave it as it is. I am trying to add both newtmgr (serial) & newtmgr over ble to our project.