Sign up ×
Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. It's 100% free, no registration required.

I was doing a small project with an Arduino Uno. It involved interrupts as I am using encoders to measure how much the differential wheel system moves forward. My robot moves only forward. So I use only a single channel from each encoder. Here are my two interrupt routines:

ISR (INT0_vect){

      encoderRPos = encoderRPos + 1;     

}

ISR (INT1_vect){

  encoderLPos = encoderLPos + 1;

}

The variables encoderRPos and encoderLPos are of type volatile int. I understand that the variables which undergoes change in any interrupt routine need to be of type volatile. This is to warn other parts of the code that use these variables that it may change anytime.

But what happened in my code was a bit strange and I could not explain it. Here is how I compute the distance moved by the left wheel:

  #define distancePerCount 0.056196868  
  float SR = distancePerCount * (encoderRPos - encoderRPosPrev);
  float SL = distancePerCount * (encoderLPos - encoderLPosPrev);

  encoderRPosPrev = encoderRPos;
  encoderLPosPrev = encoderLPos;

But when I print the following on my serial monitor, I notice an anomaly:

enter image description here

If you look at the third column, (SL) it's value is too high just for some time. This is upsetting all my calculations.

The only clue I can get it, if I take the value of SL that I got (3682), which is always a constant, and calculate back (encodeLPos - encoderLPosPrev), I will get 65519.66, which is close to the max value of unsigned int. Which means (encoderLPos - encoderLPosPrev) is causing an overflow while both the values whose difference is taken is somewhere around 5000 only!

And I managed to solve it. It was by luck. This is how I modified the code:

  static int encoderRPosPrev = 0;
  static int encoderLPosPrev = 0;

  int diffL = (encoderLPos - encoderLPosPrev);
  int diffR = (encoderRPos - encoderRPosPrev);

  float SR = distancePerCount * diffR;
  float SL = distancePerCount * diffL;

  encoderRPosPrev = encoderRPos;
  encoderLPosPrev = encoderLPos;

I can't comprehend what has happened. Is there something about volatile variables that I should have known about?

share|improve this question
    
Your question says what the third column of output is ... what are the other columns? Please edit question and add column headings – jwpat7 yesterday
    
@jwpat7 I intentionally removed them because that will only confuse the reader. But the question has already been answered well by Majenko. – daltonfury42 yesterday
    
It is hard to give detailed answers from your snippets. could you explain why it is not happening randomly but at a specific time every time I run the code? Also why does it give the particular value? - I could probably do that if I saw the whole code. Meanwhile read this: gammon.com.au/interrupts – Nick Gammon 4 hours ago
up vote 6 down vote accepted

You need to learn about critical sections.

What is probably happening is that the variables are being changed by the interrupt routines mid-way through the calculations. Your 'fix' reduces the time spent doing the calculation with the volatile variables thus making it less likely that there is a collision.

What you should do is copy the volatile variables to local variables with interrupts disabled for that brief period.

cli();
int l = encoderLpos;
int r = encoderRpos;
sei();

Because the Arduino is an 8 bit CPU it takes multiple assembly instructions to perform mathematical operations on 16 bit values. Floating point is even worse using many many instructions for a simple addition. Division and multiplication use considerably more. An interrupt has plenty of opportunity to fire during that list of instructions. By doing an assignment like that and then using the new local variables in your calculations the instructions needed to deal with the volatile variables is kept to an absolute minimum. By turning off interrupts during the assignment you guarantee that the variables can't ever be changed while you are using them. This snippet of code is called a critical section.

share|improve this answer
    
This might just be the case. Just wondering, could you explain why it is not happening randomly but at a specific time every time I run the code? Also why does it give the particular value? – daltonfury42 yesterday
    
Here is a great reference to the cli/sei. nongnu.org/avr-libc/user-manual/…. With the memory barrier volatile declaration is not really needed in the code above. Here is some fun reading on this subject. kernel.org/doc/Documentation/volatile-considered-harmful.txt – Mikael Patel yesterday
1  
@MikaelPatel Nice, but not that relevant for MCUs. Volatile is required in this situation to prevent the compiler from optimzing out instances where it thinks it's not being used (value never changes). The cli/sei is there to make the operation atomic WRT the only other thread (interrupts) that execute. – Majenko yesterday
    
Did you try compiling the code with and without volatile? But with the critical section (cli/sei). What I am trying to discuss is the concept of the memory barrier and how that provides volatile access (and correct ordering) from the compiler with having to declare variables as volatile. Most programmers are taught that any variable accessed in an ISR must be declared volatile but there is so much more to this story. – Mikael Patel yesterday
    
I don't think the compiler has much concept of what cli() and sei() do and how that would affect things like optimizing out of variables that shouldn't be optimized out. All sei() and cli() do is manipulate the global interrupt enabled flag in its register. They do nothing for the flow of code. – Majenko yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.