Very weird volatile variable issue from main

Projects Programming
1

Hi,
first thank you so much for lookng.

I just got out of arduino and into programming microcontrollers with an avr programmer. I want to go bare bones so I started with writing my own timer header file for the internal or an external clock.

here is the code for the interrupt timer.

volatile uint32_t milliSec=0; //number of milliseconds passed. volatile because it is
                              // modified in ISR but read in other functions and in main.
static uint16_t __f_=0; //Fractional number of milliseconds passed. global to reduce 
                   //code and static to prevent bugs from main.
                   uint32_t j=0; 
ISR(TIM0_OVF_vect)
{
  //toggleBit(&PORTA,1);                                                    
  __f_ += __fmpi_;
  if( __f_ < __fmax_ )
  {
    milliSec += __mpi_;
  }
  else
  {
    __f_ -= __fmax_;
    milliSec += (__mpi_ + 1); //plus 1 because fraction has overflowed. we are playing catchup
  }
  //if((milliSec-j) > 250)
  //{
   // toggleBit(&PORTA,1);
    //j = milliSec;
 // }
  //if else to acess volatile variable from memory only once to reduce overhead.
  //if fraction is less than or equal to __fmax_ then only __mpi_ has passed,
  // else fraction is over __fmax_ so another millisecond has passed. 
}
//Interupt service routine for Timer overflow on timer 0
//runs every __TICKPERINTERUPT_*__PRESCALER_ clock cycles. Use this to detmine accuracy.

f is the fraction millisecond count
fmpi is fraction millisecond per interrupt
mpi is milli per interrupt
and fmax is the max of the fraction.

Looking at the code it seems like it would run perfectly. My issue comes when I try to access the volatile variable milliSec from main. for some reason the program completely ignores the variable but only in main.

here is an example of my main code to test the timer. Has yet to work.

uint32_t I=0,boolean=1; 
int main
{
while(boolean);
{
  if((milliSec-I) > 250)
  {
    toggleBit(&PORTA,1);
    I = milliSec;
  }
return 0;
}

Now here comes the crazy part. When I run that loop in main the led I have connected does not toggle. However, when I comment out the loop in main and uncomment the if statement in the timer.h ISR it works perfectly and the led does toggle for 250 milliseconds. I say perfectly because I am able to access a real reading from the variable milliSec, which allows my delay if statement to work correctly, thus toggling the led.

So basically my question is ... Does anyone understand why i am able to read milliSec in the timer.h header file but when I try to read from main it always comes out as 0. Thanks everyone.

Basically I thought volatile was supposed to fix this.

I would love for someone to upload it and try it for me and see if we get the same error in main.

Thanks once again!

3

Post all your code. Snippets are generally useless and especially so in this case.

4

You got it! Thanks for looking.

Here is the header file

// Description: This header file includes a 32 bit volatile global variable milliSecO
// that counts milliseconds using an interrupt service routine for the Timer overflow
// interrupt.
//
// I used Nanoseconds because it makes it more accurate and comparisons are
// promoted to 16 bit anyway.
//
// Works with little error for multiples of prescaler. Recommend 8 and 16 Mhz with prescaler 8.
//
// Pre: 1) Global interrupts must be enabled on the microchip
//      2) A Timer overflow interrupt must be enabled
//      3) The clockspeed of the microcontroller must be defined by 
//               #define __CLOCKSPEED_ (x) ... x is in khz
//      4)The clock prescaler of the timer clock must be defined by
//               #define __PRESCALER_ (x) ... x is the prescaler value for the clock
//        The timer/counter must at least be enabled with a prescale of 1. 
//      5)The interrupt vector of the selected timer must be defined by
//               #define __VECTOR_ (x_vect) ... x is the interrupt vector
//
// Post: The global variable of 32 bit size is named milliSec for milliseconds.
// Overflows every 2^32 milliseconds or 48.71 days
//
// Options: A) Can be used with external crystal of any freq above 1khz.
// Pre: 1) Global interrupts must be enabled on the microchip
//      2) A corresponding interrupt must be enabled (pin or overflow depending on use)
//      3) __CLOCKSPEED_ is freq of crystal
//      4) __PRESCALER_ must be 1 for external crystal or interrupt
//      5) __VECTOR_ is the new interrupt vector or same overflow interrupt
//      6) __TICKPERinterrupt must be 1 if not using an 8 bit counter (pin interrupts)
//          B) milliSec can be put into a function if you feel it is safer. I have it global
//             to reduce overhead.
//          C) You can also change the size of milliSec if you want a longer duration.
//             I, however, recommend checking the accuracy over that longer time period.
//      

#if !defined(__CLOCKSPEED_) || !defined(__PRESCALER_) || !defined(__VECTOR_)
#error \
"Must define __CLOCKSPEED_, __PRESCALER_, and __VECTOR_. Please see beginning of header file."
#endif
//error generated if CLOCKSPEED, PRESCALER, and VECTOR are not defined to reduce user error

#ifndef timer_h
#define timer_h
//defines header file so it is included once.

#ifndef __TICKPERinterrupt_
#define __TICKPERinterrupt_ 256 //assumes timer is interrupting on 8 bit counter.
#endif
//defines __TICKPERinterrupt_ if not defined by user. Assumes running on 8 bit counter,
// so 255 + overflow then interrupt is 256 counts.

#define khz2Micros10(scaler) (((scaler) * 10000L) / __CLOCKSPEED_)
//converts clockspped in khz to microseconds

#define __mpi_ ((khz2Micros10(__TICKPERinterrupt_ * __PRESCALER_)) / 10000)
#define __fmax_ (10000) //fmax is 15625 so it will fit in a 16 bit memory slot.
#define __fmpi_ ((khz2Micros10(__TICKPERinterrupt_ * __PRESCALER_)) % 10000) 
//modulo to find remainder of fraction of milliseconds then 
// divide by 64 because we want it in scale of __fmax_.
//define milliseconds per interrupt as __mpi_ and fraction of millisecond per interrupt as 
// __fmpi_, and also defines the max of fraction of millisecond per interrupt as __fmax_

volatile uint32_t milliSec=0; //number of milliseconds passed. volatile because it is
                              // modified in ISR but read in other functions and in main.
static uint16_t __f_=0; //Fractional number of milliseconds passed. global to reduce 
                   //code and static to prevent bugs from main.
                   uint32_t j=0; 
                   uint32_t milli(void);
ISR(TIM0_OVF_vect)
{
  //toggleBit(&PORTA,1);                                                    
  __f_ += __fmpi_;
  if( __f_ < __fmax_ )
  {
    milliSec += __mpi_;
  }
  else
  {
    __f_ -= __fmax_;
    milliSec += (__mpi_ + 1); //plus 1 because fraction has overflowed. we are playing catchup
  }
  //if((milli()-j) > 250)
  //{
    //toggleBit(&PORTA,1);
    //j = milli();
  //}
  //if else to acess volatile variable from memory only once to reduce overhead.
  //if fraction is less than or equal to __fmax_ then only __mpi_ has passed,
  // else fraction is over __fmax_ so another millisecond has passed. 
}
//interrupt service routine for Timer overflow on timer 0
//runs every __TICKPERinterrupt_*__PRESCALER_ clock cycles. Use this to detmine accuracy.

//
//
//

uint32_t milli(void)
{ return milliSec; }

#endif

here is my full c file

//
//
//
//
//

#include <avr/io.h>
#include <avr/interrupt.h>
//Defines register values and interupt routines

#include "/Users/bradleybare/Desktop/avr/bare_include/bare_include/bits.h"
// --defines two functions--
//   toggleBit(data byte,bit number)
//   getBit(data byte, bit number)

#define __CLOCKSPEED_ 8000
#define __PRESCALER_ 8
#define __VECTOR_ TIM0_OVF_vect
#include "/Users/bradleybare/Desktop/avr/bare_include/timer.h"
//inculde a timer in milliseconds defined as milliSec


int main(void)
{
  DDRA = 0b00000111; // sets port A bit 0,1,2 as output
  PORTA= 0x01; // sets all pins on port A to off
  //volatile unsigned char x=4;
    
  SREG = 0b10000000;
  TIMSK0 = 0b00000001;;  
  TCCR0B = 0b00000010;
  uint32_t I=0,boolean=1; 

  while(boolean); 
  {
    if((milli()-I) > 250)
    {
      toggleBit(&PORTA,1);
      I = milli();
    }
  }          
return 0;
}

Hope we can figure this out. I also tried to declare a function to return the value of millis from the header file but I still do not get any readable value in main. My LED never blinks, unless the if statement is in the ISR.

5

Why does this work in wiring.c but not in my file? coding probelms like this really confuse me.

6

This doesn't obey the rules for a standard Arduino program, which has a hidden main() and requires the user to define setup() and loop().

I don't know what happens with the Arduino IDE in this case, in particular several timer interrupts are predefined in the Arduino IDE and you may have conflicts.

You would be better off using AVR Studio for this code (the old version IV is very easy to use and would be fine for this project).

7

here is my full c file

  while(boolean); 
  {

Extra semicolon in the while statement!

If I might offer some stylistic comments...

.h files should not include actual code. Your ISR code and milli() function should be in a (possibly separate) .c file.
(which makes a lot of the symbols local...)

  if( __f_ < __fmax_ )

Use of underscores: This is pretty ugly. There is a "convention" to prefix global symbols with underscores, BUT:

  1. you've done it on a lot of symbols that are NOT global (or should not be global, anyway.)
  2. and I think the convention says that double-underscores are for compiler and standard library functions, and single-underscores are for things closer to user libraries.
  3. Symbols that start with double-underscores per this convention should also end with double-underscores.
ISR(TIM0_OVF_vect)

This probably needs to be TIMER0_OVF_vect

// I used Nanoseconds because it makes it more accurate and comparisons are
   :
//               #define __CLOCKSPEED_ (x) ... x is in khz

Math in nanoseconds for accuracy, but CLOCKSPEED truncated to kHz?
Also, F_CPU is sort of an existing avr-gcc "standard" symbol that covers this.

  SREG = 0b10000000;
  TIMSK0 = 0b00000001;; 
  TCCR0B = 0b00000010;

Don't use "magic numbers." io.h defines nice symbols for all the peripheral bits, and you should use them.

uint32_t I=0, boolean=1;

"boolean" is SO likely to be used somewhere as a type name that it makes me very uncomfortable that you've used it as a variable name. (also int32??! you make out ok here since it stays constant, but a true/false value on AVR should be int8!) You've actually managed to obscure your infinite loop here: "while (1)" or "while (TRUE)" is instantly recognizable as an infinite loop. "while (boolean)" I'm not so sure about.

I'm not fond of "I" either (be descriptive!)

8

I should have come back and said I fixed it by rewriting the while and found the issue. Thank you for finding it!

Most of the names I do with the underscores because I don't want to accidentally name a variable the same as something that I defined just to do math. I will look into this, I did not know it was ugly I felt the code was still readable.

I did not realize about the F_CPU. I will look into that too since it would eliminate the user defining another variable.

The magic numbers were just for testing because I had an issue.

Thanks again for the help and criticism.