volatile: use it the right way and break your code

Hey

I’m need help for my Arduino Firmware. I use a rainbowduino (arduino168 based) to drive my led matrix - so far so good. I tried to improve my firmware after reading some topics about the volatile keyword:

A variable should be declared volatile whenever its value can be changed by something beyond the control of the code section in which it appears, such as a concurrently executing thread. In the Arduino, the only place that this is likely to occur is in sections of code associated with interrupts, called an interrupt service routine.

I do have an interrupt part, and two variable which gets modifies in the interuppt handler AND in other methods.

The code can be viewed here:
http://code.google.com/p/neorainbowduino/source/browse/trunk/rainbowduinoFw/Rainbow_V2_71/Rainbow_V2_71.pde

Now I added the volatile keyword to my variables line and level - the result is - its all messed up.

The qustion is why? Did my code only works fine by randomness? Did I misunderstand the volatile keyword? Or its because of the flexitimer lib?

regards
michu

How did you add the volatile keyword to?

Why are line and level sometimes global variables and sometimes local variables? I think that to avoid confusion you should use different names for the global and local variables.

the result is - its all messed up.

Well, that's obvious.

No, wait a minute. No it's not.

What exactly is "messed up"? How is it (whatever it is) "messed up"?

thanks pauls for your reply.

Ouch! thats kinda bad - you’re right. I’ll fix that and test it this evening!

thanks

Ok I cleaned up my code and removed to conflicting local/global variables. NOW.. the volatile option works fine now for the line/level variables, but NOT for the variable g_bufCurr. If I declare this variable as volatile, my rgb matrix starts to flicker and its brightness is reduced... And no, I don't think thats the normal behavior, as the flickering happens about 15 times per second (and my pwm is at 10khz). any clues?

regards

The use of the volatile keyword prevents the compiler from optimizing the variable out of existence. This may be slowing your code to the point where flicker is the result.

for (color=0;color<3;color++) {
    for (row=0;row<8;row++) {
      for (dots=0;dots<4;dots++) {
        //format: 32b G, 32b R, 32b B
        buffer[!g_bufCurr][color][row][dots]=imageBuffer[ofs++];  //get byte info for two dots directly from command
      }
    }
  }

In this code, the optimizer would typically note that g_bufCurr is never modified, so it would be read only once. The volatile keyword says that this is not necessarily true, so the value needs to be accessed fresh every time.

I’m not familiar with your code, or what it is trying to do. It may be that the risk of g_bufCurr changing as a result of an interrupt firing that you need not worry about making it volatile.

I use the volatile key because the variable gets modified in the regular code and about 10000 times per second in the interrupt code. Without using volatile keyword, the variable may get changed in the interrupt code block - and it get not notified in the "regular" code.

I'll do some more debugging - but thanks for your help!

Can you explain what this:

case 1:     
{      
   open_line1; 
   break; 
 }

is doing?

That kind of voodoo magic ;) no serious, this is part of the led pwd stuff - this code is from seeedstudios, and i'm not very familiar with that! the definition can be found here:

http://code.google.com/p/neorainbowduino/source/browse/trunk/rainbowduinoFw/Rainbow_V2_71/Rainbow.h

Nasty.
It’s usually a good idea to put macros like that in capitals, to make it obvious it’s a macro and you’re not just dereferencing a function pointer for the fun of it, or evaluating a variable and discarding the result.

I think an enclosing “do…while(0)” is in order for the macro definition.