2

I connected the output from an encoder motor to int0. this is used to increment a counter at every pulse. in the loop(), I check this counter and stops the motor if it is above certain value. But this "if statement" works erratically. The while loop should break at 380, but sometimes it breaks at 256.

Serial.print(cnt);
cnt=0;
Serial.print("  ");
Serial.print(cnt);
motor1_up(255);
motor2_up(255);
while(1)
{
    if(cnt>380)
    {
        Serial.print(" break ");
        Serial.print(cnt);
        break;
    }
}
motor1_down(255);
motor2_down(255);
Serial.print("  ");
delay(40);
Serial.println(cnt);
motor1_down(0);
motor2_down(0);
delay(2000);

Here is the serial output

 1  0 break 381  389
 396  0 break 256  264
 271  0 break 381  389
 396  0 break 381  389
 396  0 break 256  264
 270  0 break 381  389
 397  0 break 381  390
 396  0 break 381  389
 397  0 break 381  389
 396  0 break 256  264
 271  0 break 381  389
5
  • 3
    it is difficult to tell what the issue could be without seeing the entire code. Also the line with while(1) on it looks awful, separate it onto several lines.
    – user588
    Jun 15 '14 at 19:49
  • 1
    Welcome to Arduino SE! It's really hard to answer this question with such a small code snippet. It's fine if you've isolated the problem, but you don't explain the code much. For example, what is the motor1_down() for? It's fine to remove snippets that aren't useful (i.e. polling from a light sensor in this scenario), but it's generally helpful to have all the libraries/methods/etc. Can you please edit your code? Thanks! Jun 15 '14 at 21:57
  • I've just come back to have another look at this. It is very unclear what it is you are doing. Is this an ISR? If so, you've go a locking while(1) in there. If it's not, then when does this get called? Please post the full code
    – Madivad
    Jun 16 '14 at 7:35
  • @madivad I guess that for a rotary encoder you have no other option than use interrupts, otherwise you may miss ticks, depending on the encoder steps and the motor RPM. However, the posted code is obviously part of loop().
    – jfpoilpret
    Jun 16 '14 at 9:40
  • Ok, I see it now, I misread the question (and so I have removed my answer).
    – Madivad
    Jun 16 '14 at 21:51
5

Not only must cnt variable be declared volatile, but accessing it from the main loop must also be protected because:

  • it can be modified by an ISR
  • it is longer than 1 byte

In AVR architecture (8-bits), read/write operations on bytes are guaranteed to be atomic, i.e. no function can access a byte while it is read/written by an ISR.

However, operations on types larger than bytes are not atomic!

This means that reading e.g. an int variable from your main loop might be interrupted in the middle of the operation (only 1 byte read) by an ISR that could modify the value (both bytes this time); then when the ISR is finished the main loop would resume reading the second byte -modified by the ISR- of the variable, while keeping the first original byte -not modified by the ISR.

I would advise you to write a pair of functions to read and write cnt in a protected way, i.e. with interrupts disabled:

volatile int cnt;

void set_cnt(int val) {
    uint8_t status = SREG;
    cli();
    cnt = val;
    SREG = status;
}

int get_cnt() {
    uint8_t status = SREG;
    cli();
    int val = cnt;
    SREG = status;
    return val;
}

What those functions do are:

  1. get the current status of exceptions (from SREG register)
  2. disable exceptions with cli()
  3. access cnt without any risk of it being accessed concurrently
  4. restore initial status of exceptions (to SREG register)

Everywhere in your code (except in code called from an ISR where it is not necessary), you must then call these functions to access cnt variable.

2

It looks like it could be a caching issue. If you're accessing a variable from the main loop and from an interrupt handler, then you need to ensure it's declared as volatile. For example:

volatile int cnt = 0;

That tells the compiler that the contents of the variable could change unexpectedly, i.e. when an Interrupt Service Routine is triggered. The result is that it always fetches the actual value from RAM, instead of relying on a cached copy which could be out-of-date.

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.