New peek function for HardwareSerial

I was trying to help out a new user with his project, but the solution I though would work turned out to only work once.

Users post
The user wanted to send a string from his phone to control a RGB led(s), and the code I gave him was:

char * strings[3] = {
  "RED", "GREEN", "BLUE"};
byte i = 0;

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  if(Serial.available())
  {
    while(i < 3)
    {
      if(Serial.find(strings[i]))
      {
        Serial.print("Found ");
        Serial.println(strings[i]);
        i = 0;
        break;
      }
      Serial.println(i);
      i++;
    }
    if( i >= 3)
    {
      Serial.println("not found");
      i = 0;
    }
  }
}

This code should check the buffer and see if a string is found, and if not increment the index and check again.

It will find RED if typed in because it is in the first index of the strings array, but it won't find anything else. Upon further investigation, I found out that read() pops the ints (look at the .h and .cpp files for HWS) out of the buffer when it is called so this prevents my code from working. However another function peek() only looks at the first index of the buffer and doesn't move unless read is called.

I would like to add a new peek() function that allows the user to look through the buffer freely without popping anything out, thus allowing my code to work.

I have made an equivalent code that does what I want using the normal HWS library, but it is not very elegant. I'm sure I am not the only person this new function would help, so I am asking if it can be added.

New peek function, simple right?

int HardwareSerial::peek(uint8_t position)
{
  if (_rx_buffer->head == _rx_buffer->tail) {
    return -1;
  } else {
    return _rx_buffer->buffer[position];
  }
}

A somewhat related thing I have been thinking about (but not yet doing anything about) is the possibility of passing an array reference to HardwareSerial so it writes the data straight into my variable without needing to use Serial.read(). Then a user could have two or three "buffers" and alternate them to receive incoming data. The idea is to avoid the overhead of transferring the data from the HardwareSerial buffer to my buffer. It would also allow me to specify a larger buffer if necessary.

If the data is put directly into my variable I could "peek" to my heart's content with char x = myArray[indx];

...R

Well if you think about, all we need to do is access the buffer. The only problem that I can't seem to figure out is how to access the buffer

From HardwareSerial.cpp file.

struct ring_buffer
{
unsigned char buffer[SERIAL_BUFFER_SIZE];
volatile unsigned int head;
volatile unsigned int tail;
};

Is there any way to access this struct outside the library?

Is there any way to access this struct outside the library?

this works for version 1.5.6, (structure/permissions have been changed)

void setup() 
{
  Serial.begin(115200);
  Serial.println("Start ");
}

void loop() 
{ 
  int x = Serial._rx_buffer[1];
  Serial.println(x, DEC);
  int h = Serial._rx_buffer_head;
  Serial.println(h, DEC);
  int t = Serial._rx_buffer_tail;
  Serial.println(t, DEC);
  delay(1000);
}

@robtillaart
I just downloaded version 1.5.7, and I guess the permissions were changed back, because your code doesn't work anymore.

Edit:
I downloaded 1.5.6 r2 and it doesn't work either, I got the exact same error message :confused:

Error message:

Arduino: 1.5.7 (Windows 8 ), Board: "Arduino Uno"

In file included from C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/Arduino.h:221:0,
from sketch_aug16a.ino:1:
C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/HardwareSerial.h: In function 'void loop()':
C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/HardwareSerial.h:101:51: error: 'unsigned char HardwareSerial::_rx_buffer [64]' is protected
unsigned char _rx_buffer[SERIAL_RX_BUFFER_SIZE];
^
sketch_aug16a.ino:9:18: error: within this context
In file included from C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/Arduino.h:221:0,
from sketch_aug16a.ino:1:
C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/HardwareSerial.h:93:32: error: 'volatile rx_buffer_index_t HardwareSerial::_rx_buffer_head' is protected
volatile rx_buffer_index_t _rx_buffer_head;
^
sketch_aug16a.ino:11:18: error: within this context
In file included from C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/Arduino.h:221:0,
from sketch_aug16a.ino:1:
C:\Users\Andrew\Documents\arduino-1.5.7\hardware\arduino\avr\cores\arduino/HardwareSerial.h:94:32: error: 'volatile rx_buffer_index_t HardwareSerial::_rx_buffer_tail' is protected
volatile rx_buffer_index_t _rx_buffer_tail;
^
sketch_aug16a.ino:13:18: error: within this context

This report would have more information with
"Show verbose output during compilation"
enabled in File > Preferences.

Sorry,
then the only way seems to be to mod the libs... :frowning:

robtillaart:
Sorry,
then the only way seems to be to mod the libs... :frowning:

There is always a way. I'll have a crack and see what I can come up with.

Here is a solution. And you can access RX and TX buffers. Sorry for any headaches it may cause, and feel free to ask if you have questions.

template< typename Tag, typename Tag::type M >
  struct AccessMember{ 
    friend typename Tag::type get( Tag ){ return M; }
};

template< bool _RX >
struct HardwareSerial_Buffer{
  typedef uint8_t arr_t[ _RX ? SERIAL_RX_BUFFER_SIZE : SERIAL_TX_BUFFER_SIZE ];
  typedef arr_t HardwareSerial::*type;
  friend type get( HardwareSerial_Buffer );
};

template struct AccessMember< HardwareSerial_Buffer< true >, &HardwareSerial::_rx_buffer >;
template struct AccessMember< HardwareSerial_Buffer< false >, &HardwareSerial::_tx_buffer >;

uint8_t *GetRX( HardwareSerial &instance ){ return instance.*get( HardwareSerial_Buffer< true >() ); }
uint8_t *GetTX( HardwareSerial &instance ){ return instance.*get( HardwareSerial_Buffer< false >() ); }

And it is used very simply:

void setup() {
  Serial.begin( 9600 );
  
  uint8_t *rx_start = GetRX( Serial );
  uint8_t *tx_start = GetTX( Serial );
}

void loop() {}

These are awful. Typically the buffer used for serial IO at the hardware level would be separate from the buffer used for parsing, even though that is somewhat memory inefficient. Something like:

String token = Serial.readStringUntil('\n');
if (token == "RED") {
  // light red
} else if (token == "BLUE") {
  // light blue
} else if (token == "GREEN") {
  // light green
}

(Insert warning about String not being a great idea, given its current state, and maybe using Serial.readBytesUntil() and strcmp() instead.)

I made a solution to the guy's problem without any special functions, but it looks like crap.

char * strings[] = {
  "RED", "GREEN", "BLUE","ORANGE"};
  
byte i = 0, j=0;
unsigned long _startMillis = 0;
char data[20];
int index = 0;
boolean search = false, found = false;
#define arrSize(x) sizeof(x)/sizeof(x[0])

void setup()
{
  Serial.begin(115200);
  clear(20);
  Serial.println(arrSize(strings));
}

void loop()
{
  searchBuff(20);
}

void searchBuff(byte samples)
{
  if(Serial.available() > 0)
  {
    for(int S = 0, tmp; S < samples; S++)
    {
      tmp = Serial.read();
      if(tmp != '\n' || tmp != '\r' || tmp != '\0')
        data[S] = tmp; 
      delay(1); // this is needed, otherwise it will not show/store the correct data in the array.
    }
    search = true;
    found = false;
  }
  index = 0; 
  if(search)
  {
    j = 0;
    while(j < arrSize(strings))
    {
      for(byte i = 0; i < samples; i++)
      {
        if(data[i] != strings[j][index])
          index = 0; // reset index if any char does not match

        if( data[i] == strings[j][index])
        {
          if(index == strlen(strings[j])-1) // is true if all chars in the target match
          {
            Serial.print("Found ");
            Serial.println(strings[j]);
            index = 0;
            found = true;
            break;
          }
          else
            index++;
        }
      }//end of FOR loop
      if(found)
        search = false;
      j++; 
    }// End of WHILE loop
    if(!found)
    {
      Serial.println("not found");
      search = false;
    }
    
    clear(samples);
  }// End of Search
}

void clear(byte samples)
{
  int k = 0;
  while(k < samples)
  {
    data[k] = 0;
    k++;
  }
  search = false;
}

I also modified my HWS library with the peek function I wanted and it does work, but to clear the buffer, I need to use a loop to do multiple Serial.read(). I am going to see if there is a better way to clear the buffer, maybe its something really simple that I am not seeing, Idk.

I haven't looked at that code for ages but normally to clear a FIFO you just set the head pointer to = the tail pointer and (sometimes) clear a counter.


Rob

I'll try it when I get home tonight.

Yea... it didn't quite work like expected. making the head = tail (or vs) only makes Serial.available = false. I was looking for a better way to clean out the buffer without using a while/for loop. The peek function I added, worked like a charm, so I was happy with that.

HardwareSerial.cpp (13.2 KB)

HardwareSerial.h (3.4 KB)

making the head = tail (or vs) only makes Serial.available = false

Isn't that a cleared buffer?


Rob

westfw:
These are awful.

Its a solution, you may not be able to understand it, I'm sorry, however elaborate on your rant, otherwise you just fall in to the category of "one of those C guys".

@HazardsMind
It seems you still need to fix up the code pauls pointed out to you in the other thread.

If your reffering to the post where PaulS was taking about the delay I put in the code and proper delimiters, the new code doesn't need any of them. The peek function that I modified looks at everything in the Rx buffer. The buffer is never actually cleared like I thought it was, instead the tail value is decremented with each call of read() giving the appearance of it being cleared. All in all the buffer just rolls over once it collects more than 64 bytes.

elaborate on your rant, otherwise you just fall in to the category of "one of those C guys".

how about something so the code ends up looking like:

void loop() {
  char *p;
  int8_t cmd;
  int n;

  Serial.print(F("Enter command: "));

  int i = parseGetline();  // read a line of text

  do {
    enum {
      CMD_RED, CMD_GREEN, CMD_BLUE, CMD_RESET  // make sure this matches the string
    };
    cmd = parseKeyword(PSTR("red green blue reset")); // look for a command.

    if (cmd >= 0) {
      n = parseNumber();
    }
    switch (cmd) {
      case CMD_RED:
        red = n;
        break;
      case CMD_BLUE:
        blue = n;
        break;
      case CMD_GREEN:
        green = n;
        break;
      case CMD_RESET:
        red = green = blue = 0;
        break;
      case PARSER_EOL:
        Serial.print("RED = "); Serial.print(red);
        Serial.print(" GREEN = "); Serial.print(green);
        Serial.print(" BLUE= "); Serial.println(blue);
        break;
      default:
        Serial.println("Invalid command");
        break;
    }
  } while (cmd >= 0);
}

It lets you have dialogs like:

Enter command: red 123
RED = 123 GREEN = 0 BLUE= 0
Enter command: blue 789
RED = 123 GREEN = 0 BLUE= 789
Enter command: green 456
RED = 123 GREEN = 456 BLUE= 789
Enter command: reset
RED = 0 GREEN = 0 BLUE= 0
Enter command:

The current code isn't ready for publication, but it's about 600bytes of code, and 82bytes of RAM, which includes an 80byte line buffer and editing (backspace/del, ctrl-u, ctrl-r :-)) And no modifications to the arduino core.

The code I gave was just an example to show how the peek function would be useful.

I am also having similar problem explained in first post. I also viewed updated hardwareSerial files by hazardsMind. My question is can we modify the find function that actually peeks data instead of reading and clearing it from buffer? Check stream.cpp for find function.

You can do whatever you want but there are ways to solve the solution rather than mod the libraries.

Like this here. This uses no modified libraries, but it does require the string to be in a certain format.
ie R123G200B25 The code will see this string and split the letters and numbers accordingly and put them in their proper places.

**You dont need to have the string as I have it above but it does need to be a letter directly followed by a number R123. So you can just change red and blue with R123B25 and green will remain what it was.

Its nothing fancy, but it gets the job done in a minimal amount of lines.

#define IsWithin(x,a,b) ((x >= a) && (x <= b))

int Value = 0;
char Color = 'R';
const byte R_pin = 2, G_pin = 3, B_pin = 4;

void setup() 
{
  // put your setup code here, to run once:
  Serial.begin(115200);
  pinMode(R_pin, OUTPUT);
  pinMode(G_pin, OUTPUT);
  pinMode(B_pin, OUTPUT);
}

void loop()
{
  // put your main code here, to run repeatedly:
  if (Serial.available() > 0)
  {
    char tmp = Serial.read();
    if (IsWithin(tmp, '0', '9'))
      Value = (Value * 10) + tmp - '0';
    else if (IsWithin(tmp, 'a', 'z') || IsWithin(tmp, 'A', 'Z'))
    {
      Color = tmp;
      Value = 0;
    }

    switch (Color)
    {
      case 'R':
      case 'r':
        analogWrite(R_pin, Value);
        break;

      case 'G':
      case 'g':
        analogWrite(G_pin, Value);
        break;

      case 'B':
      case 'b':
        analogWrite(B_pin, Value);
        break;
    }
  }
}