Skip to content

Is it possible to suspend HardwareSerial to protect a global buffer? #8203

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

Closed
1 task done
techtoys opened this issue May 15, 2023 · 11 comments
Closed
1 task done

Is it possible to suspend HardwareSerial to protect a global buffer? #8203

techtoys opened this issue May 15, 2023 · 11 comments
Assignees
Labels
Type: Question Only question

Comments

@techtoys
Copy link

Related area

HardwareSerial feature

Hardware specification

ESP32 PICO D4

Is your feature request related to a problem?

Using HardwareSerial to get sensor data with a buffer written as uint8_t ser_buf[SIZE] in the global space. A task is running to scan the availability of data in that buffer in a period of 250ms. Because it is not a fast scanning rate, from time to time I can see that the buffer is corrupted by new data coming from the sensor data acquisition task.

Describe the solution you'd like

I wish to suspend HardwareSerial when there is new data in ser_buf[] and resume it after I have processed the data.

Describe alternatives you've considered

I have increased the scanning period to 50ms to improve data integrity. It seems to work but I am still worry about intermittent data corruption.

Additional context

No response

I have checked existing list of Feature requests and the Contribution Guide

  • I confirm I have checked existing list of Feature requests and Contribution Guide.
@techtoys techtoys added the Type: Feature request Feature request for Arduino ESP32 label May 15, 2023
@VojtechBartoska
Copy link
Contributor

@SuGlider please help with triage, thanks.

@SuGlider
Copy link
Collaborator

@techtoys - This task needs to send data to the sensor before receiving back the sensor response?

Or the sensor is programmed to send data every 50ms?

If possible, please provide a basic sketch, just to understand clearly what is your need and how it is currently done.

@lbernstone
Copy link
Contributor

Queues are a "protected global buffer", and can be used to signal data availability. See the example at https://github.com/espressif/arduino-esp32/tree/master/libraries/ESP32/examples/FreeRTOS/Queue

@SuGlider
Copy link
Collaborator

There are a few ways to do not loose data while receiving and processing UART data:

  1. Increase the UART internal data buffer
    size_t setRxBufferSize(size_t new_size); - this function shall be called before calling Serial.begin(). It will set a bigger internal data buffer. While it is not processed by the sketch, it will be store in an internal RingBuffer, therefore, it is protected and there is no overrun or data corruption.

Example:

void setup () {
  Serial1.setRxBufferSize(2048);   // it will keep up to 2048 bytes received from UART_1 even if it is not read
  Serial1.begin(115200);
}

char uart_processing_data[256];

void loop() {
  // read and process the UART_1 data
  Serial1.read(uart_processing_data, 256);
  // wait or do something else
  delay(100);
}
  1. React to any data received and process it in real time. This is like having an ISR for the Serial port.
    As soon as the data is availble, it will execute an user callback function.
    void onReceive(OnReceiveCb function, bool onlyOnTimeout = false); is used for that.

Example:

// User callback to process Serial data as soon as it is available -- process all incoming data here!
void Serial_Data_Processor() {
  // process the data the way it is necessary.
  // For instance, just read and print the data in a different UART port
  int incomingByte = 0; // for incoming serial data

  // check for all available incoming data from UART 2
  while (Serial2.available() > 0) {
    // read the incoming byte:
    incomingByte = Serial2.read();

    // prints the received data to the console UART
    Serial.print("I received: ");
    Serial.println((char)incomingByte);
  }
}

void setup () {
  // UART 2 i connected to the sensor
  Serial2.begin(9600);
  // Sets the User Callback function "Serial_Data_Processor()"
  Serial2.onReceive(Serial_Data_Processor);

  // UART 0 for seeing the data from UART 2
  Serial.begin(115200);
}


void loop() {
  // Nothing to be done here... It is all done in the callback function.
}
  1. Use both a bigger RX Buffer and a RX Callback function, combined.

@techtoys
Copy link
Author

techtoys commented May 16, 2023

2. React to any data received and process it in real time. This is like having an ISR for the Serial port.

I am using the callback method as you have mentioned in point #2 above but I think my practice of using unprotected global variables for message buffer static uint8_t rxMsgBuffer[128] and message available flag static volatile bool rxMsgPending below is not good. That may be the root of my problem.

My message format is fixed with this pattern:
[MSG_RX_HEADER] [length_byte] [command_id_byte][channel_number][message_content][checksum]
The first byte is a fixed header byte, and the 2nd byte indicates length of the message (channel_number + sizeof(message_content)) coming from the controller of sensor node.
The 3rd byte is the command ID.
The 4th byte is the channel number since I am dealing with some kind of RF.
The next byte onwards include the message content which length is not fixed.
The last byte is the checksum.

The sensor dispatches periodic messages at 4Hz and there are async. messages triggered by the user as well. As a result, there will be random messages interleave in the middle of periodic sensor messages in the period of 250ms.

Example message:
FEh 03h 42h 00h ABh CDh [checksum]
In this case, FEh is the header, 03h is the length of channel_number(00h) + sizeof(ABh+CDh) = 3
Command_id = 42h
Byte 00h indicates the channel number.

To receive the message, snippet of my code is like:

//SerialInterface.cpp
static uint8_t rxMsgBuffer[128];
static volatile bool rxMsgPending;
static void SerialReceiveCb(void)
{
  uint8_t index;
  uint8_t char_rx; //character received
  uint8_t checksum;
  if(Serial1.available()){
    char_rx = Serial1.read();
    if(char_rx==MSG_RX_HEADER){ ///Everything message stream starts with a header byte
      checksum = char_rx;   ///if it is a valid header, save the first byte to checksum variable
    } else{
      Serial.print(F("Exception: ")); Serial.println(__LINE__);
      HouseKeepingFcn();
    }
  }

  if(Serial1.available()){
   char_rx = Serial1.read();
   uint8_t length_byte= char_rx+2; ///+2 is an offset for command_id and the lenght_byte itself
   index = 0;
   do{
     rxMsgBuffer[index] = char_rx;
     checksum^=char_rx;
     char_rx = Serial1.read();
   }while(++index < length_byte);

   if(checksum == char_rx)
   {
     rxMsgPending = true;
   }else{
     Serial.print(F("Exception: ")); Serial.println(__LINE__);
     HouseKeepingFcn();
   }
  }
}

API of this module is:

uint8_t *SerialGetPendingMessage(void)
{
   if(rxMsgPending )
   {
     rxMsgPending = false;
     return rxMsgBuffer;
   }
   return NULL;
}

As you have mentioned:

  1. While it is not processed by the sketch, it will be store in an internal RingBuffer, therefore, it is protected and there is no overrun or data corruption.

Although the internal RingBuffer is protected, but my static uint8_t rxMsgBuffer[128] is not. That may cause a problem.

From a book (Embedded Systems Building Blocks) written by Jean J Labrosse, there is a chapter on Serial Interface in Mirium RTOS. Although it is not FreeRTOS, the principle should be the same. Extract from the book it states:

ISR CommRxISR(void)
{
   INT8U c;
   c = Get byte from RX port;
   if(Rx Ring Buffer is no full){
     Put received byte into Ring Buffer;
     if(received byte is the end-of-command byte) {
       Signal Rx Semaphore;
     }
   }
   Return from Interrupt;
}

INT8U CommGetCommand(INT8U *command, INT8U *nbytes)
{
   INT8U c;
   INT8U nrx;

   wait for command to be received (using semaphore with timeout)
   Disable interrupts
   c = Get bytes from Ring Buffer;
   while(c != end-of-command byte){
     *command++ = c;
     nrx++;
     c = Get byte from Ring Buffer;
   }
   Enable interrupts;
}

Please notice that there are Disable interrupts and Enable interrupts. I tried this with ESP32 dialect but there is a wdt timeout error. That is why I am asking if there is function to suspend/resume HardwareSerial in this thread.

@SuGlider
Copy link
Collaborator

Ok, I see. Maybe there are a few details about how the HardwareSerial::onReceive() callback works that can help you in order to let you make the best software design decisions:

ESP32 HardwareSerial is based on the UART IDF 4.4 code and its ISR internal driver.
IDF UART works this way by default:

1- Every byte that arrives to the UART port will be stored in the ESP32 internal hardware FIFO up to a limit, by default of 120 bytes. When this limit is reached IDF will copy those 120 bytes to the Arduno HardwareSerial internal buffer and, only then, the Arduino sketch will be able to read them or to know that there is data available by using something like Serial.available() API.

Note: HardwareSerial.begin(baudRate); will set this FIFO threshold to 1 if baudRate is lower than 57600bps. Threfore, it may be important to set this value after starting the Serial port in order to make sure it is correct for the application.

2- There is a second way to force the UART data to be copied to the HardwareSerial internal buffer, which is when a timout happens. This timeout is measured in number of UART symbols based on the actual baud rate of the serial port. Therefore, is just 4 bytes arrives, and nothing comes into the UART within TIMEOUT, the IDF driver will copy those 4 bytes to Arduino use it.

It is possible to control those two parameters using the HardwareSerial API:
void HardwareSerial::setRxTimeout(uint8_t symbols_timeout);
void HardwareSerial::setRxFIFOFull(uint8_t fifoBytes);

onReceive() also can be called only when all data is received and a TIMEOUT happens. This may help to make sure that the whole sensor message has arrived. By default, the callback is called on both situation (number of FIFO available byte and/or by Timeout).
void HardwareSerial::onReceive(OnReceiveCb function, bool onlyOnTimeout = false);

I think that you may try to use it with onlyOnTimeout as true. Like this:
Serial.onReceive(SerialReceiveCb, true);

@mrengineer7777 mrengineer7777 added Type: Question Only question and removed Type: Feature request Feature request for Arduino ESP32 labels May 16, 2023
@techtoys
Copy link
Author

techtoys commented May 17, 2023

will set this FIFO threshold to 1 if baudRate is lower than 57600bps

My baud rate is exactly 57600bps. Is the default FIFO 120bytes?

This timeout is measured in number of UART symbols based on the actual baud rate of the serial port.

Is UART symbol meaning an 11-bit time? I found it from https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/uart.html, it states:

uart_set_rx_timeout()
UART set threshold timeout for TOUT feature.
...

tout_thresh – This parameter defines timeout threshold in uart symbol periods. The maximum value of threshold is 126. tout_thresh = 1, defines TOUT interrupt timeout equal to transmission time of one symbol (~11 bit) on current baudrate. If the time is expired the UART_RXFIFO_TOUT_INT interrupt is triggered. If tout_thresh == 0, the TOUT feature is disabled.

@SuGlider
Copy link
Collaborator

SuGlider commented May 17, 2023

My baud rate is exactly 57600bps. Is the default FIFO 120bytes?

No, using 57600 or less, sets it to 1 in Serial.begin(). For your baud rate, it is set to 1.
It can be changed with Serial.setRxFIFOFull(uint8_t) from 1 up to 125 bytes.

NOTE: If onReceive() is set with onlyOnTimeout flagged true, the software will always force it to be 120, otherwise, the IDF UART driver doesn't work correctly.

Is UART symbol meaning an 11-bit time? I found it from https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/uart.html

Yes, UART symbol means 11 bits with 8E1 (1 start bit - 8 bits - 1 Parity bit - 1 stop bit).
For no Parity, it means about 10 bits.

@SuGlider
Copy link
Collaborator

You can experiment with all of it by using a callback like this:

void testingCallback() {
  // Here I supose that it is using UART 0 (Serial), but it is possible to set a different call back for each UART
  // by using, for instance, Serial1.onReceive() or Serial2.onReceive()
   
  int nBytes = Serial.available();
  Serial.printf("\nThere are %d bytes ready in the UART buffer\n", nBytes);
  Serial.println("Received data:");
  for (int i = 0; i < nBytes; i++) {
    char c = Serial.read();
    Serial.printf("'%c'[0x%x] ", c, c);
    if (((i + 1) % 10) == 0) Serial.println();
  }
}

Use a Serial Terminal/Monitor and send N blocks of 64 characters (like the one below) in one time to the UART port:
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!#

The call back will tell you how many bytes it has received. By default, for 57600 bps, it should tell you 1 by 1...
But printing the output takes time, therefore, it won't report saying that it has received 1 by 1, it will work for the first 2 or so bytes, then in the time it takes to print the report, all other bytes will be already in the FIFO and the timeout will happen as well, flushing all of them to the internal Arduino HardwareSerial buffer...

Everything is about timing and how things happen in parallel!

@SuGlider
Copy link
Collaborator

There are some examples about FIFO threshold, FIFO timeout and onReceive() in the
Arduino IDE Menu-->File-->Examples-->ESP32-->Serial-->xxxxxx

@techtoys
Copy link
Author

techtoys commented May 18, 2023

There are some examples about FIFO threshold, FIFO timeout and onReceive() in the
Arduino IDE Menu-->File-->Examples-->ESP32-->Serial-->xxxxxx

SuGlider, thanks a lot for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Question Only question
Projects
None yet
Development

No branches or pull requests

5 participants