Wrong condition result

Projects Programming
1

Hi

My program controls the print head of the printer. And sometimes appear problem with incorrect stop of motor.
Here is the peaces of code (i removed needless strings) :

class Motor {
  public:
    Motor (byte pinEnable, byte pinA, byte pinB);
    void move(byte moveSpeed, int distance, int timeLmt);
    void stop();
    volatile boolean moveDone = true;
    volatile int pos; //absolute position of head
    int upperLimit;
    int lowerLimit;
private:
    byte _pinEnable;
    byte _pinA;
    byte _pinB;
};

This is interrupt function (position sensor):

void headMove() {
  if (D7_Read == D2_Read) {
    head.pos--;
  } else {
    head.pos++;
  }
}

And this part of main cycle, wich should stop motor when position (pos) > upperLimit:

  if (!head.moveDone){ 
    if (head.pos < head.lowerLimit||head.pos > head.upperLimit){
        head.stop();
        Serial.print(head.pos);
        Serial.print('>');
        Serial.println(head.upperLimit);
    }
  }

Usually in serial monitor i get this: 2426>2423
But sometimes the printhead don't reach the goal position and in serial monitor this: 2306>2423
I don't now how it possible.

2

The problem is in the code you forgot to post. Please read and follow the instructions in the How to use this forum post.

3

"pos" is volatile, which indicates that it might change due to an external factor (e.g. in an ISR).
You cannot assume that the head.pos in the condition has the same value as the head.pos in the print statement, it could have changed between checking and printing.

The correct way to do this is:

  1. Turn off interrupts
  2. Make a copy of head.pos
  3. Turn on interrupts
  4. Use the copy

Pieter

4

PieterP:
You cannot assume that the head.pos in the condition has the same value as the head.pos in the print statement, it could have changed between checking and printing.

Its worse than that - its an int, which is 2 bytes on the Uno, and each byte has to be read one by one to read the variable. An ISR could run and change the bytes inbetween reading one and the other so you can get complete garbage.

So you must always use a criticial section (atomically executed) to read or write a multi-byte volatile variable from outside the ISR. Which is what the noInterrupts()/interrupts() wrapped round the access does.

5

jremington:
The problem is in the code you forgot to post. Please read and follow the instructions in the How to use this forum post.

In "Read this before posting a programming question ..."
6. Getting help on the forum

  • With coding problems, if possible post a "minimal" sketch that demonstrates the problem - not hundreds of
    lines of code.

PieterP:
"pos" is volatile, which indicates that it might change due to an external factor (e.g. in an ISR).
You cannot assume that the head.pos in the condition has the same value as the head.pos in the print statement, it could have changed between checking and printing.

The correct way to do this is:

  1. Turn off interrupts
  2. Make a copy of head.pos
  3. Turn on interrupts
  4. Use the copy

Pieter

Thank you for answer, Pieter.
You are right of course. "head.pos" can change it value only in ISR ("headMove" function) one by one and only in positive direction in my case. It can't be so big difference between "upperLimit" and "pos". And i can't turn off interrupts becase I can skip step and lose position.
Perhaps I do not fully understand how works "volatile".

6

Did you notice that the code snippets you posted are not "a minimal sketch that demonstrates the problem"?

It can't be so big difference between "upperLimit" and "pos". And i can't turn off interrupts becase I can skip step and lose position.

Both of these comments are incorrect.

  1. If a multibyte shared variable is incremented, then accessing it during the propagation of an overflow to the next byte introduces very large errors. The correct way to deal with this is to use something like:
noInterrupts();
multibyte_variable_copy=multibyte_variable;
interrupts();
  1. It is extremely unlikely that the print head moves so fast that the fix used above will fail. If so, you need a much faster processor.
7 
  if (D7_Read == D2_Read) {

Are 'D7_Read' and 'D2_Read' macros that actually read the pins?!?

8

PieterP, MarkT and jremington, you all was right. Thank you for explanation of multibyte shared variable. I didn't know that.

johnwasser:
Are 'D7_Read' and 'D2_Read' macros that actually read the pins?!?

Exactly. Faster than digitalRead. It from external library.

9

Andetlt:
Usually in serial monitor i get this: 2426>2423
But sometimes the printhead don't reach the goal position and in serial monitor this: 2306>2423
I don't know how it possible.

2306 is 0x0902.
If 'head.pos' changed from 0x08FF (2303) to 0x0900 (2304) between reading the LSB and MSB of 'head.pos' you might read 0x09FF (2559) which would trigger the '> 2423' comparison. but by the time you fetch 'head.pos' again to print it the value is 0x0902 (2306).
The 'volatile' keyword just tells the compiler to fetch a fresh copy every time the variable's value is needed (don't cache the value in a register) because it may change at any time. It is still up to you to prevent an interrupt from changing the value between byte fetches.