patterncMinor
Microcontroller ringbuffer
Viewed 0 times
microcontrollerringbufferstackoverflow
Problem
I'm pretty new to microcontrollers and C in general, so I thought asking here would be a good way not to get started with bad habits.
```
/*
* ringbuffer.h
*
* Created on: Jun 11, 2014
* Author: Silly Freak
*/
#ifndef RINGBUFFER_H_
#define RINGBUFFER_H_
//for size_t
//TODO is it a good idea to force size_t for head and available?
#include
//defines a buffer type with the given element type and size
//usage: RBUF(type, size) rbufname;
#define RBUF(TYPE, SIZE) \
struct { \
TYPE buf[SIZE]; \
size_t head, available; \
}
//initializes the buffer as being empty
#define RBUF_INIT(NAME) \
do { \
(NAME).head = 0; \
(NAME).available = 0; \
} while(0)
//number of slots in the buffer, i.e. buffer size/element size
#define RBUF_SIZE(NAME) \
(sizeof((NAME).buf)/sizeof((NAME).buf[0]))
//number of filled slots in the buffer
#define RBUF_AVAILABLE(NAME) \
((NAME).available)
//number of free slots in the buffer
#define RBUF_FREE(NAME) \
(RBUF_SIZE(NAME) - RBUF_AVAILABLE(NAME))
//index of the reading head into the ring buffer
#define RBUF_RHEAD(NAME) \
((NAME).head)
//index of the writing head into the ring buffer
#define RBUF_WHEAD(NAME) \
((RBUF_RHEAD(NAME) + RBUF_AVAILABLE(NAME)) % RBUF_SIZE(NAME))
//pushes an entry into the ring buffer
//the client has to ensure that RBUF_FREE > 0
#define RBUF_PUSH(NAME, VALUE)
```
/*
* ringbuffer.h
*
* Created on: Jun 11, 2014
* Author: Silly Freak
*/
#ifndef RINGBUFFER_H_
#define RINGBUFFER_H_
//for size_t
//TODO is it a good idea to force size_t for head and available?
#include
//defines a buffer type with the given element type and size
//usage: RBUF(type, size) rbufname;
#define RBUF(TYPE, SIZE) \
struct { \
TYPE buf[SIZE]; \
size_t head, available; \
}
//initializes the buffer as being empty
#define RBUF_INIT(NAME) \
do { \
(NAME).head = 0; \
(NAME).available = 0; \
} while(0)
//number of slots in the buffer, i.e. buffer size/element size
#define RBUF_SIZE(NAME) \
(sizeof((NAME).buf)/sizeof((NAME).buf[0]))
//number of filled slots in the buffer
#define RBUF_AVAILABLE(NAME) \
((NAME).available)
//number of free slots in the buffer
#define RBUF_FREE(NAME) \
(RBUF_SIZE(NAME) - RBUF_AVAILABLE(NAME))
//index of the reading head into the ring buffer
#define RBUF_RHEAD(NAME) \
((NAME).head)
//index of the writing head into the ring buffer
#define RBUF_WHEAD(NAME) \
((RBUF_RHEAD(NAME) + RBUF_AVAILABLE(NAME)) % RBUF_SIZE(NAME))
//pushes an entry into the ring buffer
//the client has to ensure that RBUF_FREE > 0
#define RBUF_PUSH(NAME, VALUE)
Solution
As for your
Reduce use of macros
The problem with the overuse of macros like this is that they lead to limitations. For example, let's say I want to write a
What should that
The other reason not to abuse macros like this is code bloat. If you use
I would have expected structure member variable
Avoid the use of the
Especially in embedded systems, the
Think about data alignment
On some processors, access to misaligned data causes a fault; on others it just slows things down. Because your
Return a value from
Instead of requiring the passing of an lvalue into your
Think of your user
It makes sense that your interface doesn't (presumably for performance reasons) force the user to call the various bounds checks for every push or pop, but it would be nice to have provided a convenience function which would do so, removing some of the burden from the user of your buffer.
TODO related question, yes, absolutely, if you use size_t as part of your code's interface, you should #include the relevant definition for it, so what you've got is correct as it stands. However, there are some things here that I think could be improved.Reduce use of macros
The problem with the overuse of macros like this is that they lead to limitations. For example, let's say I want to write a
print routine to print out the contents of a buffer. void printbuff(something *buff);What should that
something be? Because of the way your RBUF macro is defined, the struct is anonymous and therefore there is no way to make reference to it. For that same reason, there's no easy way to create an array of ring buffers or to include one in a structure. The other reason not to abuse macros like this is code bloat. If you use
RBUF_PUSH at five hundred locations in your code, you'll create five hundred copies of that code. That's not good for embedded systems which are often resource constrained.available is a misleading variable nameI would have expected structure member variable
available to keep a count of the available slots in the ring buffer. Instead, it actually keeps a count of the used slots. Even worse is that the comment and the actual operation of RBUF_AVAILABLE directly contradict each other.Avoid the use of the
% operatorEspecially in embedded systems, the
% operator is computationally costly and often of variable execution time. For both of those reasons, it's best avoided. The usual way to do this on small microcontrollers is to have the ring buffer size equal to some power of 2 and then use the bitwise & operator to wrap the buffer around. Think about data alignment
On some processors, access to misaligned data causes a fault; on others it just slows things down. Because your
struct is defined with the data buffer first, it means that an odd SIZE buffer may (depending on how your compiler packs structure members) put both head and available at odd addresses even if the struct itself is aligned. For that reason, it is probably better to place those ahead of the buffer instead.Return a value from
RBUF_POPInstead of requiring the passing of an lvalue into your
RBUF_POP macro, consider instead simply returning the value. This would allow constructs such as this:switch(RBUF_POP(ringbuf1))
{
case 'a':
/* etc... */
}Think of your user
It makes sense that your interface doesn't (presumably for performance reasons) force the user to call the various bounds checks for every push or pop, but it would be nice to have provided a convenience function which would do so, removing some of the burden from the user of your buffer.
Code Snippets
void printbuff(something *buff);switch(RBUF_POP(ringbuf1))
{
case 'a':
/* etc... */
}Context
StackExchange Code Review Q#53962, answer score: 3
Revisions (0)
No revisions yet.