Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mk ble uart bug fixes with tx fifo enhancements #147

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mkirkhart
Copy link

While using this board support package for a project, I ran some tests using a modified version of the bleuart sketch and the Adafruit Bluefruit LE Connect app on my Samsung tablet, and found some problems with the BLEUart service:

  1. With TX buffering turned off and sending more than 20 characters from the Serial Monitor, only upto 20 would be transmitted, even the bleuart.write() function indicated all of them had been sent.

  2. With TX buffering turned off and sending more than 20 characters from the Serial Monitor, not all them would be transmitted, even the bleuart.write() function indicated all of them had been sent. Moreover, the ones that did get sent would be sent out-of-order somewhere past the 20th character.

  3. The TX buffer, when enabled, was set to a fixed size not settable by the user.

Here are some screenshots from the Arduino Serial Monitor and my Android tablet illustrating the problems.

In this case, 123456789012345678901234567890 (with a LF terminator) was sent. Notice how they are out of order after the 20th character ('0', or 0x30) was sent.
no-fixes-send-123456789012345678901234567890-hex

In this case, ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz (with a LF terminator) was sent. Notice how they are out of order after the 20th character ('T', or 0x54) was sent.
no-fixes-send-abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz-hex

Here, the Serial Monitor reports all characters (31 for 123456789012345678901234567890, 63 for ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz) were sent, even though they were not.
serial-monitor-no-fixes

Within my development branch (mk_BLE_UART_bug_fixes_with_TX_FIFO_enhancements), I have attempted to fix all three of these problems. I have run some tests with these changes, and thus far all 3 problems have been corrected.

Here are some screenshots from the Arduino Serial Monitor and my Android tablet illustrating the same tests after the fixes.

In these test cases, the TX buffer is turned off.

Only 20 characters, as expected, were sent.
with-fixes-send-12345678901234567890-hex-no-tx-buffer

Again, only 20 characters, as expected, were sent.
with-fixes-send-abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz-hex-no-tx-buffer

Notice how bleuart.write() now returns the actual number of characters sent.
serial-monitor-with-fixes-tx-buffer-off

In these test cases, the TX buffer is turned on.

21 characters (all of them) were sent.
with-fixes-12345678901234567890-hex

63 characters (all of them) were sent.
with-fixes-send-abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz-hex

Again, bleuart.write() now returns the actual number of characters sent.
serial-monitor-with-fixes-tx-buffer-on

Here the modified version of the bleuart sketch I used for testing:

/*********************************************************************`
 This is an example for our nRF52 based Bluefruit LE modules

 Pick one up today in the adafruit shop!

 Adafruit invests time and resources providing this open source code,
 please support Adafruit and open-source hardware by purchasing
 products from Adafruit!

 MIT license, check LICENSE for more information
 All text above, and the splash screen below must be included in
 any redistribution
*********************************************************************/
#include <bluefruit.h>

// BLE Service
BLEDis  bledis;
BLEUart bleuart;
BLEBas  blebas;

// Software Timer for blinking RED LED
SoftwareTimer blinkTimer;
static const int BLEUARTTXBufferSize = 1024;
static const int SerialBaudRate = 9600;
static const boolean EnableTxBuffering = true;

void setup()
{
  Serial.begin(SerialBaudRate);
  Serial.println("Bluefruit52 BLEUART Example");
  Serial.println("---------------------------\n");

  // Initialize blinkTimer for 1000 ms and start it
  blinkTimer.begin(1000, blink_timer_callback);
  blinkTimer.start();

  // Setup the BLE LED to be enabled on CONNECT
  // Note: This is actually the default behaviour, but provided
  // here in case you want to control this LED manually via PIN 19
  Bluefruit.autoConnLed(true);

  // Config the peripheral connection with maximum bandwidth 
  // more SRAM required by SoftDevice
  // Note: All config***() function must be called before begin()
  Bluefruit.configPrphBandwidth(BANDWIDTH_MAX);

  Bluefruit.begin();
  // Set max power. Accepted values are: -40, -30, -20, -16, -12, -8, -4, 0, 4
  Bluefruit.setTxPower(4);
  Bluefruit.setName("Bluefruit52");
  //Bluefruit.setName(getMcuUniqueID()); // useful testing with multiple central connections
  Bluefruit.setConnectCallback(connect_callback);
  Bluefruit.setDisconnectCallback(disconnect_callback);

  // Configure and Start Device Information Service
  bledis.setManufacturer("Adafruit Industries");
  bledis.setModel("Bluefruit Feather52");
  bledis.begin();

  // Configure and Start BLE Uart Service
  bleuart.begin();

  // turn on transmit data buffering
  if(EnableTxBuffering)
  {
    Serial.println("Enabling TX buffering");
    bleuart.bufferTXD(1);
  }

  // Start BLE Battery Service
  blebas.begin();
  blebas.write(100);

  // Set up and start advertigithub add screenshot to readmesing
  startAdv();

  Serial.println("Please use Adafruit's Bluefruit LE app to connect in UART mode");
  Serial.println("Once connected, enter character(s) that you wish to send");
}

void startAdv(void)
{
  // Advertising packet
  Bluefruit.Advertising.addFlags(BLE_GAP_ADV_FLAGS_LE_ONLY_GENERAL_DISC_MODE);
  Bluefruit.Advertising.addTxPower();

  // Include bleuart 128-bit uuid
  Bluefruit.Advertising.addService(bleuart);

  // Secondary Scan Response packet (optional)
  // Since there is no room for 'Name' in Advertising packet
  Bluefruit.ScanResponse.addName();
  
  /* Start Advertising
   * - Enable auto advertising if disconnected
   * - Interval:  fast mode = 20 ms, slow mode = 152.5 ms
   * - Timeout for fast mode is 30 seconds
   * - Start(timeout) with timeout = 0 will advertise forever (until connected)
   * 
   * For recommended advertising interval
   * https://developer.apple.com/library/content/qa/qa1931/_index.html   
   */
  Bluefruit.Advertising.restartOnDisconnect(true);
  Bluefruit.Advertising.setInterval(32, 244);    // in unit of 0.625 ms
  Bluefruit.Advertising.setFastTimeout(30);      // number of seconds in fast mode
  Bluefruit.Advertising.start(0);                // 0 = Don't stop advertising after n seconds  
}

void loop()
{
  // Forward data from HW Serial to BLEUART
  while (Serial.available())
  {
    // Delay to wait for enough input, since we have a limited transmission buffer
    delay(2);

    uint8_t buf[64];
    int count = Serial.readBytes(buf, sizeof(buf));
    int bytesWritten = bleuart.write( buf, count );

    Serial.print(count);
    Serial.print(" bytes to send, ");
    Serial.print(bytesWritten);
    Serial.println(" sent");
  }

  // Forward from BLEUART to HW Serial
  while ( bleuart.available() )
  {
    uint8_t ch;
    ch = (uint8_t) bleuart.read();
    Serial.write(ch);
  }

  // Request CPU to enter low-power mode until an event/interrupt occurs
  waitForEvent();
}

void connect_callback(uint16_t conn_handle)
{
  char central_name[32] = { 0 };
  Bluefruit.Gap.getPeerName(conn_handle, central_name, sizeof(central_name));

  Serial.print("Connected to ");
  Serial.println(central_name);
}

void disconnect_callback(uint16_t conn_handle, uint8_t reason)
{
  (void) conn_handle;
  (void) reason;

  Serial.println();
  Serial.println("Disconnected");
}github add screenshot to readme

/**
 * Software Timer callback is invoked via a built-in FreeRTOS thread with
 * minimal stack size. Therefore it should be as simple as possible. If
 * a periodically heavy task is needed, please use Scheduler.startLoop() to
 * create a dedicated task for it.
 * 
 * More information http://www.freertos.org/RTOS-software-timer.html
 */
void blink_timer_callback(TimerHandle_t xTimerID)
{
  (void) xTimerID;
  digitalToggle(LED_RED);
}

/**
 * RTOS Idle callback is automatically invoked by FreeRTOS
 * when there are no active threads. E.g when loop() calls delay() and
 * there is no bluetooth or hw event. This is the ideal place to handle
 * background data.
 * 
 * NOTE: FreeRTOS is configured as tickless idle mode. After this callback
 * is executed, if there is time, freeRTOS kernel will go into low power mode.
 * Therefore waitForEvent() should not be called in this callback.
 * http://www.freertos.org/low-power-tickless-rtos.html
 * 
 * WARNING: This function MUST NOT call any blocking FreeRTOS API 
 * such as delay(), xSemaphoreTake() etc ... for more information
 * http://www.freertos.org/a00016.html
 */
void rtos_idle_callback(void)
{
  // Don't call any other FreeRTOS blocking API()
  // Perform background task(s) here
}

Please take a look at these changes, and if they seem good, please incorporate them in the board support package.

Thanks for your help

Michael Kirkhart

Michael Kirkhart added 4 commits March 25, 2018 11:46
* BLEUart::bufferTXD() : set TX FIFO pointer to NULL after deletion to insure FIFO can be disabled and then re-enabled
* BLEUart::write() : insure function returns actual number of bytes written
* BLEUart::write() : fix for out-of-order transmissions if len > TX FIFO size
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant