

UART Receive Buffering - simplyembedded
http://www.simplyembedded.org/tutorials/interrupt-free-ring-buffer/

======
crshults
Important to remember for each data byte in the buffer to also store the LSR
along with that data. Windows serial port driver fails to do this, meaning you
can tell there was (for example) a parity error on a byte in your fifo, you
just don't know on which byte.

~~~
shabble
How would you implement that though, assuming memory is tight? The simplest
would be to just store a multibyte entry for each item in the ring buffer: the
actual data, and the status flags set at the time it was copied. But that
doubles your memory requirements, and you'll (probably) not need that status
info all the time.

I suppose you could define 'normal' and keep a smaller structure which holds
only abnormal values, but then you need a way to map which input byte each
value refers to, and make sure it's invalidated when the ring buffer rolls
around.

Or you could put some of the error-handling logic into your buffering code
(but it'd be in an interrupt handler, which you probably don't want), and you
might need your higher level framing or packetisation to know how far back you
need to invalidate.

I'm not sure there is a single optimal solution (although if you know of one,
I'd love to know). Windows getting it wrong is interesting/odd though, unless
it just dates to a time when tehy were cpu/memory restricted, adn it's just
stuck about for compatibility.

~~~
cnvogel
I assume that your serial port will run some kind of machine-to-machine
protocol, and not just a serial console with user data (for which, frankly,
I'll just ignore most errors). These serial prototols (like modbus, or the
10000s of ad-hoc specified "proprietary" protocols many devices speak) are
usually _very_ simple.....

And if memory is tight, you'll probably also want to move some part of your
protocol handling down the stack, probably up to the interrupt handler,
because otherwise you'll needlessly have double buffers (characters, and
decoded protocol datagrams). For most stuff running on serial ports the code
will be simple enough not to create too many headaches. Then just store the
fact that you've encountered a parity error next to your datagram. (msg->flags
|= MSGFLASG_PARITYERR );

And yes, I know that this is a "blatant layering violation" for a proper
operating system, but on a microcontroller really no one cares that you have
two variations of an interrupt handler, specific to your protocol handling.

EDIT/ADDED: It's not only parity errors, but there are also things like RS485
links with 9-bit addresses, BREAK characters with semantic meaning and
sometimes your communication protocol might specify when you switch directions
of half-duplex links, with hard timing requirements.

------
kabdib
The mix of global buffer and passed-in struct defining the buffer made me sad.
So did the lack of atomic increments and decrements (volatile ain't
sufficient). Don't get me started on the getchar routine.

This stuff just isn't hard. Sigh.

~~~
vardump
> The mix of global buffer and passed-in struct defining the buffer made me
> sad. So did the lack of atomic increments and decrements (volatile ain't
> sufficient). Don't get me started on the getchar routine.

It's for MSP430, and seems to be oriented in single core embedded
microcontrollers. Those things _have no atomic operations_ , other than
disabling interrupts.

You'd be right for multicore chips or if data type is larger than word size or
if data is not aligned.

But in this case, this code is correct for this architecture. All of those
things you mentioned are perfectly normal and acceptable when programming
microcontrollers.

[http://www.simplyembedded.org/tutorials/msp430-architecture/](http://www.simplyembedded.org/tutorials/msp430-architecture/)

[http://en.wikipedia.org/wiki/TI_MSP430](http://en.wikipedia.org/wiki/TI_MSP430)

~~~
kabdib
I guess you're assuming the compiler will issue an "increment" instruction for
the ++, and that instruction is atomic.

It's not a great coding practice. Move to a different processor (this
happens!) and your code turns into a load-add-store, gets interrupted halfway
through, and you scratch your head for a while about the mysterious and flaky
FIFO failures.

Been there. At least a gesture towards an atomic operation would be nice, when
you have data shared between interrupt and non-interrupt code.

~~~
onnoonno
How about putting

#if !defined(__AVR__) && !defined(__MSP430__)

#warning "Fix this FIFO - ++/\-- possibly not atomic!"

#endif

into the header of that thingy then. Save work on small uCs, and be warned
when moving it somewhere else.

------
8_hours_ago
There's at least 1 simple bug that I found: ring_buffer_get() returns an error
code, but it's being ignored in uart_getchar().

------
2bluesc
Love seeing embedded stuff on HN!

