Ring Buffer

Log in or register to post comments
Go To Last Post
65 posts / 0 new
View All

Pages

Author
Message
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
#1
Posted by Tuurbo46: Mon. May 2, 2016 - 05:26 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

I am struggling to implement a Ring Buffer.  I have searched through many examples and they all start at a high level and pointers are not my strongest asset.  I have been working through the below link:

 

http://embedjournal.com/implemen...

 

The parts I do not understand:

 

How do they generate this macro?

#define CIRCBUF_DEF(x,y) uint8_t x##_space[y]; circBuf_t x = { x##_space,0,0,y} // How do they generate this?

 

Also is buffer supposed to be my Array of Rx data?

 

typedef struct
{
    uint8_t * const buffer;
    int head;
    int tail;
    const int maxLen;
}circBuf_t;

 

Do you have any simpler examples that I can get started with.

 

Cheers,

Tags:

AVR Microcontrollers, megaAVR and tinyAVR
Top
bloody-orc
bloody-orc's picture
Level: Posting Freak
Joined: Sat. Dec 17, 2005
Posts: 1659 View posts
Location: Europe- Estonia- Tallinn
Posted by bloody-orc: Mon. May 2, 2016 - 07:20 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

How about this:
http://www.fourwalledcubicle.com...

 

Seearch Lightweight Ring Buffer for examples

 

Used in LUFA and maaaaaaany other projects.

 

Rain

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Mon. May 2, 2016 - 02:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Maybe the concept of array is easier to imagine? If you send packets of 20 bytes, a 40 byte array should hold 2 packets.... one being pulled out and process, one filling in with the rx interrupt. Just need two array indexes... in and out. Add chars from the interrupt handler at the in index, pullem out with getchar() from the out index. Easy concept so far?

 

Imagecraft compiler user

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 2, 2016 - 02:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Agree with Bob. Don't try to over complicate this. A ring buffer is nothing more than an array of N elements and each time a new character arrives you just store it in the "next" slot in the array until you reach the end then you go back to the beginning (which is why it's known as a "ring"). That's all there is to it.

Things are, of course, just a little more complex because as well as writing and storing to the array you also need to read from it and the read point could well be" behind" the write point so you need to keep track of both. Indeed the test of whether there's anything in the buffer is to test whether the read point has caught up to the write point.

But just start by thinking how you write to the array in a circular / ring fashion.

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 2, 2016 - 05:36 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

Thanks for your help.  OK I will give it try.  I will come back if I get stuck.  

 

On another note, do you use the Atmel Studio debugger?   Oddly enough I never do, I just use print to screen and terminal.  I have never put in the time to learn.  Would this help with my development or not really?

 

Thanks,

 

Tuurbo46

Top
kk6gm
kk6gm's picture
Level: Raving Lunatic
Joined: Fri. Sep 11, 2009
Posts: 4022 View posts
Location: Sacramento, CA
Posted by kk6gm: Mon. May 2, 2016 - 05:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First, as an aside, if pointers are not your strongest asset, it would pay you to get more familiar with them.

Anyway,

Ring_buf_init: set head var to 0.  set tail var to 0.  set count to 0.

Ring_buf_put: if count == BUF_SIZE, return error, else (buf[head] = data, head++ (with wrap-around), count++, return success)

Ring_buf_get: if count == 0, return error, else (data = buf[tail], tail++ (with wrap-around), count--, return data)

 

 

Top
ka7ehk
ka7ehk's picture
Level: 10k+ Postman
Joined: Fri. Nov 22, 2002
Posts: 22871 View posts
Location: Tangent, OR, USA
Posted by ka7ehk: Mon. May 2, 2016 - 05:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Debugger has some positives and some negatives. Personally, I could not live without it.

 

Positive, You can view the value of any variable (in scope) and RAM, without having to make any pre-run choices. It also does not slow execution.

 

Cons: You can only get the information out when the MCU is stopped (break). This can cause it to loose serial characters and really mess with TWI or SPI or timers (depending on settings). 

 

The Atmel Studio debugger is not difficult to use. Like I said, I could not live without some kind of debugger.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

Top
Bronze Markawneil
awneil's picture
Level: 10k+ Postman
Joined: Fri. Jul 1, 2005
Posts: 25563 View posts
Location: Basingstoke, Hampshire, UK
Posted by awneil: Mon. May 2, 2016 - 06:02 PM (Reply to #6)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:
 if pointers are not your strongest asset, it would pay you to get more familiar with them.

Absolutely!

 

You really won't get far with 'C' programming in general - and embedded 'C' in particular - without a solid grip on pointers!

 

However, for a ring buffer, it's really just an array - so you can just as well use indexes instead of pointers...

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Mon. May 2, 2016 - 07:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have a little minimonitor I compile into my program... it has examine ram and examine flash, and one can dump ram and see the stack, and see the value of vars in ram (I look up the addr in the map). If you are running a program that loops 30 times a sec and I want to see a variable changing, I use a print statement, usually with a flag to print or not to print. (Printing slows it down a little). I guess there is a way to watch a variable when running in some debuggers, but I'm not up to speed on any of them.

 

Imagecraft compiler user

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Wed. May 11, 2016 - 03:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

So I have been working through the below Ring Buffer example and I am stuck.  I can push data into the buffer but I cannot pop data out.  

 

I am currently doing the below:

 

* push data in

* print data

 

I would like to:

 

* push data in

* pop data out

* print data

 

but I cannot do this, is this because I am not putting a long enough string into the FIFO buffer to pop?  I  don't fully understand how to pop data.

 

cheers,

 

Tuurbo46

 

#include <stdio.h>
#include <stdlib.h>

#define BUFFER_SIZE 6

typedef struct
{
    unsigned char * buffer[BUFFER_SIZE];
    int head;
    int tail;
    int maxlen;
}circbuff;

// Push Data In
int push_buff(circbuff *c, unsigned char data) 
{

    int next = c->head + 1; 
    if (next >= c->maxlen)  
        next = 0;

    // Cicular buffer is full

    if (next == c->tail)
        return -1;  // quit with an error

    c->buffer[c->head] = data;
    c->head = next;
    return 0;

}

// Read Data Out
int pop_buff(circbuff *c, unsigned char *data) 
{
    // if the head isn't ahead of the tail, we don't have any characters
    if (c->head == c->tail)
        return -1;  // quit with an error

    *data = c->buffer[c->tail];
    c->buffer[c->tail] = 0;  // clear the data (optional)

    int next = c->tail + 1;
    if(next >= c->maxlen)
        next = 0;

    c->tail = next;

    return 0;

}

int main()
{
    circbuff cbuff;      
    cbuff.maxlen = 6;    
    cbuff.buffer[6] = 0; 
    unsigned char i; int result;
    unsigned char usart_data_in[6] = "hello"; // usart rx in string

    for(i=0; i<5; i++)
    {
        // read rx port and load buffer
        push_buff(&cbuff.buffer, usart_data_in[i]);
        //pop_buff(&cbuff.buffer, &usart_data_in[i]);
        printf("%c\n", cbuff.buffer[i]);
    }

// read rx port
// circ_buff_push
// circ_buff_pop
// print data
return 0;
}

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. May 11, 2016 - 04:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tuurbo46 wrote:
I don't fully understand how to pop data.

Your pop_buff() looks "promising". In what way does it not work then?

 

BTW the elements of a [6] array are numbers [0]..[5]. It's probably fairly unwise to go doing things like:

    cbuff.buffer[6] = 0;

because there is no 7th element (numbered 6) in the array. As it happens this probably writes into cbuff.head so may not matter.

 

Actually, talking of cbuff.head. You do realise that by using:

int main()
{
    circbuff cbuff;

this cbuff is created on the stack and C gives no guarantees about what values will be in the variable? As such .head and .tail may start with any random value. The chances are it will probably be 0x00 but I wouldn't rely on that assumption. In fact this is where C++ scores over C for this kind of thing because in C++ you can associate some code (the "constructor") with the creation of this variable and in that code you could ensure that head/tail are set to sensible values. As it stands I think I would use:

int main()
{
    circbuff cbuff;      
    cbuff.head = 0;
    cbuff.tail = 0;

Or you may simply want to implement some kind of:

void cbuff_init(circbuff * pbuf) {
    pBuf->head = 0;
    pBuf->tail = 0;
}

then use:

int main()
{
    circbuff cbuff;      
    cbuff_init(&cbuff);

While it is a laudable learning exercise to try and reinvent circular buffers from the scratch for the ten gazillionth time is life not a lot simple if you just use pre written/tried/tested code such as Dean Camera's "Lightweight RingBuffer" found in LUFA? (or if you aren't going to use at least have a look as a "template" and see what it is he's doing that you aren't):

 

http://www.fourwalledcubicle.com...

 

As you can see he provides RingBuffer_InitBuffer() to do that all important initialisation. (his implementation is a bit different to yours as he uses pointers to the array rather than indices though).

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. May 11, 2016 - 04:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK so I fixed your code so it builds without warning and it works - both push and pop. Suggest you diff my version against yours above to see what I changed. In reality most of what I did was listen to the warnings the compiler gave me and fix the reasons for each warning.:

 

$ cat cbuff.c
#include <stdio.h>
#include <stdlib.h>

#define BUFFER_SIZE 6

typedef struct
{
    unsigned char buffer[BUFFER_SIZE];
    int head;
    int tail;
    int maxlen;
} circbuff;

// Push Data In
int push_buff(circbuff *c, unsigned char data) 
{

    int next = c->head + 1; 
    if (next >= c->maxlen)  
        next = 0;

    // Cicular buffer is full

    if (next == c->tail)
        return -1;  // quit with an error

    c->buffer[c->head] = data;
    c->head = next;
    return 0;

}

// Read Data Out
int pop_buff(circbuff *c, unsigned char *data) 
{
    // if the head isn't ahead of the tail, we don't have any characters
    if (c->head == c->tail)
        return -1;  // quit with an error

    *data = c->buffer[c->tail];
    c->buffer[c->tail] = 0;  // clear the data (optional)

    int next = c->tail + 1;
    if(next >= c->maxlen)
        next = 0;

    c->tail = next;

    return 0;

}

int main(void)
{
    unsigned char i;
    unsigned char usart_data_in[] = "hello"; // usart rx in string
    unsigned char c;

    circbuff cbuff;
    cbuff.maxlen = 6;
    cbuff.head = 0; 
    cbuff.tail = 0;

    for(i=0; i<5; i++)
    {
        push_buff(&cbuff, usart_data_in[i]);
    }
    for(i=0; i<5; i++)
    {
        pop_buff(&cbuff, &c);
        printf("%c\n", c);
    }

    return 0;
}

uid23021@lxl0131u:~$ gcc cbuff.c -o cbuff
uid23021@lxl0131u:~$ ./cbuff 
h
e
l
l
o

For example in the struct definition you had:

    unsigned char * buffer[BUFFER_SIZE];

The '*' in that meant you were creating an array of six POINTERS TO char rather than 6 char values. When you later tried to do something like:

    c->buffer[c->head] = data;

the compiler warned that you were "trying to make a pointer from an integer without a cast" because "data" is simply an integer value but as you had it the elements of the buffer[] array in the struct were all pointers.

 

When you did your pushing you were using:

        push_buff(&cbuff.buffer, usart_data_in[i]);

The compiler told you:

cbuff.c:15:5: note: expected ‘struct circbuff *’ but argument is of type ‘unsigned char (*)[6]’

that's because the first parameter is supposed to be the address of a whole circbuff struct not just the buffer member within it. So the line should have been:

        push_buff(&cbuff, usart_data_in[i]);

Also you wrote a (perfectly valid) interface for the pop() routine but then didn't seem to know how to use it. Your pop() routine interface was:

int pop_buff(circbuff *c, unsigned char *data)

while some people would have chosen to simply return the unsigned char from the routine itself it is perfectly valid to use the approach of passing in an "usigned char *" but to use it you have to give it some "unsigned char" to point to. I did that with:

    unsigned char c;

and then:

        pop_buff(&cbuff, &c);

so for the second "data" parameter I am passing it "&c" - that is the address of an unsigned char variable called "c". When the routine then does "*data = XXX" it will be writing into the location of "c". So "c" gets updated by a call to the pop() routine.

 

So the overall main() just pushes the bytes in the test string "hello" then pop's them out again printing each in turn. Seems to work :)

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Wed. May 11, 2016 - 05:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Clawson,

 

Thanks for you help, my C is not quite there yet.  I did try to use the http://www.fourwalledcubicle.com... example but I could not work through it.  Its easy when you know how but I did not know how.

 

However now I have worked through my example I think I understand it, in your opinion which is the better most stable solution out of the two?

 

cheers,

 

Tuurbo46.

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. May 11, 2016 - 05:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dean's code is well proven (lots of use by lots of people). The basics may be the same (except as I say he uses pointers while you use indices) but he has subtle things covered such as atomic protection where it may be needed if the input/output is being shared between ISR and main-line code. You may well get there in the end but do you, for example, even know why such atomic protection is required at this stage? If not then it could be a while before your code is as "good" as Dean's.

 

I suppose it's the same reason I would use itoa() or printf() or something rather than trying to write my own. You use library code because it's tried/tested and it saves you the effort. While writing stuff from scratch like HD44780 or DS1307 driver code or this ring buffer stuff is a good learning experience. If all you actually want is an LCD or RTC or ring buffer code you can trust so you can just use it and get on with the more interesting stuff in life then I think it just makes sense to use someone else's proven code (like strcpy and printf or FatFs and so on).

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Wed. May 11, 2016 - 05:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks Clawson,

 

OK I will work through Dean's as you say its proven.  

 

And another learning curve question, Deans code below is in a .h file.  Should this not be in a .c file?  

 

/*
             LUFA Library
     Copyright (C) Dean Camera, 2010.
              
  dean [at] fourwalledcubicle [dot] com
      www.fourwalledcubicle.com
*/

/*
  Copyright 2010  Dean Camera (dean [at] fourwalledcubicle [dot] com)

  Permission to use, copy, modify, distribute, and sell this 
  software and its documentation for any purpose is hereby granted
  without fee, provided that the above copyright notice appear in 
  all copies and that both that the copyright notice and this
  permission notice and warranty disclaimer appear in supporting 
  documentation, and that the name of the author not be used in 
  advertising or publicity pertaining to distribution of the 
  software without specific, written prior permission.

  The author disclaim all warranties with regard to this
  software, including all implied warranties of merchantability
  and fitness.  In no event shall the author be liable for any
  special, indirect or consequential damages or any damages
  whatsoever resulting from loss of use, data or profits, whether
  in an action of contract, negligence or other tortious action,
  arising out of or in connection with the use or performance of
  this software.
*/

/** \file
 *
 *  Ultra lightweight ring buffer, for fast insertion/deletion. This uses inlined functions
 *  for maximum speed. All buffers created with this library must be of the same size, however
 *  multiple independant buffers can be created.
 *
 *  Note that for each buffer, insertion and removal operations may occur at the same time (via
 *  a multithreaded ISR based system) however the same kind of operation (two or more insertions
 *  or deletions) must not overlap. If there is possibility of two or more of the same kind of
 *  operating occuring at the same point in time, atomic (mutex) locking should be used.
 */
 
#ifndef _ULW_RING_BUFF_H_
#define _ULW_RING_BUFF_H_

	/* Includes: */
		#include <util/atomic.h>
	
		#include <stdint.h>
		#include <stdbool.h>

	/* Defines: */
		/** Size of each ring buffer, in data elements - must be between 1 and 255. */
		#define BUFFER_SIZE            255
		
		/** Type of data to store into the buffer. */
		#define RingBuff_Data_t        uint8_t

		/** Datatype which may be used to store the count of data stored in a buffer, retrieved
		 *  via a call to \ref RingBuffer_GetCount().
		 */
		#if (BUFFER_SIZE <= 0xFF)
			#define RingBuff_Count_t   uint8_t
		#else
			#define RingBuff_Count_t   uint16_t
		#endif

	/* Type Defines: */
		/** Type define for a new ring buffer object. Buffers should be initialized via a call to
		 *  \ref RingBuffer_InitBuffer() before use.
		 */
		typedef struct
		{
			RingBuff_Data_t  Buffer[BUFFER_SIZE]; /**< Internal ring buffer data, referenced by the buffer pointers. */
			RingBuff_Data_t* In; /**< Current storage location in the circular buffer */
			RingBuff_Data_t* Out; /**< Current retrieval location in the circular buffer */
			RingBuff_Count_t Count;
		} RingBuff_t;
	
	/* Inline Functions: */
		/** Initializes a ring buffer ready for use. Buffers must be initialized via this function
		 *  before any operations are called upon them. Already initialized buffers may be reset
		 *  by re-initializing them using this function.
		 *
		 *  \param[out] Buffer  Pointer to a ring buffer structure to initialize
		 */
		static inline void RingBuffer_InitBuffer(RingBuff_t* const Buffer)
		{
			ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
			{
				Buffer->In    = Buffer->Buffer;
				Buffer->Out   = Buffer->Buffer;
				Buffer->Count = 0;
			}
		}
		
		/** Retrieves the minimum number of bytes stored in a particular buffer. This value is computed
		 *  by entering an atomic lock on the buffer while the IN and OUT locations are fetched, so that
		 *  the buffer cannot be modified while the computation takes place. This value should be cached
		 *  when reading out the contents of the buffer, so that as small a time as possible is spent
		 *  in an atomic lock.
		 *
		 *  \note The value returned by this function is guaranteed to only be the minimum number of bytes
		 *        stored in the given buffer; this value may change as other threads write new data and so
		 *        the returned number should be used only to determine how many successive reads may safely
		 *        be performed on the buffer.
		 *
		 *  \param[in] Buffer  Pointer to a ring buffer structure whose count is to be computed
		 */
		static inline RingBuff_Count_t RingBuffer_GetCount(RingBuff_t* const Buffer)
		{
			RingBuff_Count_t Count;
			
			ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
			{
				Count = Buffer->Count;
			}
			
			return Count;
		}
		
		/** Atomically determines if the specified ring buffer contains any free space. This should
		 *  be tested before storing data to the buffer, to ensure that no data is lost due to a
		 *  buffer overrun.
		 *
		 *  \param[in,out] Buffer  Pointer to a ring buffer structure to insert into
		 *
		 *  \return Boolean true if the buffer contains no free space, false otherwise
		 */		 
		static inline bool RingBuffer_IsFull(RingBuff_t* const Buffer)
		{
			return (RingBuffer_GetCount(Buffer) == BUFFER_SIZE);
		}

		/** Atomically determines if the specified ring buffer contains any data. This should
		 *  be tested before removing data from the buffer, to ensure that the buffer does not
		 *  underflow.
		 *
		 *  If the data is to be removed in a loop, store the total number of bytes stored in the
		 *  buffer (via a call to the \ref RingBuffer_GetCount() function) in a temporary variable
		 *  to reduce the time spent in atomicity locks.
		 *
		 *  \param[in,out] Buffer  Pointer to a ring buffer structure to insert into
		 *
		 *  \return Boolean true if the buffer contains no free space, false otherwise
		 */		 
		static inline bool RingBuffer_IsEmpty(RingBuff_t* const Buffer)
		{
			return (RingBuffer_GetCount(Buffer) == 0);
		}

		/** Inserts an element into the ring buffer.
		 *
		 *  \note Only one execution thread (main program thread or an ISR) may insert into a single buffer
		 *        otherwise data corruption may occur. Insertion and removal may occur from different execution
		 *        threads.
		 *
		 *  \param[in,out] Buffer  Pointer to a ring buffer structure to insert into
		 *  \param[in]     Data    Data element to insert into the buffer
		 */
		static inline void RingBuffer_Insert(RingBuff_t* const Buffer,
		                                     const RingBuff_Data_t Data)
		{
			*Buffer->In = Data;
			
			if (++Buffer->In == &Buffer->Buffer[BUFFER_SIZE])
			  Buffer->In = Buffer->Buffer;

			ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
			{
				Buffer->Count++;
			}
		}

		/** Removes an element from the ring buffer.
		 *
		 *  \note Only one execution thread (main program thread or an ISR) may remove from a single buffer
		 *        otherwise data corruption may occur. Insertion and removal may occur from different execution
		 *        threads.
		 *
		 *  \param[in,out] Buffer  Pointer to a ring buffer structure to retrieve from
		 *
		 *  \return Next data element stored in the buffer
		 */
		static inline RingBuff_Data_t RingBuffer_Remove(RingBuff_t* const Buffer)
		{
			RingBuff_Data_t Data = *Buffer->Out;
			
			if (++Buffer->Out == &Buffer->Buffer[BUFFER_SIZE])
			  Buffer->Out = Buffer->Buffer;

			ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
			{
				Buffer->Count--;
			}
			
			return Data;
		}

#endif

 

 

 

 

Top
Bronze Markawneil
awneil's picture
Level: 10k+ Postman
Joined: Fri. Jul 1, 2005
Posts: 25563 View posts
Location: Basingstoke, Hampshire, UK
Posted by awneil: Wed. May 11, 2016 - 06:01 PM (Reply to #15)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tuurbo46 wrote:
  Should this not be in a .c file?  

No - because all the functions there are inline 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Top
Bronze MarkJohanEkdahl
JohanEkdahl's picture
Level: 10k+ Postman
Joined: Wed. Mar 27, 2002
Posts: 26945 View posts
Location: Lund, Sweden
Posted by JohanEkdahl: Wed. May 11, 2016 - 06:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No need for this to be in a .c file since all function definitions are "static inline". That's the exception to the "no code in .h files" rule.

Dean actively chose that scheme so that there would be no call overhead - a wise decision since all functions are quite "compact".

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here. 

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Wed. May 11, 2016 - 08:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

//--------------------
//UART0 initialize
// desired baud rate: 115200
// actual: baud rate:117647 (2.1%)
void uart0_init(void){

  UDR0=0x00;
  UCSR0A=0x02; //double speed
  UCSR0C=0x06; //8n1
  UBRR0H=0x00;
  UBRR0L=0x10; //115200 at 16
  UCSR0B=0x18; //rx tx enab              //<--chg this to 0x98 (rx int on)
}

//-----------RS232----------------
#if 0
unsigned char kbhit(void){
//return nonzero if char waiting  polled version
unsigned char b;

  b=0;
  if(UCSR0A & (1<<RXC0)) b=1;
  return b;
}

//-----------------------
int getchar(void){
//polled version... 
unsigned char c;

  while(!kbhit()){}; //wait for char
  c=UDR0;             //get char from usart
  return c;  
}
#endif

//#if 0
//----------------------------
#define RXBUFSIZ 200
unsigned char rxbuf[RXBUFSIZ]; 
unsigned char rxindx,rxondx;   

//-------------------------
unsigned char kbhit(void){
//return nonzero if char waiting  intr version
unsigned char b;

  b=rxindx != rxondx;
  return b;
}

//-----------------------
int getchar(void){
//intr version... overrides version in library
char c;

  while(!kbhit()){}; //wait for char
  c=rxbuf[rxondx++]; //get char from rxbuf
  if(rxondx >= RXBUFSIZ) rxondx=0;
  return c;  
}

//------------------------------
#pragma interrupt_handler uart0_rx_isr:iv_USART0_RX
void uart0_rx_isr(void){
//uart has received a character in UDR
char c;
unsigned int lrxindx;

  lrxindx=rxindx;
  c=UDR0;            //get char from usart
  rxbuf[lrxindx++]=c; //put char in rxbuf
  if(lrxindx >= RXBUFSIZ) lrxindx=0;
  rxindx=lrxindx;	  
}

//---------------------
int putc(char c){
//put char to usart0... dont add lf after cr

  while((UCSR0A & 0x20)==0){};	//0x20 is UDRE0. ==0 means data register full
  UDR0=c; //send char
  return c;
}

//----------------------  
int putchar(char c)	{
//adds lf after cr

  if(c=='\n'){
    putc('\r');
  }  
  putc(c);
  return c;
}

//--------------------
void crlf(void){
//send cr lf

  putchar('\n');
}

Maybe a smaller version is less filling?

 

Imagecraft compiler user

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Thu. May 12, 2016 - 01:29 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK,

 

So I have Deans example up and running manually loading the buffer, but now I want to load directly for the UDRO.  If for example I try to load 'hello' into the buffer I get out 'hhhhhhhhheeeeeeeeeeeeeeee'  I have tried adding delays in different places but no luck.  Can you confirm my read function is correct?

 

void usart_data_into_ring_buffer(void)
{
	unsigned char data;
	
	while(!(UCSR0A & (1<<RXC0)))
	{
	
		data = UDR0;
	
		RingBuffer_Insert(&Buffer, data);
	}
	 
}

 

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Thu. May 12, 2016 - 02:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
void usart_data_into_ring_buffer(void)
{
	unsigned char data;

	while(!(UCSR0A & (1<<RXC0)))
	{

		data = UDR0;

		RingBuffer_Insert(&Buffer, data);
	}

}

WRONG! You don't want to keep doing an insert all the time that RXC0 is not set. You want to wait for it to become set (to indicate something valid has been received then just pick up and insert that thing once. So...

void usart_data_into_ring_buffer(void) {
	unsigned char data;

        // wait for a character to be received...
	while(!(UCSR0A & (1<<RXC0)));

	data = UDR0;

	RingBuffer_Insert(&Buffer, data);
}

To be honest I don't really see the purpose of the "data" variable in this. Why not simply:

void usart_data_into_ring_buffer(void) {
        // wait for a character to be received...
	while(!(UCSR0A & (1<<RXC0)));

	RingBuffer_Insert(&Buffer, UDR0);
}

?

 

PS forgot to say that if you are doing reception by synchronous polling then what point is there in a ring buffer anyway? The usual purpose of a ring buffer is for interrupt driven reception - it acts as a "holding tank" to buffer data that has arrived before there is a chance to make use of it.

Last Edited: Thu. May 12, 2016 - 03:01 PM
Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Thu. May 12, 2016 - 03:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Clawson,

 

This sure is a step learning curve - thanks.  The only reason I am polling is because main has two interrupts running.  I cannot put function and function calls in interrupts so I put a flag.  How would I do this properly then without polling?

 

Thanks in advance.

 

ISR(INT0_vect) 
{  
 
      EIFR |= (1 << INTF0);     
      DataReady = 1;

}

ISR(USART0_RX_vect) 
{   
 
      int8_t Data;
      RxDataReady = 1;
      Data = UDR0;

}

int main(void)
{

   if(DataReady) 
   {    
                
        // Do some sensor stuff
        // reset sensor                 
                        
   }

   if(RxDataReady) 
   {    
                
    
        usart_data_into_ring_buffer();
        RxDataReady = 0;                    
                        
   }
}

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Thu. May 12, 2016 - 03:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Tuurbo46 wrote:
I cannot put function and function calls in interrupts so I put a flag. How would I do this properly then without polling?

I think you are missing a concept in there somewhere!

 

Yes it's true that you should always endeavour to keep ISRs as short as possible and often just setting a flag is all it needs and yes it's true that calling a function from an ISR can "cost" a lot of time/space because it may involve the compiler having to save/restore a lot of registers but that is part of the genius of Dean's Ringbuffer stuff.

 

You asked previously about it being in a header and others said "ah that's OK because all the "functions" are static inline" but there's more to this. By being "static inline" they are not CALLd but when you type Ringbuffer_insert() in your code it does not just put in a "CALL RingBuffer_Insert" which might well force the caller to have to PUSH/POP tons of registers on either side of the call if it sere made from an ISR but instead when you have:

ISR(RXC_vect) {
  Ringbuffer_Insert(&Buffer, UDR0);
}

the entire code of RingBuffer_Insert() that is listed in the .h file will be placed within ISR(RXC_vect). While not identical it's going to be a bit like expanding a macro. So it's almost as if you just wrote:

ISR(RXC_vect) {
	*Buffer.In = UDR0;
	
	if (++Buffer.In == &Buffer.Buffer[BUFFER_SIZE])
	  Buffer.In = Buffer.Buffer;

	ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
	{
		Buffer.Count++;
	}
}

So there is no CALL overhead here.

 

As I said Dean is a very smart guy and he designed this stuff really well as he knew all about the traps like not making CALLs from ISRs and, indeed, this is almost certainly why all the functions are "static inline" and, once you do that, you might as well then put the code definitions in a .h file as they won't generate standalone, callable copies of the functions but, instead, the body of them will simply be inserted (a bit like a macro) at the point of invocation.

 

So drop you RxDataReady flag and DO put an invocaiton of RingBuffer_Insert() directly into the RXC ISR code.

 

In this way all the "input" stuff for the RingBuffer happens on the ISR side of the fence and from main() the only code involving the RingBuffer you are likely going to be using is the one-time call during setup to the RingBuffer_InitBuffer() function and then most RingBuffer_Remove() where you want to actually get access to the buffered characters that have been collected by the ISR. To help you it's possible you might also find yourself calling things like RingBuffer_GetCount() to see how many characters are waiting in the buffer. As the comments say you should probvaly also call RingBuffer_IsEmpty() before attempting RingBuffer_Remove() as you don't want to try and remove a character if there is none waiting to be removed.

 

From the ISR as well as the actual RingBuffer_Insert() you might want to make a call to (well "invocation of" in fact) RingBuffer_IsFull() and take some kind of special action if this ever says "true" as it's effectively saying you are getting RXC's so often that there wasn't time to use up the characters in the buffer - in other words you probably made the buffer too small or you are taking too long with other stuff in main() and not attempting to process the received characters often enough.

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Thu. May 19, 2016 - 07:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

So I have the ring buffer working BUT, I have a few problems.

 

1) I can read in my fist set of data, and print out to confirm I am reading correctly.

 

2) When I read in my second lot of data it become corrupt.  Do I have to reset/ initialize again on each read?

 

3) I have 4 operations I want to complete and it is not working.  Below are my list:

   

   3.1) Read data 1 in and clear the buffer.  This is the initial hello from the modem.  I can read this data in and chop and parse nicely.

 

   3.2) Send out a command to a modem

 

   3.3) Read data 2 in and clear the buffer.  This data is all corrupt.  I have  also tried initializing the buffer after 3.1 but no luck.

 

The first read 3.1 is very clean.  How do I overcome the problem with 3.3?  I have tried many things and still no luck.  

 

Thanks,

 

Tuurbo46.

 

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Thu. May 19, 2016 - 07:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You could poll for chars coming from the modem at 9600bps and not worry about interrupts if you could check the uart every ms. Next best approach is use the uart receive interrupt and put the incoming char in an array (like in msg 18). If the interrupt handler takes 2usecs every 1ms, thats only using a fraction of a percent of the cpu. 

Imagecraft compiler user

Top
kk6gm
kk6gm's picture
Level: Raving Lunatic
Joined: Fri. Sep 11, 2009
Posts: 4022 View posts
Location: Sacramento, CA
Posted by kk6gm: Thu. May 19, 2016 - 09:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You should never need to clear the buffer.  If you do, all you need to do is set tail = head, and maybe set count = 0 if you use a count variable.

 

Debugging the buffer will involve keeping track of (and displaying) all the incoming data, and the head and tail values for each buffer put and get.  Is your first data large enough to force the ring buffer to wrap around?  If so, then you know that works because it reads the data correctly.  If not, and your second data does cause wrap around, then I'd look there.

 

What if you send data 1 twice, that is, instead of sending data 2 after data 1, you resend data 1?  Does it read both times correctly?

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Fri. May 20, 2016 - 10:35 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi kk6gm and bodgardener,

 

It looks like a head tail problem, it is not going round in a circle.  If I send data 1 three times, it overflows and the mcu restarts.

 

So I will have to go back and try again.

 

If I cant get the ring buffer to work, I think I will have to resort to a fixed length array.  I have spent very many hours trying to get this working :-(

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Fri. May 20, 2016 - 11:28 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A ring buffer should effectively be invisible. Think about the non-buffered case. You might just have a:

while(whatever) {
    c = UART_getchar();
    process(c);
}

this just reads characters from time to time and does something with them. The big problem comes when you call process() (whatever that really means) and it takes 5 minutes to complete. Meanwhile characters are arriving at your UART but there's nothing there to pick them up so they are lost. So you implement:

while(whatever) {
    c = Buffer_getchar();
    process(c);
}

// and elsewhere
ISR(UART) {
    stuffBuffer(UDR);
}

Buffer[big];

Nothing has really changed here EXCEPT that now there is an "invisible" buffer in the system. Now the arrival and storage of the characters is a completely independent activity. It will go on and capture the inbound characters whatever happens (though there is a remote danger that the buffer might fill up). Back in the while() loop it is business as usual. The getchar() just keeps getting chars only now they are not direct from UART but from the "holding tank". If process() takes 5 minutes to chew on a particular character it does not matter because the buffer handles stuff.

 

Other than this nothing really changed. As far as the while() loop is concerned it is business as usual. Whether the buffer has only received "hello" or has received "now is the time for all good men to come to the aid of the party" is completely immaterial. What's gone before has (should have!!) no effect on what happens now. Sure the write and read pointers in the buffer may have moved on a bit. They may even have moved so far that they hit the end of the bufefr and returned to the start (which is why it is "ring"/"circular") but each time the read pointer and the write pointer meet (all waiting characters removed) the buffer is effectively "empty" and it doesn't matter if both those pointers are at item [3] or [15] or [31] (and both about to wrap back to [0]).

 

So it's simply a fault if previous activity has any bearing on what happens "now". You need a debugger to investigate why that might be. Has something dire happened like the read pointer has passed the write pointer or something like that? If so, how? Only your debugger can tell you.

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Fri. May 20, 2016 - 11:35 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How many characters does the modem send? 10? Make the buffer twice that size. The modem can send a 2nd msg while the 1st one is being read byte by byte.

 

Imagecraft compiler user

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Fri. May 20, 2016 - 12:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
/************ This part works great ******/

// initial hello from modem to mcu after text "Test" sent
+CMT: "+441234567890","","16/05/20,12:10:39+04"
Test
52 // number of values in ring buffer

// usart received data printed back out to terminal
+CMT: "+441234567890","","16/05/20,12:10:39+04"

/************ This part does not work, I cannot grab "Test"  ******/
// Command sent to modem to return message "Test" text
AT+CMGR=1

// Message sent back from modem
+CMGR: "REC UNREAD","+441234567890","","16/05/20,12:10:39+04"
Test

// Message sent back from modem
+CMGR: "REC READ","+441234567890","","16/05/20,12:10:39+04"
Test


I cannot read back the message "Test".  Is this better suited to a ring buffer or a standard array?

 

I cannot explain this very well but this is happening:

 

1) phone sends modem text

2) modem receives text and sends hello to mcu and fires interrupt, usart reads data into ring buffer, and it is printed back out to terminal.

/************ This part works great ******/

// initial hello from modem to mcu after text "Test" sent
+CMT: "+441234567890","","16/05/20,12:10:39+04"
Test
52 // number of values in ring buffer

// usart/ ring buffer received data and printed back out to terminal
+CMT: "+441234567890","","16/05/20,12:10:39+04"

3) mcu sends  AT+CMGR = 1 to retrieve the stored text "Test" from modem

4) modem fires another interrupt and the below is returned

/************ This part does not work, I cannot grab "Test"  ******/
// Command sent to modem to return message "Test" text
AT+CMGR=1

// Message sent back from modem
+CMGR: "REC UNREAD","+441234567890","","16/05/20,12:10:39+04"
Test

// Message sent back from modem
+CMGR: "REC READ","+441234567890","","16/05/20,12:10:39+04"
Test

 

So my problem is when every text is received, I get the initial below unwanted string, I do not want this, I only want the text "Test".  So the first 52 chars are unwanted.  I kind of have a double interrupt fire that i cannot get over:

 

/************ This part works great ******/

// initial hello from modem to mcu after text "Test" sent
+CMT: "+441234567890","","16/05/20,12:10:39+04"
Test
52 // number of values in ring buffer

// usart/ ring buffer received data and printed back out to terminal
+CMT: "+441234567890","","16/05/20,12:10:39+04"

I have one rx interrupt, and one insert data function into ring buffer.  So the first interrupt, data in the ring buffer I do not want,  I only want the second interrupt data, and as a result my ring buffer is getting jammed up because I am reading twice, and not resetting properly.

 

I have been trying to read first interrupt, re initialize ring buffer, to clear out unwanted data.  Then when second interrupt is fired I can read this data, but it is not working out.

 

If i had one string It would be easier, but I have to much data to process.

 

How is the best way to get over this?

 

Many thanks.

Top
dbrion0606
dbrion0606's picture
Level: Raving Lunatic
Joined: Sat. Jan 7, 2012
Posts: 3693 View posts
Location: North of France
Posted by dbrion0606: Fri. May 20, 2016 - 12:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

"The modem can send a 2nd msg while the 1st one is being read byte by byte."

Well, but what happens if the modem wants to send a 3rd ... nth message?

What seems relevant would be  the maximum number of incoming characters while the application is not processing them (doing other things, such as displaying/decoding) .

 

Arduino chooses a length of 64 ,  if there is enough RAM (much greater than a 6 bytes length; if there is little RAM, it uses 16 :

 

 less arduino-1.6.5-r5/hardware/arduino/avr/cores/arduino/HardwareSerial.h

#if (RAMEND < 1000)
#define SERIAL_TX_BUFFER_SIZE 16
#else
#define SERIAL_TX_BUFFER_SIZE 64
#endif

 

Dean Camera 's ringbuffer (which is recommanded : easier to understand than Arduino's one) has examples with a 128 items size....

 

 

 

 

 

Last Edited: Fri. May 20, 2016 - 12:33 PM
Top
Kartman
Kartman's picture
Level: 10k+ Postman
Joined: Thu. Dec 30, 2004
Posts: 24979 View posts
Location: Melbourne,Australia
Posted by Kartman: Fri. May 20, 2016 - 12:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So how is this a problem with the ring buffer? Double interrupt fire? You get an interrupt for each character - so you're talking about 52 interrupts.
Your code needs to parse the responses from the modem. There are plenty of example code to do this. I'd look at the Arduino code for an idea. The Arduino implements a circular buffer as well.

Top
dbrion0606
dbrion0606's picture
Level: Raving Lunatic
Joined: Sat. Jan 7, 2012
Posts: 3693 View posts
Location: North of France
Posted by dbrion0606: Fri. May 20, 2016 - 12:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

"The Arduino implements a circular buffer as well"

 

Main issue with Arduino's ringbuffer is that it is difficult to understand (works very well) as it supports a huge number of configurations (and one does not have a C preprocessor in one's head...)

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Fri. May 20, 2016 - 02:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Kartman,

 

I would be happy to parse the below string, but I just cannot get it in and out of the buffer all in one string:

 

+CMT:"+441234567890","","16/05/20,12:10:39+04"+CMGR:"REC UNREAD","+441234567890","","16/05/20,12:10:39+04"Test

 

I is what I have struggling to explain

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Fri. May 20, 2016 - 02:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tuurbo46 wrote:
cannot get it in and out of the buffer all in one string

Why do you need it in the buffer all at the same time?

 

That buffer is just a "holding tank". When things are working really well it will never hold more than one character. If the foreground code slows down or gets involved in long work the buffer may start to fill up a bit but you would never expect it to have a huge string like that in it unless the foreground code became really dormant.

 

Just treat the RingBuffer_Remove() just like a plain UART_getchar(). Try to forget that the buffer is even there. It is just to collect up the few characters you cannot handle immediately.

 

If you were doing direct UART reads you would do something like:

char buffer[64];

do {
    c = UART_getchar();
    buffer[idx++] = c;
} while (c != '\n');

and that would keep reading characters until you have a complete line (until '\n'ew line) in your local buffer. Then you process that buffer. All that changes with a ringbuffer is:

char buffer[64];

do {
    c = RingBuffer_Remove();
    buffer[idx++] = c;
} while (c != '\n');

as you are not in direct contact with the UART itself any more.

 

Don't confuse this foreground "line buffer" with the 'hidden' buffer in the middle of the ringbuffer stuff that is just there to catch characters on an interrupt that might have got by uncollected.

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Fri. May 20, 2016 - 05:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If the modem sends 52 chars, make the array 200 chars (see msg 18). 

Imagecraft compiler user

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 02:10 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Clawson,

 

I need your guru help.

 

I just cant do it.  If the buffer went back to zero on every removal of the string it would be easy, but I just cannot get main correct.  What ever I test for count it just keeps moving so, I cannot get it correct.  

 

With the below four functions, I cannot get it to work correctly.

 

RingBuffer_Get_Count()

 

RingBuffer_Get_Free_Count()

 

RingBuffer_Is_Full();

 

RingBuffer_Is_Empty();

 

Can you give me any ideas as completely lost.  Below is my main,  it constantly prints my string to screen because Count does not reset to zero when printed out.

 

Thanks.

 

#include <avr/io.h>
#include <avr/interrupt.h>

volatile uint16_t BufferCount;
RingBuff_t Buffer;

ISR(USART0_RX_vect) 
{
	RingBuffer_Insert(&Buffer, UDR0);
}


int main(void)
{
// I have changed BUFFER_SIZE in header file to 64
RingBuffer_InitBuffer(&Buffer);
		
	while(1)
	{
				
		BufferCount = RingBuffer_GetCount(&Buffer);
				
		if(BufferCount) 
		{
			putcr_usart_0(RingBuffer_Remove(&Buffer));
			BufferCount = 0;
			
			_delay_ms(100);
			usart_flush_0();
			
		}
		
		
	}
}

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 23, 2016 - 02:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tuurbo46 wrote:
it constantly prints my string to screen

How does that print a "string"? I only see it removing a single character at a time - not whole strings?!?

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 02:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes OK, my mistake, a char a time, that prints my whole string to terminal.

 

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 23, 2016 - 02:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And the problem with that is...?

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 02:42 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A calculated guess, Buffer will never empty when printing chars, so BufferCount will always be greater than 1, and the loop will keep printing.  If the string is printed, BufferCount will equal zero?

 

 

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 23, 2016 - 02:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I still don't see your problem. if you send it:

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test

and it prints:

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test

then where is the problem in that? If it printed something like:

+++++++CCCCCCCCMMMMMMMMMMMMTTTTTTT::::::::          """"""""""""++++++++++++++++ etc.441234567890","","16/05/23,14:45:51+04"
Test

then, "yes" that would be a problem. But it does not. It appears to print each character received exactly once as it "consumes" it from the buffer. Surely that's what you want?

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 02:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes it prints correctly, but I cannot stop it, it prints over and over and over.  The condition of BufferCount never reaches zero.  My if condition is wrong some how,  once the string of chars is printed, it repeats over and over again.  How do I stop this.

 

Thanks,

 

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test 

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test 

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test 

+CMT: "+441234567890","","16/05/23,14:45:51+04"
Test

 

Top
dbrion0606
dbrion0606's picture
Level: Raving Lunatic
Joined: Sat. Jan 7, 2012
Posts: 3693 View posts
Location: North of France
Posted by dbrion0606: Mon. May 23, 2016 - 03:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

well, I do not understand what is the use of BufferCount  and it seems a litlle redundant

 

you just test whether it is == 0  I.e RingBuffer_Is_Empty() or not (arduino's serial.avalaible() ;

 

What would happen if :

you tested with a terminal (typing arbitrary lines)

 

if a character comes in, you put it into another buffer (lets call it "current line") if it is different of \n

if it is \n, you print the line and set its index to zero?

 

 

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 23, 2016 - 03:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dbrion0606 wrote:
well, I do not understand what is the use of BufferCount and it seems a litlle redundant

Completely full = completely empty! That's why Dean keeps a count as well as the pointers.

 

@Tuurbo - add code to print BufferCount along with every character you remove. That is something like:

char countbuf[20];
	while(1)
	{
				
		BufferCount = RingBuffer_GetCount(&Buffer);
				
		if(BufferCount) 
		{ 
                        sprintf(countbuf, "%d\n", BufferCount);
                        putstring_usart_0(countbuf); // I assume you have this kind of function?
			putcr_usart_0(RingBuffer_Remove(&Buffer));
			usart_flush_0();
			
		}

 

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 03:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Clawson,

 

I have added your modifications, after the initial string, it constantly prints random numbers to terminal.

char countbuf[20];
	while(1)
	{
				
		BufferCount = RingBuffer_GetCount(&Buffer);
				
		if(BufferCount) 
		{ 
                        sprintf(countbuf, "%d\n", BufferCount);
                        write_usart_0(countbuf); //putstring_usart_0(countbuf); // I assume you have this kind of function?
			putcr_usart_0(RingBuffer_Remove(&Buffer));
			usart_flush_0();
			
		}

 

CMT: "+441234567890","","16/05/23,16:25:47+04"
Test
1
5962+65+6867157477+8068388689692795198101510571094113117121712071297133110151007109411311711671257124133113721414140149153115731563165169117311773176718518421921197119692052204212121712169

 

Top
frog_jr
frog_jr's picture
Level: Raving Lunatic
Joined: Sat. Jan 15, 2011
Posts: 2227 View posts
Location: Woodway, Texas
Posted by frog_jr: Mon. May 23, 2016 - 03:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tuurbo46 wrote:
BufferCount = 0;

I may be missing something, but using Dean's code the BufferCount is decremented in the Remove routine so I don't see why you want to set it to 0 here. Let the RingBuffer_Remove do the decrement and then keep sending chars in your loop till BufferCount is 0.

 

Also:

  What is sending data to the UART? (In post #19 you say you are manually loading it.)

  What is receiving the data from the UART that you are printing?

David

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104954 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. May 23, 2016 - 03:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What happened to the '\'n I asked for? That output is meaningless.

 

Also what i was expecting to see was something line

 

1

C2

M3

T4

:5

...

 

if it really were the case that BufferCount was increasing. In reality what I was expecting was:

 

1

C1

M1

T1

:1

...

 

because I don't really believe the count will get above 1.

 

So how does:

5962+65+6867157477+8068388689692795198101510571094113117121712071297133110151007109411311711671257124133113721414140149153115731563165169117311773176718518421921197119692052204212121712169

relate to all this?!?

Top
dbrion0606
dbrion0606's picture
Level: Raving Lunatic
Joined: Sat. Jan 7, 2012
Posts: 3693 View posts
Location: North of France
Posted by dbrion0606: Mon. May 23, 2016 - 03:58 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What happens if your consumer eats too  much time?

If a computer/GPS sends characters very fast, and if you echo them + debugging characters : echoing them with a CPU loop will eat some CPU time ; if, on the average, (Ring Buffers are meant to cope with peaks) you eat too much CPU printing, you do not have time to consume incoming characters (that is why I would like to know whether tests are done by typing or with a GPS/PC automatically sending)

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 04:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
+CMT: "+441234567890","","16/05/23,17:09:37+04"
Test
1

60
64
+68
472
76

80
+84
688
892
96

100
4105
7110
2115
120

125
130

135
140
1145
3150
while(1)
	{
	
		BufferCount = RingBuffer_GetCount(&Buffer);
		
		if(BufferCount)
		{
			sprintf(countbuf, "%d\n", BufferCount);
			write_usart_0(countbuf); //putstring_usart_0(countbuf); // I assume you have this kind of function?
			putcr_usart_0(0x0D); 
			putcr_usart_0(RingBuffer_Remove(&Buffer));
			_delay_ms(100);
			usart_flush_0();
			
		}
	
				
		
		
	}

    @ Clawson, I added a '\n' as requested.  I hope the output is now correct.

 

     @ Frog, 

 

    What is sending data to the UART? (In post #19 you say you are manually loading it.).  I am sending a text message to a modem, and the modem sends in initial hello data message to the UART.

 

    What is receiving the data from the UART that you are printing?.  I am printing to a terminal program.

 

 

 

Top
Tuurbo46
Tuurbo46's picture
Level: Hangaround
Joined: Thu. Jun 11, 2015
Posts: 214 View posts
Posted by Tuurbo46: Mon. May 23, 2016 - 04:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@ dbrion0606

Hi,

 

I have the UART connected to the modem using the Tx and Rx line, paralleled off from these lines is a rs232 board going off to hyper terminal .  

 

I am not typing any data into terminal, I am only watching live data coming in from the modem and going out from the UART.  So a text message is sent, the modem receives, and sends to the UART.  I do no typing.

 

 

Top
kk6gm
kk6gm's picture
Level: Raving Lunatic
Joined: Fri. Sep 11, 2009
Posts: 4022 View posts
Location: Sacramento, CA
Posted by kk6gm: Mon. May 23, 2016 - 05:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Try printing BufferCount in your loop when it is != 0.  I notice that the Remove function does not test for an empty buffer (even though your code does), and I just wonder if the count has wrapped around "below" zero.  Or could it be that the PC is echoing the received characters back to the AVR?  In any case, I think printing the count value for each character removed from the buffer might help you debug this.

Top

Pages

View All