1

I am trying to implement the simplest circular buffer in Arduino. I think I am able to implement it because my push and pop functions do not return any errors. However, when I want to print the data in the buffer using Serial.print(), the code goes into an infinite loop. Here is my code:

#include <stdio.h>

typedef struct circular_buffer
{
    int buffer_arr[5];
    int head;
    int tail;
} circular_buffer;

void setup()
{  
  Serial.begin(9600); 
  Serial.println("Circular buffer started");
}

void loop()
{  
  circular_buffer *cb;
  cb_init(cb);
  if(Serial.available()){
    char instruction = Serial.read();
    Serial.println(instruction);
    if(instruction == 'a'){
      if(Serial.available()){
        char instruction = Serial.read();
        Serial.println("here");
      }
      char key = Serial.read();
      cb_push_front(cb, key);
      Serial.print(String(key));
      Serial.println("    is entered");
    }
    else if(instruction == 'd'){
      cb_pop_back(cb);
      Serial.println("last item removed");
    }
//    for(int i=0; i<5; i++){
//      Serial.print(cb->buffer_arr[i]);
//      
//    }
    Serial.println("-");
  }
}



void cb_init(circular_buffer *cb)
{
    // cb->buffer_arr = {0,0,0,0,0};
    for(int i=0; i<5; i++){
      cb->buffer_arr[i] = 'c';
//      Serial.print(sizeof(*cb));
    }
//      Serial.print(cb->buffer_arr[0]);
    cb->head = 0;
    cb->tail = 0;
}


void cb_push_front(circular_buffer *cb, char item)
{
    cb->head++;
    cb->buffer_arr[cb->head] = item;
}

void cb_pop_back(circular_buffer *cb)
{
    cb->tail++;
    cb->buffer_arr[cb->tail] = 0;
}

The for loop which I commented out is the problematic one - or at least what I believe. My problem can be very much related to my poor C-language skills. I thought that cb->buffer_arr = {0,0,0,0,0}; code piece in the initialization function should have worked (thinking very much Python-ic way) but it didn't.

So, if anyone can see the issue and help me fixing it, I'd be very much appreciated. Thanks in advance.

EDIT

After Majenko's answer I have updated the code like that. I still have issues how to print though. I am not sure whether I am printing the address of the circular buffer as ASCII in this way. I tried to pass the cb to a print function in two ways. None seems to work, though...

#include <stdio.h>

typedef struct circular_buffer
{
    int buffer_arr[5];
    int head;
    int tail;
} circular_buffer;

circular_buffer cb;

void setup()
{  
  Serial.begin(9600); 
  cb_init(cb);
  Serial.println("Circular buffer started");
}

void loop()
{  
  if(Serial.available()){
    char instruction = Serial.read();
    Serial.println(instruction);
    if(instruction == 'a'){
      Serial.print("a pressed");  
      cb_push_front(cb, instruction);
    }
    else if(instruction == 'd'){
      Serial.print("d pressed");
    }
    else{
      Serial.print("UNKNOWN");
    }
    Serial.println("-");
  }
}



void cb_init(circular_buffer &cb)
{
    for(int i=0; i<5; i++){
      cb.buffer_arr[i] = 'c';
    }
    Serial.println("here init");
    cb_print1(&cb);
    cb_print2(cb);
    cb.head = 0;
    cb.tail = 0;
}


void cb_push_front(circular_buffer &cb, char item)
{
    cb.head++;
    cb.buffer_arr[cb.head] = item;

    cb_print1(&cb);
    cb_print2(cb);
}

void cb_pop_back(circular_buffer &cb)
{
    cb.tail++;
    cb.buffer_arr[cb.tail] = 0;
}

void cb_print1(circular_buffer *cb)
{
  Serial.println("here print1");
  for(int i=0; i<5; i++){
    Serial.print(cb->buffer_arr[i]);
    
  }
}

void cb_print2(circular_buffer cb)
{
  Serial.println("here print2");
  for(int i=0; i<5; i++){
    Serial.print(cb.buffer_arr[i]);
    
  }
}

And the returned output is like that:

here init
here print1
9999999999here print2
9999999999Circular buffer started
a
a pressedhere print1
9997999999here print2
9997999999-
a
a pressedhere print1
9997979999here print2
9997979999-
3
  • Thats not really a circular buffer. The indexes need to wrap around at the limit of the buffer (5). Otherwise you will write outside of the buffer with messages long enough.
    – chrisl
    Apr 8 at 14:02
  • You might find this useful: github.com/MajenkoLibraries/CircularBuffer
    – Majenko
    Apr 8 at 14:06
  • Thank you very much for the help. I sure have issues with how I pass the addresses and point to the variables. You are correct about limiting the buffer size but in this particular application, there is no way to fill the buffer - unless I code wrong :) That's why I wanted to keep as simple as possible.
    – TheClem
    Apr 12 at 9:28

2 Answers 2

4

You create a variable that can point to a circular buffer (*cb) but at no point do you actually point it at a circular buffer.

Either you need to create a new circular buffer or don't create a pointer, but create a statically allocated circular buffer. The latter is the preferred method.

Simply remove the * from your cb declaration, which makes a standard struct variable, then take the address of that (with &) to pass as a pointer to your other functions. Or change the other functions to pass by reference.

For example:

circular_buffer *cb;

becomes

circular_buffer cb;

And then

cb_init(cb);

becomes:

cb_init(&cb);

As do all the other functions that you pass the cb variable to.

Also every time you pass through loop() you will be wiping and making a new circular_buffer variable. Instead cb should really be global and the cb_init() call should be in setup().

1
  • Thank you for the answer. I have updated the code according to your answer. However, it seems that I still have issues with printing. Sorry to bother with those beginner questions.
    – TheClem
    Apr 12 at 9:46
1

There are multiple issues with this code. Majenko already showed the main one, I will add a few more here.

First, about the circular buffer implementation. As has been stated in a comment, for the buffer to be considered circular, the indices should wrap around. I would add a small nitpick: it is customary, in C++, to start filling an array from index 0. Thus you should post-increment (rather than pre-increment) the indices:

void cb_push_front(circular_buffer *cb, char item)
{
    cb->buffer_arr[cb->head] = item;
    cb->head++;                            // post-increment
    if(cb->head == circular_buffer_size)   // with wrap-around
      cb->head = 0;
}

There is still an issue though: the buffer can overflow. You should only add an item if there is room to do so. And you should only remove an item if the buffer is not empty. You know that the buffer is empty when tail=head, but how do you know when it is full? If you completely fill the buffer and the indices wrap around as they should, you end up with head=tail, and you cannot tell the difference between a full buffer and an empty buffer. There are two ways around this:

  • You can keep an extra variable in the struct that helps you tell the difference. This could be a count of the elements in the buffer, or a boolean that tells you when the buffer is empty, or a boolean that tells you when it is full.

  • You can refrain from completely filling the array and consider that the buffer is full when it has been filled to its full capacity, where the “capacity” is defined as the size of the array minus one. Then the buffer is full when
    (head + 1) = tail [modulo array length].

Here is an implementation of the second strategy:

void cb_push_front(circular_buffer *cb, char item)
{
    int next_head = cb->head + 1;          // increment
    if(next_head >= circular_buffer_size)  // with wrap around
      next_head = 0;
    if(next_head == cb->tail){
      Serial.println("buffer overflow");
      return;
    }
    cb->buffer_arr[cb->head] = item;
    cb->head = next_head;
}

int cb_pop_back(circular_buffer *cb)
{
    if(cb->tail == cb->head) {
      Serial.println("buffer underflow");
      return -1;
    }
    char item = cb->buffer_arr[cb->tail];
    cb->tail++;
    if(cb->tail >= circular_buffer_size)
      cb->tail = 0;
    return item;
}

Note that there is no point in cb_pop_back() overwriting the buffer. On the other hand, it is often very useful for this function to return the item that has just been popped.

Now, a few random comments:

  • Rather than hard-coding the array size (5) in multiple places, define a constant at the beginning of the program (circular_buffer_size in my examples) and use it everywhere. This makes it clearer what the number means, and makes it easier to change the size if you ever need to.

  • Since you are storing characters, use an array of char.

  • Your code for handling Serial does not cope well with the situation where there is a small delay between the 'a' instruction and the character to be added. If you can afford blocking code, the simplest solution is to block until the user provides the character:

if(instruction == 'a'){
  while(!Serial.available())
    ;  // wait for the key to be entered
  char key = Serial.read();
  cb_push_front(&cb, key);
  Serial.print(key);
  Serial.println(" is entered");
}
2
  • Very important points, thank you! I've been struggling with passing the cb properly to the functions so I didn't put much effort on -optimization- so to say. I still have some issues in printing. I've updated the question. But I will implement these points after it. Thank you so much.
    – TheClem
    Apr 12 at 9:50
  • Re “I will implement these points after it”: that's not a good idea. Fix these issues right now, then you program will work as expected. Or, if it doesn't, then it will be much easier to understand what is going wrong. Apr 12 at 18:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.