• State Resolved
  • Locked Locked
  • Replies 1 reply
  • Subscribers 52 subscribers
  • Views 338 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

BLE Bridge reference software design contains a synchronization bug

mike snyder
Intellectual 945 points

Hi e2e, just sharing my findings.

I had a thorough code review of the BLE bridge reference code, while diagnosing a failure involving random supervisor timeouts, intermittent disconnects, and unexpected serial numbers during connection events. It turns out, my problems were a symptom of a lack of synchronization between the circular buffer producer and consumer in the BLE Bridge reference software. 

The following line registers the packet parser as a callback, invoked by the osal to poll the UART buffers. 

  NPI_InitTransport(cSerialPacketParser);

The packet parser both consumes from the 128 byte UART ring buffer, and populates the 500 byte software 'circular buffer'. 

When the time comes to check the circular buffer for data to send ( SBP_SEND_EVT_PERIOD ), the BLE_Bridge_ProcessEvent function has no synchronization for circular_add and circular_diff. The heads and tails of the 500 byte buffer can be changed at any time. This is a legit race condition that was causing my system to fail. 

To fix this, I pass a null pointer to NPI_InitTransport, to eliminate the asynchronous nature of the problem. I do not want this parsing happening outside of my 'scheduled' operation. I also eliminated the 500 byte circular buffer, along with the very strange circular_add() and circular_diff() functions. I've increased the UART ring buffer from 128 bytes to 500 bytes and eliminated an unpredictable layer of indirection. 

I hope this can help someone else. Cheers

  • 0
    TI__Mastermind 34660 points

    Hello. I'm glad you got it working for your use case but I don't really understand the problem here and have some comments on your solution (or what I understand of it).  Maybe the demo is making some assumptions that are not valid in your use case. Hopefully you can comment below to clarify

    Firstly, the race condition:
       - The packet parser is reading data from the UART buffer, placing it in the circular buffer, and updating the serialBufferOffset to indicate where the last available valid byte is.
       - At SBP_SEND_EVT, BLE_Bridge_ProcessEvent() is checking for available bytes to send (by finding how many bytes are between buffer_tail (the index of last byte sent) and serialBufferOffset) and then sending these over-the-air.
       - serialBufferOffset is only modified in the packet parser. buffer_tail is only modified in the periodic event. The packet parser does not run in an ISR context; it is called from the HalUARTPoll() function. So there is no way for the packet parser to update serialBufferOffset while the periodic event is checking it to send data.
      - I suppose it would be possible for the 500 byte circular buffer to overflow if you are blasting UART data, have used  a longer connection interval, and there is a lot of RF interference causing dropped packets.  In this case, you would just need to increase the circular buffer size to fit the use case.

    Regarding your solution, I'm not entirely sure but it sounds like you are sending notifications directly from the serial parser after the header bytes have been stripped off.  While certainly simpler, the only drawback of this would be a potential loss in throughput.  If you are sending serial packets that are less then the maximum notification length (20 bytes), the notifications will be sent as they arrive over UART.
    I'm also not sure how you are handling failures to queue up a notification when there are no Tx queues available.  Maybe it just isn't an issue based on your serial data rate.  For example, if you are reading from the UART buffer but can't send a notification because all of the Tx queues are full, you'll need to cease processing (i.e. can't just continue retrying the notification) at some point so that the controller can process to send notifications and free up Tx queues.  This means that you will need to store the data that you read from the UART buffer somewhere to try to send a notification again later. This is the main reasoning for the circular buffer.

    Just some food for thought.