5

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?

Update: Here is the entire code if you ever want to take a look. And it's working very well after changing it to what was suggested in the accepted answer.

  • Your question says what the third column of output is ... what are the other columns? Please edit question and add column headings – James Waldby - jwpat7 Dec 16 '15 at 18:06
  • @jwpat7 I intentionally removed them because that will only confuse the reader. But the question has already been answered well by Majenko. – daltonfury42 Dec 16 '15 at 18:21
  • 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 Dec 18 '15 at 0:10
  • @NickGammon Here you go: paste.ubuntu.com/14085127 – daltonfury42 Dec 18 '15 at 4:37
  • 3683 / .056196868 = 65537 so it looks like it incremented at the wrong moment, yes? You are accessing a variable that might be changed in an interrupt multiple times in that code, so getting a local copy, while interrupts are off, would be much safer. – Nick Gammon Dec 18 '15 at 21:27
8

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.

  • 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 Dec 16 '15 at 18:26
  • 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 Dec 16 '15 at 20:48
  • 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 Dec 16 '15 at 20:51
  • 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 Dec 16 '15 at 21:04
  • 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 Dec 16 '15 at 21:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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