Declaring and defining buffer index in avr gcc

Go To Last Post
19 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello. I want to make a truly reusable ATMega UART code, since I make projects which require UART communications. There are many libraries available, e.g., Peter Fluery's lib. This uses ring buffers with UART.  I am using this as a base. I have three files, uart.h, uart.c and the project file main.c. I wish to specify the ring buffer index in my project main.c() while keeping the declaration of the buffer in uart.h file. All UART functions are defined in uart.c, and called from main.c.

After compiling, I get errors:

Error        conflicting types for 'UART_RxBuf'    UART_test    G:\Microcontrollers\AVR\Programs\ATMega328\UART_test\UART_test\uart.c    49

Error        conflicting types for 'UART_TxBuf'    UART_test    G:\Microcontrollers\AVR\Programs\ATMega328\UART_test\UART_test\uart.c    48

Error        expected identifier or '(' before numeric constant    UART_test    <command-line>    0

Error        variably modified 'UART_RxBuf' at file scope    UART_test    G:\Microcontrollers\AVR\Programs\ATMega328\UART_test\UART_test\uart.c    49

Error        variably modified 'UART_TxBuf' at file scope    UART_test    G:\Microcontrollers\AVR\Programs\ATMega328\UART_test\UART_test\uart.c    48

 

The code snippets are as shown:

uart.h
#include <avr/pgmspace.h>
extern unsigned char UART_TxBuf[];	//declare buffers
extern unsigned char UART_RxBuf[];
...<more vaiables & function declarations>


uart.c  // reusable "library" file
#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <stdint.h>
#include "uart.h"

/******constants and macros *******/

/* size of RX/TX buffers */
extern uint8_t UART_RX_BUFFER_SIZE;
extern uint8_t UART_TX_BUFFER_SIZE;
 
#define UART_RX_BUFFER_MASK ( UART_RX_BUFFER_SIZE - 1)
#define UART_TX_BUFFER_MASK ( UART_TX_BUFFER_SIZE - 1)

/* module global variables */
static volatile unsigned char UART_TxBuf[UART_TX_BUFFER_SIZE]; //line 48 ERROR 
static volatile unsigned char UART_RxBuf[UART_RX_BUFFER_SIZE]; //line 49 ERROR
static volatile unsigned char UART_TxHead; //indices to buffers
static volatile unsigned char UART_TxTail;
...<more vaiables & function declarations>

uart_test.c // main project file

#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <stdint.h>
#include "uart.h"

#define F_CPU 16000000UL

//define UART parameters
#define UBRR_DATA 	103 //UBRR value for 9600 baud
#define BAUDx2		0	//U2X = 0

//uint8_t UART_RX_BUFFER_SIZE = 8;	//define buffer size, ^2
//uint8_t UART_TX_BUFFER_SIZE = 8;
#define UART_RX_BUFFER_SIZE 8
#define UART_TX_BUFFER_SIZE 8
#define DEBUG	1	// 1 or 0 to enable or disable error compilations

...<more code>

Also, what is the error showing up in line 0? No file is specified. I have specified F_CPU = 16000000UL in Atmel Studio 7->Project-> Properties->Tools->Symbols->-DF_CPU=16000000UL.

Any help will be appreciated. 

Thanks.
 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
After compiling, I get errors:

You seem to be looking at the Atmel Studio "error list"

 

You will get far better information from looking at the 'Output' window.

 

See: https://www.avrfreaks.net/commen...

 

amorawala wrote:
what is the error showing up in line 0?

it says:

Error        expected identifier or '(' before numeric constant    UART_test    <command-line>    0

So it's referring to something wrong on the command line. ie, not anything in your source code.

 

Again, the 'Output' window would give better information.

 

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 oscillatorhttps://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...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
Error        conflicting types for 'UART_RxBuf'    UART_test    G:\Microcontrollers\AVR\Programs\ATMega328\UART_test\UART_test\uart.c    49

"conflicting" means that there must be another declaration of 'UART_RxBuf' somewhere else - and it gives it a different type.

 

Again, the 'Output' window would give better information - including details of where that other declaration is, and what the conflict is.

 

Actually, it is here:

uart.h
#include <avr/pgmspace.h>
extern unsigned char UART_TxBuf[];	//declare buffers
extern unsigned char UART_RxBuf[];

There you have declared them as 'extern', but in the .c file they are 'static'

 

The whole point of 'static' in this context is that it prevents the item from being visible outside its own source file - so you cannot declare it as 'extern' in a header! 

 

extern uint8_t UART_RX_BUFFER_SIZE;
extern uint8_t UART_TX_BUFFER_SIZE;

Usually, ALL UPPERCASE names are reserved for #defined macros - not variables.

 

Why do you want these to be variables? Usually, they would be compile-time constants.

 

EDIT

 

The 'C' programming language does not allow the length of a non-auto array to be specified by a variable

 

https://www.geeksforgeeks.org/variable-length-arrays-in-c-and-c/

 

(and, if you're using a 'C' version before C99, it does not allow it at all).

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 oscillatorhttps://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...
Last Edited: Sat. Jun 6, 2020 - 09:23 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Having looked over the Peter Fleury UART code. I would say he's gone a long way to making it "Truly Universal" already. All that's missing is support for newer AVRs and helper functions for determining the number of characters in RX & TX buffers.

 

I would suggest what you're doing is wrong. You want a universal library but you're pulling variables that library needs into main. This is absolutely not the way to go, it breaks the modular structure of his code.

 

I'm second guessing here but if your intention is to control the size of those buffers then Mr Fleury already has that covered:

/** @brief  Size of the circular transmit buffer, must be power of 2 
 *
 *  You may need to adapt this constant to your target and your application by adding 
 *  CDEFS += -DUART_TX_BUFFER_SIZE=nn to your Makefile.
 */
#ifndef UART_TX_BUFFER_SIZE
#define UART_TX_BUFFER_SIZE 32
#endif

All you need to do is follow his instructions.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes Winterbottom, Peter has indeed taken pains to make this a reusable code. What I am concerned with is this: I may have concurrent multiple projects using UART, with different buffer lengths, due to RAM constraints or whatever. So I will have to change the buffer size in the uart.h file for each project. That means I will have to copy uart.h and adjust the buffer size for each project. I want to avoid this. That is why I would like to do change the buffer size in my project files, leaving uart.h alone.

 

Just edited my reply: Upon going through the comment in the uart.h file, I can see tat the buffer size can be modified in the makefile for individual projects. 

 

"You may need to adapt this constant to your target and your application by adding * CDEFS += -DUART_TX_BUFFER_SIZE=nn to your Makefile."

 

I use Atmel Studio 7, in which I can do a similar thing from Projects->Properties->tools-> Symbols->-D->UART_TX_BUFFER_SIZE = nn.

 

Am I right Winterbottom? 

Last Edited: Sat. Jun 6, 2020 - 12:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

N.Winterbottom wrote:
All you need to do is follow his instructions.

 

Those instructions are:

 *  You may need to adapt this constant to your target and your application by adding
 *  CDEFS += -DUART_TX_BUFFER_SIZE=nn to your Makefile.

Granted; few still use Makefiles but the functionality can be obtained by an Atmel Studio Project settings like this:

 

 

 

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Great. Thanks Winterbottom. I will check this out and revert. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


N.Winterbottom wrote:

/** @brief  Size of the circular transmit buffer, must be power of 2 

N.Winterbottom wrote:

It looks like those are contradictory...or am I missing something?

David

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh Yeah.

        /* calculate and store new buffer index */
        tmptail = (UART_TxTail + 1) & UART_TX_BUFFER_MASK;

Well that's a WTF; tis a silly UART library:

I'll admit to writing code like that many years ago on Hitachi H8 and thinking I was the bestest programmer in the whole wide world because I'd saved 4 bytes of ROM and a few clock cycles. When I ran out of SRAM because of the oversized buffers, I realised I'd been an idiot.

 

This section deserves a rewrite:

    /* calculate and store new buffer index */
    tmptail = ((UART_TxTail + 1) < UART_TX_BUFFER_SIZE) ? (UART_TxTail + 1) : 0;

Similarly for the RX buffer:

    /* calculate buffer index */
    tmptail = ((UART_RxTail + 1) < UART_RX_BUFFER_SIZE) ? (UART_RxTail + 1) : 0;

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The buffer size has to be a power of 2. Must be a typo, it may be 32. In projects where the comms speed is slow, I may not actually use a buffer. In which case, I can reduce he size to 4 or 8, just so that I can use the code.

As an aside, there is also lot of error checking in Peter Fleury's library. In case I do not want to perform any error checks, can I use something like #define DEBUG, then #ifdef Debug <compile the relevant error check code>. Can I set this DEBUG value also in AS7 -> -D as you had shown? 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

AS7 automatically sets DEBUG if you build "Debug" and NDEBUG if you build "Release"

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

IMHO, the original statement seems fine. These are ring buffers, so '&'ing the next index (index+1) with the mask (which is buff size -1) will wrap the index around to 0 if the index > buffer size. Can you please explain the logic behind the conditional statement?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello Clawson. I am sorry I do not understand how AS7 sets DEBUG. In my projects I aim to use DEBUG as a means to place some debugging points or send some strings to the PC monitor. So I may use this macro DEBUG. How will AS7 set this macro if I do not instruct it? And what is Release?

Please elaborate so I can use this advice in my projects.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not sure how I can be any clearer. The default projects in AS7 do -D's so use them.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok Clawson.

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
The buffer size has to be a power of 2. Must be a typo

The error was in my example  UART_TX_BUFFER_SIZE=24 I missed the power of 2 restriction and would have been caught by Peter Fluery's error checking. Which by-the-way does not add any code because the pre-processor performs the checks.

 

amorawala wrote:
Can you please explain the logic behind the conditional statement?

This is how I would remove the power of two restriction.

 

To examine:

    tmptail = ((UART_TxTail + 1) < UART_TX_BUFFER_SIZE) ? (UART_TxTail + 1) : 0;

The ? is called a ternary operator and the entire statement is a ternary expression. It is a concise way of providing a conditional value.

 

  1. The part before the ? is evaluated as a boolean expression
  2. If true; the expression between ? and : is used as the result of the ternary expression. (and loaded into tmptail)
  3. If false; the expression after : is used as the result of the ternary expression. (and loaded into tmptail)

 

In it's pure form it is a single statement but some abuse it by including intermediate statements or even nesting ternary operators where an IF; THEN; ELSE; would serve better.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

For non power of 2 buffer size you can of course do the plain and simple

    tmptail = tail + 1;
    if (tmptail >= BUFFER_SIZE)
        tmptail = 0;

It's unlikely to compile to more code than writing as ternary. If anything, could be less code, depending on how smart or not so smart the compiler is, and also whether tail is volatile or not.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
I may have concurrent multiple projects using UART, with different buffer lengths

So what's the problem there, then?

 

If you have multiple projects, then each project just sets the size definitions as required.

 

As Neil said in #4, that is pretty much the whole point of a library!

 

And don't forget, in Atmel Studio:

  • A Solution can have multiple Projects - each with its own configuration options;
  • As well as the standard 'Build' and 'Release', you can add your own Project configurations - which could vary the buffer size ...

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 oscillatorhttps://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...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sometimes all the "window dressing" is almost as much work as the project itself.  You can get the basic code running quickly & simply, but then envision you'd like it to be extra flexible for a million circumstances (selectable chips, different IO config, bigger buffer, no buffer, multiple instances, etc)...Then after all that, you end up never using all (or any) of the flexibility you fought for.   The boss just says, what took so long to get it to send out hello?

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!