Circular/Ring buffer problem

First, your receive ringbuffer should not decide what the incoming data means. Just put every incoming byte into the ringbuffer and forget it. The function that gets the data from the ringbuffer has to decide what to do with the incoming bytes. (first rule for ISRs: keep them as quick and straight as possible)
This will allow you later to decide whether to get a single byte or a sequence or a line, based on how you fetch the data from the ringbuffer. It allows implemetation of a stdio-compatble set of functions.

Next thing is that you should access the RXBUF once, exactly and only once. But in your ISR you access it twice, once for checking for '\n' then actually reading it into the buffer. Normally, it won't do any harm in this special case, but for other setups, reading the buffer may trigger further events. Here, it will only clear the RXIFG bit (which has been already cleared by reading the IV in teh switch statement). But in theory, it could be possible that another byte has just arrived, so your comparison isn't true anymore, and you accidentally drop a byte.

Also, a ringbuffer usually has two pointers, a read pointer and a write pointer. Both are just incremented to the size of the buffer and then start again with zero. (in case of a 256 byte buffer, an usigned char variable will do). If a byte is pushed into the buffer, the write pointer is incremented after the write. If the read pointer != read pointer, then there's something in the buffer and can be read and then the read pointer is incremented.
The only thing that is more complex (a little) is a buffer overflow. If you don't care, then on an overflow the buffer content is lost when the write pointer is incremented to the value of the read pointer. Or you check whether the read pointer is one ahead of the write pointer (buffer full) and drop the byte (you'll still have to read RXBUF to clear the interrupt) or overwrite the previous (last) byte.
These pointers are never reset (on a flush, the read pointer is set to the write pointer) and the buffer has always the same size.

Your code doesn't really look like a ring- or circular buffer at all. It rather looks like a dnymaically allocated linear line buffer (I cannot tell exactly, as same subroutines are missing). Which is something completely different and rather a job for the main level than for an interrupt level function.

On systems with low ram (and an MSP definitely is), the use of malloc should be avoided. It fragments the available memory and will in most cases sooner or later lead to no more memory available.
Use fixed buffers, so you can better estimate your memory usage. And won't run into dynamically generated space problems which are very difficult to track.

Hi, Jens-Michael, can you post working code of safe ring buffer for MSP430 (size more than 256B),  //which is writing into ring buffer in ISR, which occurs when is received character via UART ?

That complexity what you saying (testing if is buffer full (overrun)) is some problem for me.

Also I can mentionfor all, that wrap around is effecient, when write/read index  is ANDing with buffer size-1 . Then must be buffer size power of two, ie when buff size is 256, and WrIndex has value 256:

WrIndex &= 255;  //after execution thus, Wrindex = 0

What is faster method to fill/read buffer, probably nothing with pointers and directly access to arrays, ie buffer[WrIndex]=stored_char ?

Thanks (I´ll finding accross internet, some codes are difficult, some I think non effective and non safe)

Mykro Std said:

Hi, Jens-Michael, can you post working code of safe ring buffer for MSP430 (size more than 256B),  //which is writing into ring buffer in ISR, which occurs when is received character via UART ?

Mykro Std said:

That complexity what you saying (testing if is buffer full (overrun)) is some problem for me.

The condition for ring buffer full is simple. If (read-write ==1) then the buffer is full. If read==write, then the buffer is empty. This is because if read and write are both unsigned chars, the math result is also an unsigned char and wraps around.
To avoid ambiguity, this means the maximum buffer capacity is one less than buffer size (or else the conditions for buffer full and buffer empty would be identical and therefore ambiguous)

Mykro Std said:

What is faster method to fill/read buffer, probably nothing with pointers and directly access to arrays, ie buffer[WrIndex]=stored_char ?

The important thing is that when you check for buffer empty, read the value and increment the read pointer (RX direciton) or check for buffer full, writ ethe value and increment the write pointer (tx direction)  you must disable interrupts, or incoming or outgoing ISR may also access/change the pointers and mess things up.

256B implementation and "natural" overflow I understand, but other size need &nding :)

"The important thing is that when you check for buffer empty, read the value and increment the read pointer (RX direciton) or check for buffer full, writ ethe value and increment the write pointer (tx direction)  you must disable interrupts, or incoming or outgoing ISR may also access/change the pointers and mess things up."

this is what I little misunderstood, When I disable ISR (Uart received character), I lost that character. Is there need to write into main code behind "Read Rutine with disabled ISR" ,checking condition if is RXFIFO full?

Because if is into "Read Rutine with disabled ISR" code to disable ISR while is increment/decrement read pointer, received char on UART can apper and there will be lost because ISR is disabled. So after executed "Read Rutine with disabled ISR" there is need to write condition if is RXFIFO full ?

 (two successive reads from RXFIFO, one occur by ISR, second by test condition) ? // hope that you understand :D

A very crude way to use a circular buffer of 256 bytes is as follows:

a) Define an array of 256 bytes as the circular buffer and two 8-bit integers as index.
    unsigned char buffer[256], idx1=0, idx2=0;

b) Inside the ISR to handle incoming data, do:
    buffer[idx1++] = RXBUF;

c) To decide if there is any character in the buffer, do:
    if (idx1 == idx2) … nothing there …
    else … at least one, at most 255 characters in the buffer  …

d) To read or flash one character from the buffer, do:
    next_ch_in_buffer = buffer[idx2++];

A short coming of this simple-minded approach is, if you do not read or flash fast enough and the buffer becomes almost full, you will lose all those unread characters without any warning. It is not very difficult to add “low water mark”, “high water mark”, and issue “flow control” or warnings accordingly.

Mykro Std said:

this is what I little misunderstood, When I disable ISR (Uart received character),

Example:

    while((SndBufRCnt-SndBufWCnt)==1); // buffer full -> wait
    _DINT();
    if((U0SndBufRCnt==U0SndBufWCnt) { // buffer empty - restart ISR
      U0TXBUF=c;
    }
    else {
      U0SndBuf[U0SndBufWCnt++]=c;
    }
    _EINT();

My TX ISR will clear its TXIFG bit when there is nothing more to send, so no more interrupts happen. Hence the special case to 'restart' the ISR.
The full code in my library is a bit more complex, as it also handles different states like 'buffer full', 'buffer empty', 'sending' etc. which can be checked by software and can be used for thread handling (e.g. thread sleep on TX buffer full or on RX buffer empty etc.)