Embedded C, and AVR GCC compilation issue with multiple definitions

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

It seems that my compiler does not willing to build the firmware in stepper_motor.c and as you can see below, it throws this error, which I am unable to solve. I have created a stepper_motor.c source where all my functions and variables are placed, and a stepper_motor.h header where all the #defines and function prototypes are. In >> main.c << I just include the "stepper_motor.h" header and use the functions. As you can see from shell log data, it does not complile and tells that 4 x variables are defined multiple times. Those ones are used in ISR() routine for a timer, so they need to be also volatile.

Any information would be highly appreciated on this issue.

 

this is my main.c includes: 

#include <avr/io.h>
#include <Hardware_Bay.h>
#include <util/delay.h>
#include <USART.h>
#include <string.h>
#include <inttypes.h>
#include <stdint.h>
#include <stdbool.h>
#include <avr/interrupt.h>
#include <scale16.h>
#include <stepper_motor.h>

const uint8_t motor_Phases[] = {

(1 << COIL_B1),                             
(1 << COIL_B1) | (1 << COIL_A2),            
(1 << COIL_A2),                             
(1 << COIL_A2) | (1 << COIL_B2),            
(1 << COIL_B2),                             
(1 << COIL_B2) | (1 << COIL_A1),            
(1 << COIL_A1),
(1 << COIL_A1) | (1 << COIL_B1),
};
volatile uint8_t stepPhase = 0;
    volatile uint16_t stepCounter = 0;
    volatile int8_t direction = FORWARD;
    
    

   ISR(TIMER0_COMPA_vect){
                                                                     
    stepPhase += direction;                                                                   
    stepPhase &= 0b00000111;                                                           
    STEPPER_PORT = motor_Phases[stepPhase];                                            // write phase out to motor COIL-1 A/B
    HALL_PORT = motor_Phases[stepPhase];                                               // write phase out to motor COIL-2 A/B
    stepCounter ++;
}

-----------------------------header file -----------------------------------------------

#ifndef STEPPER_MOTOR_H_INCLUDED
#define STEPPER_MOTOR_H_INCLUDED



 #endif // STEPPER_MOTOR_H_INCLUDED




#define FORWARD           1                      
#define BACKWARD         -1
#define TURN              200  

#define MAX_DELAY         255                                                  
#define MIN_DELAY         10                                                      
#define ACCELERATION      16                                        

#define RAMP_STEPS        (MAX_DELAY - MIN_DELAY) / ACCELERATION


void stepperDrive(uint8_t number_of_steps, uint8_t delay);

void trapezoidDrive_Stepper(int16_t number_of_steps);

 

avr-gcc -Wl,-Map,Physalis_GEO_version_3.6.map -Wl,--gc-sections -mmcu=atmega1284p fuse.o main.o stepper_motor.o USART.o -o Physalis_GEO_version_3.6.elf stepper_motor.o: In function stepperDrive':
D:\Users\Arhitect\Documents\C_Programs\Physalis_Banshee\Physalis_GEO_version_3.6/stepper_motor.c:50: multiple definition ofstepCounter' main.o:(.bss.stepCounter+0x0): first defined here stepper_motor.o:(.data.direction+0x0): multiple definition of direction'
main.o:(.data.direction+0x0): first defined here
stepper_motor.o:(.rodata.motor_Phases+0x0): multiple definition ofmotor_Phases' main.o:(.rodata.motor_Phases+0x0): first defined here stepper_motor.o: In function stepperDrive':
D:\Users\Arhitect\Documents\C_Programs\Physalis_Banshee\Physalis_GEO_version_3.6/stepper_motor.c:50: multiple definition ofstepPhase' main.o:(.bss.stepPhase+0x0): first defined here make: *** [Physalis_GEO_version_3.6.elf] Error 1 PS D:\Users\Arhitect\Documents\C_Programs\Physalis_Banshee\Physalis_GEO_version_3.6>

 

This topic has a solution.

work in progress...

Last Edited: Sun. Feb 2, 2020 - 12:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dave_Zeb. wrote:
It seems that my compiler does not willing to build

Actually, it's the Linker - not the compiler - that's complaining.

 

It tells you where the 2 definitions are - so you need to decide which one to keep, and which is just a duplicate.

 

EDIT

 

Take a look at the diagrams in these posts to understand how linker & compiler relate:

 

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

 

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

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. Feb 1, 2020 - 12:46 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The error is very clear isn't it? It says both main.c and stepper_motor.c both create an object called stepCounter. Sounds a lot like you have definitions not just declarations in a shared .h file. Did you forget "extern"?

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




#include <avr/io.h>
#include <Hardware_Bay.h>
#include <stepper_motor.h>

extern const uint8_t motor_Phases[] = {

    (1 << COIL_B1),                             
    (1 << COIL_B1) | (1 << COIL_A2),            
    (1 << COIL_A2),                             
    (1 << COIL_A2) | (1 << COIL_B2),            
    (1 << COIL_B2),                             
    (1 << COIL_B2) | (1 << COIL_A1),            
    (1 << COIL_A1),
    (1 << COIL_A1) | (1 << COIL_B1),
};

extern volatile uint8_t stepPhase = 0;//
extern volatile int8_t direction = FORWARD;
extern volatile uint16_t stepCounter = 0;

void stepperDrive(uint8_t number_of_steps, uint8_t delay){

    OCR0A = delay;                              
    stepCounter = 0;                           
    TIMSK0 |= (1 << OCIE0A);                    
    while(!(stepCounter == number_of_steps)){;  
    }
    TIMSK0 &= ~(1 << OCIE0A);                   
}

void trapezoidDrive_Stepper(int16_t number_of_steps){

    uint16_t delay = MAX_DELAY;
    uint16_t stepsTaken = 0;

    if(number_of_steps > 0){

        direction = FORWARD;
    }
    else{
        direction = BACKWARD;
        number_of_steps = -number_of_steps;
    }

    if(number_of_steps > (RAMP_STEPS * 2)){ 
        while(stepsTaken < RAMP_STEPS){    
                stepperDrive(1, delay);     
                delay -= ACCELERATION;
                stepsTaken ++;
        }                                   

        delay = MIN_DELAY;
        stepperDrive((number_of_steps - 2 * RAMP_STEPS), delay);
        stepsTaken += (number_of_steps - 2 * RAMP_STEPS);
                                                                
        while(stepsTaken < number_of_steps){
                stepperDrive(1, delay);
                delay += ACCELERATION;
                stepsTaken ++;
        }
    }
    else{                                                   
        while(stepsTaken <= number_of_steps / 2){
            stepperDrive(1, delay);
            delay -= ACCELERATION;
            stepsTaken ++;
        }
        delay += ACCELERATION;
        while(stepsTaken < number_of_steps){
            stepperDrive(1, delay);
            delay += ACCELERATION;
            stepsTaken ++;
        }
    }
}

I checked the both files. and there is only one place where these variable are declared, that file is the "stepper_motor.c" as posted above.

now, when I'm building the error has changed itself to the one below.

PS D:\Users\Arhitect\Documents\C_Programs\Physalis_Banshee\Physalis_GEO_version_3.6> make flash
avr-gcc -Os -g -std=gnu99 -Wall -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -ffunction-sections -fdata-sections -DF_CPU=20000000UL -DBAUD= -I. -I../../Physalis_GEO_version_3.6 -mmcu=atmega1284p -c -o main.o main.c
main.c: In function '__vector_16':
main.c:666: error: 'stepPhase' undeclared (first use in this function)
main.c:666: error: (Each undeclared identifier is reported only once
main.c:666: error: for each function it appears in.)
main.c:666: error: 'direction' undeclared (first use in this function)
main.c:668: error: 'motor_Phases' undeclared (first use in this function)
main.c:670: error: 'stepCounter' undeclared (first use in this function)
make: *** [main.o] Error 1
PS D:\Users\Arhitect\Documents\C_Programs\Physalis_Banshee\Physalis_GEO_version_3.6>




and that place is where the ISR() is written, why the ISR() does not see these variables which are declared in "stepper_motor.c" source file as extern=?

 

ISR(TIMER0_COMPA_vect){
                                                                         // Timer/Counter-0 Compare match interrupt vector enable
        stepPhase += direction;                                                                   // take step in right direction
        stepPhase &= 0b00000111;                                                           // keep the stepPhase in range (0 - 7)
        STEPPER_PORT = motor_Phases[stepPhase];                                            // write phase out to motor COIL-1 A/B
        HALL_PORT = motor_Phases[stepPhase];                                               // write phase out to motor COIL-2 A/B
        stepCounter ++;
}

 

work in progress...

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

You can’t have an initialiser with an extern.
Eg: extern int a;
Not extern int a=1;

There was probably some errors that you didn’t show us.

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

Dave_Zeb. wrote:
I checked the both files

remember that we can only see what you include in your posts.

 

. and there is only one place where these variable are declared

The Linker is complaining about definitions - it does not see declarations.

 

http://c-faq.com/decl/decldef.html

 

As clawson suggests, if you have a definition (rather than a declaration) in a header file - that will result in that definition appearing in each & every .c file which #includes it.

 

, that file is the "stepper_motor.c" as posted above.

In the OP, you said the file was called main.c - we have not seen  "stepper_motor.c"

 

 

 

why the ISR() does not see these variables which are declared in "stepper_motor.c" source file as extern=?

I think you need to go back to your 'C' textbook, to understand the meaning of 'extern'

 

The 'extern' needs to be in the which is "receiving" the variables - not the one which is "publishing" them.

 

Again, see: http://c-faq.com/decl/decldef.html

 

Also see this thread:  https://www.avrfreaks.net/forum/codevision-isr-and-variables

 

All this is standard 'C' stuff - nothing specific to AVR.

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

It's difficult to read your build output due to poor formatting but it appears you may have included stepper_motor.o twice.

 

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

As Kartman says you cant have initialisers on external but anyway the whole idea is that you put the external declaration in a .h file. Every compilation unit that includes that header can then "see" and access the common object. You DEFINE the object in just one compilation unit. It does not matter (as long as the types match!) that the same compilation unit might also include the .h with the extern declarations.

 

It might be worth learning the basic C stuff in a non AVR environment then apply what you learn later to AVR development.

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

clawson wrote:
It does not matter (as long as the types match!) that the same compilation unit might also include the .h with the extern declarations.

In fact, it is a Good Thing if the .c file with the definitions does #include the  .h with the extern declarations!

 

This will ensure that the compiler can detect any inconsistencies between the definitions and  the extern declarations!

 

It might be worth learning the basic C stuff in a non AVR environment then apply what you learn later to AVR development.

Absolutely!

 

Here are some 'C' learning & referece materials - including a free online textbook:

 

http://blog.antronics.co.uk/2011...

 

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

>does not see these variables which are declared in "stepper_motor.c" source file

 

No 'c' file can see another 'c' file. They are compiled separately and have no knowledge of anything but themselves. Which is where headers enter the picture- they inform a 'c' file about things it does not know.

 

 

//led.h

void blinkLed(); //this declaration is all anyone needs to know to blink the led (so use #include "led.h" when you want to blink an led in any c file)

 

//led.c

#include "led.h" //whether needed or not (in this case not), it will make sure the header is correct (declaration matches definition)

void blinkLed(){ /* do something here */ } //here is our function we are letting other c files use (via them including led.h)

 

//main.c

#include "led.h" //this main.c file knows nothing about blinkLed(), now it knows what it looks like, and will take your word that it will exist

int main(){

    blinkLed(); //we hope there really is a function like that

}

 

now compile- led.c compiles ok, main.c compiles ok, the linker now connects everything together. If you happened to forget to add the led.c file to your project, the linker will complain (error) because you told the compiler (when compiling main.c) that there would be a function called ledBlink 'somewhere else', but the linker cannot find it. 

 

The sooner you can square away the compile/link process, and realize each c file is compiled separately, you can get to a point where you can better understand why a compiler or linker is producing errors. It will also make your use of headers become easier to understand/create.

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

The include guard in your header file makes little sense. I'm surprised no one has brought this up. You are effectively not using any include guard. That probably causes some warnings during compilation.

 

The header file should look like this:

#ifndef STEPPER_MOTOR_H_INCLUDED
#define STEPPER_MOTOR_H_INCLUDED

#define FORWARD           1                      
#define BACKWARD         -1
#define TURN              200  

#define MAX_DELAY         255                                                  
#define MIN_DELAY         10                                                      
#define ACCELERATION      16                                        

#define RAMP_STEPS        (MAX_DELAY - MIN_DELAY) / ACCELERATION


void stepperDrive(uint8_t number_of_steps, uint8_t delay);

void trapezoidDrive_Stepper(int16_t number_of_steps);

#endif // STEPPER_MOTOR_H_INCLUDED

 

/Jakob Selbing

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

As well, the need for externs suggests some architectural improvements are needed.
Those variables should have no direct visibility outside the C file.

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

Dear all,

after all, I was able to resolve the issues with modules and shared variables from various source files with the help of all of you. Many thanks for your feedback and assistance. Now I learned to use the "extern" and developing modular programs hehe.  Great to be a member of this amazing community.

Have a nice rest of weekend everyone.

 

work in progress...