I need a library that will allow me to easily create byte FIFO's in my code. The Arduino HardwareSerial library uses such a FIFO to store RX characters (struct called ring_buffer). Unfortunately I'm not quite sure how to copy this functionality into a new library. I think I need a header file like this:
I get confused when it comes to writing the code for the methods in the class. I can copy the ring_buffer struct as well as the store_char functions from HardwareSerial, but I'm not sure how to pass around the buffer variable (or pointer?). In HardwareSerial a ring_buffer is created for each UART. I would like my library to create a new ring buffer for each instance.
I'm also not sure what to include in the constructor. Ideally I'd even be able to define the FIFO depth when I create it, but that isn't absolutely necessary. Here is my current ByteFifo.cpp:
#define RX_BUFFER_SIZE 128
struct ring_buffer {
unsigned char buffer[RX_BUFFER_SIZE];
uint8_t head;
uint8_t tail;
};
inline void store_char(unsigned char c, ring_buffer *rx_buffer)
{
int i = (rx_buffer->head + 1) % RX_BUFFER_SIZE;
// if we should be storing the received character into the location
// just before the tail (meaning that the head would advance to the
// current location of the tail), we're about to overflow the buffer
// and so we don't write the character or advance the head.
if (i != rx_buffer->tail) {
rx_buffer->buffer[rx_buffer->head] = c;
rx_buffer->head = i;
}
}
// Constructors ////////////////////////////////////////////////////////////////
ByteFifo::ByteFifo()
{
_rx_buffer = rx_buffer;
}
// Public Methods //////////////////////////////////////////////////////////////
void ByteFifo::write(uint8_t c)
{
store_char(c,_rx_buffer);
}
uint8_t ByteFifo::read(void)
{
// if the head isn't ahead of the tail, we don't have any characters
if (_rx_buffer->head == _rx_buffer->tail) {
return -1;
} else {
unsigned char c = _rx_buffer->buffer[_rx_buffer->tail];
_rx_buffer->tail = _rx_buffer->tail + 1;
_rx_buffer->tail %= RX_BUFFER_SIZE;
return c;
}
}
uint8_t ByteFifo::available(void)
{
//return (RX_BUFFER_SIZE + _rx_buffer->head - _rx_buffer->tail) % RX_BUFFER_SIZE;
uint16_t tmp = (_rx_buffer->head + RX_BUFFER_SIZE - _rx_buffer->tail);
tmp %= RX_BUFFER_SIZE;
return (uint8_t)tmp;
}
void ByteFifo::flush()
{
// don't reverse this or there may be problems if the RX interrupt
// occurs after reading the value of rx_buffer_head but before writing
// the value to rx_buffer_tail; the previous value of rx_buffer_head
// may be written to rx_buffer_tail, making it appear as if the buffer
// don't reverse this or there may be problems if the RX interrupt
// occurs after reading the value of rx_buffer_head but before writing
// the value to rx_buffer_tail; the previous value of rx_buffer_head
// may be written to rx_buffer_tail, making it appear as if the buffer
// were full, not empty.
_rx_buffer->head = _rx_buffer->tail;
}
Similar to how the rx buffer works in hardware serial, I would like the write to the FIFO in an ISR and read from the FIFO in the main program loop. Right now I want to write adc input data in an ISR and operate on it in the main loop.
A byte or character FIFO should allow me to move data this way.
I would like the write to the FIFO in an ISR and read from the FIFO in the main program loop
The code AlphaBeta provided may not work reliably in this context. I think interrupts may need to be disabled when accessing and adjusting numberOfElements.
There may be a boundary problem with nextIn and nextOut. As written, the two are unbounded. They will eventually wrap to negative numbers.
I think there's a discontinuity when nextIn or nextOut wraps if rawSize is not an even power of two.
I could declare the functions/variables as volatile, but I do not know if that would help.
I have no way of testing the class on an arduino at the moment, but if anyone do try it, and experience some problems, do not hesitate to mail me or PM or whatever
[edit]BTW: rawSize does not need to be a power of 2[/edit]
The code AlphaBeta provided may not work reliably in this context. I think interrupts may need to be disabled when accessing and adjusting numberOfElements.
Can you explain how this is unsafe? Doesn't the HardwareSerial library do the exact same thing to move data from the UART_RX ISR to be read in the main loop?
I'm not certain that it is unsafe. The problem with numberOfElements is that it is more than a single byte (an int is two bytes). On an AVR processor, two machine instructions are required to manipulate or access the value. If an interrupt occurs after the first instruction but before the second instruction, the value of numberOfElements can be "sheared".
Generally, when multi-byte values are shared between an ISR and non-interrupt code, it is necessary to protect the value.
However, if AlphaBeta's code is only used with buffers of 256 bytes or less, there shouldn't be a problem.
Doesn't the HardwareSerial library do the exact same thing to move data from the UART_RX ISR to be read in the main loop?
The code in HardwareSerial has the same kind of problem. head and tail are unprotected multi-byte values. Because the ring buffers are 128 bytes, it doesn't matter. The high-order byte of head and tail is never used.
The dequeue for loop checks sFIFO.count() on each iteration. Since we are reading one value on every iternation, count() will decrease by 1. So i and count will progress like so:
i = 0, count() = 5
i = 1, count() = 4
i = 2, count() = 3
i = 3, count() = 2
So the loop will only read the first 3 points from the FIFO. It's easily fixed by storing the initial count() value in another variable. I also found it useful to print the count() value to the serial port.
I'll plug it in to my code and do a more comprehensive test next.
If you don't mind, I have a question about style. You've put the method definitions in the header (which is my preference) but you've seperated them from the class definition. I'll use dequeue to illustrate...
template<typename T, int rawSize>
class FIFO {
public:
...
T dequeue(); // declaration here
...
};
...
// definition here; after the class definition
template<typename T, int rawSize>
T FIFO<T,rawSize>::dequeue()
{
T item;
numberOfElements--;
...
The declaration and definition can be combined like this (more of a Java style)...
template<typename T, int rawSize>
class FIFO {
public:
...
T dequeue()
{
T item;
numberOfElements--;
// reset of dequeue code is here
...
My preference is the latter because it means less typing. There also seems to be less bouncing back and forth when working on the code.
First: Thank you! It always feel nice when the work one put into something carries fruits.
I'm always prepared to answer questions of any kind related to code or Arduino, so no; I absolutely do not mind at all!
I think both styles can be argued for, and actually I do both myself.
I tend to do it this way when the code is not intended for my eyes only. This because it's faster to get a feeling as to what the library actually does.
One glimpse of the header and you immediately see that one can dequeue, enqueue, peek, flush and count on a FIFO object.
I value this when I read other peoples code, so I value it in my own.
I also makes it faster to revert to the classical h/cpp if that is needed.
