Rewrite RingBuffer class

[xcvista]

What's up with UART classes going in there accessing member variables of RingBuffer class?

Here is a rewritten RingBuffer class that no longer allow access to its member variables but can be used too. All condition checking can be rewritten as spin locking on the return value of putchar and getchar

/*
 * RingBuffer.h
 *
 *  Created on: 2016年10月6日
 *      Author: max
 */

#ifndef _RING_BUFFER_
#define _RING_BUFFER_

#include <stdint.h>

// Define constants and variables for buffering incoming serial data.  We're
// using a ring buffer, in which head is the index of the location
// to which to write the next incoming character and tail is the index of the
// location from which to read.
#ifndef SERIAL_BUFFER_SIZE
#define SERIAL_BUFFER_SIZE 128
#endif

#ifdef __cplusplus

class RingBuffer
{
private:
    volatile uint8_t buffer[SERIAL_BUFFER_SIZE] ;
    volatile uintptr_t head ;
    volatile uintptr_t tail ;

public:
    RingBuffer(void);
    int putchar(uint8_t c);
    int getchar(void);
    int peekchar(void);
    size_t flen(void);
};

#endif

#endif /* _RING_BUFFER_ */

#include <RingBuffer.h>
#include <string.h>

RingBuffer::RingBuffer(void)
{
    memset(static_cast<void *>(buffer), 0, SERIAL_BUFFER_SIZE);
    head = 0;
    tail = 0;
}

int RingBuffer::putchar(uint8_t c)
{
    uintptr_t i = (head + 1) % SERIAL_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 != tail)
    {
        buffer[head] = c;
        head = i;
        return 0;
    }
    else
    {
        return -1;
    }
}

int RingBuffer::getchar(void)
{
    int ch = getchar();

    if (ch >= 0)
    {
        tail = (tail + 1) % SERIAL_BUFFER_SIZE;
    }

    return ch;
}

int RingBuffer::peekchar(void)
{
    if (tail != head)
    {
        return buffer[tail];
    }
    else
    {
        return -1;
    }
}

size_t RingBuffer::flen(void)
{
    return (SERIAL_BUFFER_SIZE + head - tail) % SERIAL_BUFFER_SIZE;
}

[PaulStoffregen]

Does this impact performance?

[xcvista]

@PaulStoffregen I believe the impact is minimal (and you can make most functions inline.) However this does result in much cleaner code and clears up some code smell.

[PaulStoffregen]

As written, this appears to add function calls within the timing-sensitive Serial interrupt.

You're asking maintainers of a successful open source project used by millions of people to change timing-critical code in what is probably the most important part of the core library, and the best answer you have for performance is "believe the impact is minimal".

This sort of change needs to be very carefully tested and its performance impact well known. Successful projects with huge user bases don't go about changing their most important code with little or no testing only because someone doesn't like the coding style.

[xcvista]

@PaulStoffregen You could have also used things like macros and non-virtual inline methods. The resulting assembler code, especially when using macros, will be absolutely identical to the current code, yet it improves readability.

I am attempting this code in a new core I am writing for another board (based on STM32F103CB @ 72MHz) and I seriously doubt even with the slower method calls this code can result in negative impacts.