Delay function doesn't work

cosmicone · November 13, 2020, 2:19am

Hello, I am testing a DC gearmotor with an incremental optical encoder and want to calibrate it for the encoder clicks.

I am using an Arduino Mega 2560 and using pins 2 and 3 for A and B outputs of the encoder. I am using a Cytron MD 13S motor driver. The PWM output is connected to Mega's pin 4 and Direction outputput is connected to pin 7.

When I run the code without the encoders with interrupts it works fine but with the interrupts, the motor just keeps going and going as though the delay (1000) in line 28 doesn't exist. The debugging line after that doesn't appear on the serial monitor. What am I doing wrong? There are no compiler errors.

#include "CytronMotorDriver.h"
#define encoder0PinA  2
#define encoder0PinB  3

// Configure the motor driver.
CytronMD motor(PWM_DIR, 4, 7);  // PWM = Pin 4, DIR = Pin 7.


volatile unsigned int encoder0Pos = 0;

void setup() {
  pinMode(encoder0PinA, INPUT);
  pinMode(encoder0PinB, INPUT);

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);

  // encoder pin on interrupt 1 (pin 3)
  //attachInterrupt(1, doEncoderB, CHANGE);

  Serial.begin (9600);
}

void loop() {


  motor.setSpeed(128);  // Run forward at 50% speed.
  delay(1000);

  Serial.println ("First part Done");

  //motor.setSpeed(255);  // Run forward at full speed.
  //delay(1000);



  motor.setSpeed(0);    // Stop.
  delay(1000);

  motor.setSpeed(-128);  // Run backward at 50% speed.
  delay(1000);

  Serial.println ("Second Part Done");

  //motor.setSpeed(-255);  // Run backward at full speed.
  //delay(1000);

  motor.setSpeed(0);    // Stop.
  delay(1000);



}

void doEncoderA() {
  // look for a low-to-high on channel A
  if (digitalRead(encoder0PinA) == HIGH) {

    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == LOW) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  else   // must be a high-to-low edge on channel A
  {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == HIGH) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
  Serial.println (encoder0Pos, DEC);
  // use for debugging - remember to comment out
}

void doEncoderB() {
  // look for a low-to-high on channel B
  if (digitalRead(encoder0PinB) == HIGH) {

    // check channel A to see which way encoder is turning
    if (digitalRead(encoder0PinA) == HIGH) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  // Look for a high-to-low on channel B

  else {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinA) == LOW) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
}

gfvalvo · November 13, 2020, 2:30am

Get rid of the Serial.print in the ISR. May not be the only problem. But, it is a problem.

Blackfin · November 13, 2020, 2:33am

How many ticks/rotation on that encoder? I assume it's on the motor shaft, not the gearbox output, correct?

cosmicone · November 13, 2020, 2:35am

Ok .. that worked, thanks! But now I don't get the readout of the encoder - how do I get it?

cosmicone · November 13, 2020, 2:42am

Blackfin:
How many ticks/rotation on that encoder? I assume it's on the motor shaft, not the gearbox output, correct?

The specs say it is 200 cpr and the encoder is attached to the main shaft and not the output shaft. I want to get the exact count so that I can use this motor for precise positioning.

6v6gt · November 13, 2020, 2:47am

An external interrupt has a higher priority here than the overflow ISR of timer0 which controls delay() and other timing functions. A heavy weight external interrupt ISR will block these timing functions. In principle, ISRs must be short and fast. As pointed out, the Serial function is definitely bad.

Blackfin · November 13, 2020, 2:53am

cosmicone:
The specs say it is 200 cpr and the encoder is attached to the main shaft and not the output shaft. I want to get the exact count so that I can use this motor for precise positioning.

OK. My thought was that it might be a 1024tick/rev encoder and was flooding the MCU with so many interrupts it couldn't service delay() in a timely way.

Glad to see it was just the Serial.print() in the ISR.

Because the encoder value is not a single byte, reading its value will require a few bus cycles and so could be interrupted by the encoder ISR.

If you want to read the value, you could do something like

    noInterrupts();
    uint16_t nowEnc = encoder0Pos;
    interrupts();

cosmicone · November 13, 2020, 3:21am

Blackfin:
OK. My thought was that it might be a 1024tick/rev encoder and was flooding the MCU with so many interrupts it couldn't service delay() in a timely way.

Glad to see it was just the Serial.print() in the ISR.

Because the encoder value is not a single byte, reading its value will require a few bus cycles and so could be interrupted by the encoder ISR.

If you want to read the value, you could do something like
    noInterrupts();

uint16_t nowEnc = encoder0Pos;
interrupts();

Where do I insert that code and how do I see the encoder value on the serial monitor?

Blackfin · November 13, 2020, 3:26am

As an experiment try making your loop something like:

void loop() 
{
    motor.setSpeed(128);  // Run forward at 50% speed.
    delay(1000);
    motor.setSpeed(0);    // Stop.
    delay(1000);

    noInterrupts();
    uint16_t nowEnc = encoder0Pos;
    interrupts;    
    Serial.print( "Encoder value after part 1: " ); Serial.println( nowEnc );

    motor.setSpeed(-128);  // Run backward at 50% speed.
    delay(1000);
    motor.setSpeed(0);    // Stop.
    delay(1000);

    noInterrupts();
    uint16_t nowEnc = encoder0Pos;
    encoder0Pos = 0;
    interrupts;    
    Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

}

cosmicone · November 13, 2020, 3:33am

Blackfin:
As an experiment try making your loop something like:
void loop() 

{
motor.setSpeed(128); // Run forward at 50% speed.
delay(1000);
motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
uint16_t nowEnc = encoder0Pos;
interrupts;
Serial.print( "Encoder value after part 1: " ); Serial.println( nowEnc );

motor.setSpeed(-128); // Run backward at 50% speed.
delay(1000);
motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
uint16_t nowEnc = encoder0Pos;
encoder0Pos = 0;
interrupts;
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

}

Won't compile.

Arduino: 1.8.13 (Windows Store 1.8.42.0) (Windows 10), Board: "Arduino Mega or Mega 2560, ATmega2560 (Mega 2560)"

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino: In function 'void loop()':

cytron_with_encoder:42:5: error: 'interrupts' was not declared in this scope

interrupts;

^~~~~~~~~~

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino:42:5: note: suggested alternative: 'intptr_t'

interrupts;

^~~~~~~~~~

intptr_t

cytron_with_encoder:57:14: error: redeclaration of 'uint16_t nowEnc'

uint16_t nowEnc = encoder0Pos;

^~~~~~

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino:41:14: note: 'uint16_t nowEnc' previously declared here

uint16_t nowEnc = encoder0Pos;

^~~~~~

exit status 1

'interrupts' was not declared in this scope

Blackfin · November 13, 2020, 3:38am

Oops. Put a pair of brackets at the end of interrupts:

interrupts();

cosmicone · November 13, 2020, 3:44am

Blackfin:
Oops. Put a pair of brackets at the end of interrupts:
interrupts();

Still won't compile
Arduino: 1.8.13 (Windows Store 1.8.42.0) (Windows 10), Board: "Arduino Mega or Mega 2560, ATmega2560 (Mega 2560)"

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino: In function 'void loop()':

cytron_with_encoder:57:14: error: redeclaration of 'uint16_t nowEnc'

uint16_t nowEnc = encoder0Pos;

^~~~~~

exit status 1

redeclaration of 'uint16_t nowEnc'

Blackfin · November 13, 2020, 3:51am

Copy paste error on my part. Remove the uint16_t in front of the 2nd nowEnc:

    noInterrupts();
    nowEnc = encoder0Pos;
    encoder0Pos = 0;
    interrupts();    
    Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

cosmicone · November 13, 2020, 3:56am

Blackfin:
Copy paste error on my part. Remove the uint16_t in front of the 2nd nowEnc:
    noInterrupts();

nowEnc = encoder0Pos;
encoder0Pos = 0;
interrupts();
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

Still getting compiling errors

Arduino: 1.8.13 (Windows Store 1.8.42.0) (Windows 10), Board: "Arduino Mega or Mega 2560, ATmega2560 (Mega 2560)"

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino: In function 'void loop()':

cytron_with_encoder:41:5: error: 'nowEnc' was not declared in this scope

nowEnc = encoder0Pos;

^~~~~~

C:\Users\IT Support\Documents\Arduino\cytron_with_encoder\cytron_with_encoder.ino:41:5: note: suggested alternative: 'noTone'

nowEnc = encoder0Pos;

^~~~~~

noTone

exit status 1

'nowEnc' was not declared in this scope

Blackfin · November 13, 2020, 3:59am

Please post the code as it sits now.

cosmicone · November 13, 2020, 4:48am

Blackfin:
Please post the code as it sits now.
#include "CytronMotorDriver.h"

#define encoder0PinA 2
#define encoder0PinB 3

// Configure the motor driver.
CytronMD motor(PWM_DIR, 4, 7); // PWM = Pin 4, DIR = Pin 7.

volatile unsigned int encoder0Pos = 0;

void setup() {
pinMode(encoder0PinA, INPUT);
pinMode(encoder0PinB, INPUT);

// encoder pin on interrupt 0 (pin 2)
attachInterrupt(0, doEncoderA, CHANGE);

// encoder pin on interrupt 1 (pin 3)
//attachInterrupt(1, doEncoderB, CHANGE);

Serial.begin (9600);
}

void loop() {

motor.setSpeed(128); // Run forward at 50% speed.
delay(1000);

Serial.println ("First part Done");

//motor.setSpeed(255); // Run forward at full speed.
//delay(1000);

motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
nowEnc = encoder0Pos;
encoder0Pos = 0;
interrupts();
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

motor.setSpeed(-128); // Run backward at 50% speed.
delay(1000);

Serial.println ("Second Part Done");

//motor.setSpeed(-255); // Run backward at full speed.
//delay(1000);

motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
nowEnc = encoder0Pos;
encoder0Pos = 0;
interrupts();
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

}

void doEncoderA() {
// look for a low-to-high on channel A
if (digitalRead(encoder0PinA) == HIGH) {

// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinB) == LOW) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}

else // must be a high-to-low edge on channel A
{
// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinB) == HIGH) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}
//Serial.println (encoder0Pos, DEC);
// use for debugging - remember to comment out
}

void doEncoderB() {
// look for a low-to-high on channel B
if (digitalRead(encoder0PinB) == HIGH) {

// check channel A to see which way encoder is turning
if (digitalRead(encoder0PinA) == HIGH) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}

// Look for a high-to-low on channel B

else {
// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinA) == LOW) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}
}

Blackfin · November 13, 2020, 4:57am

OK. I think you erased both instances of the "uint16_t"; I think this works now (it compiles here...)

#include "CytronMotorDriver.h"
#define encoder0PinA  2
#define encoder0PinB  3

// Configure the motor driver.
CytronMD motor(PWM_DIR, 4, 7);  // PWM = Pin 4, DIR = Pin 7.


volatile unsigned int encoder0Pos = 0;

void setup() {
  pinMode(encoder0PinA, INPUT);
  pinMode(encoder0PinB, INPUT);

  // encoder pin on interrupt 0 (pin 2)
  attachInterrupt(0, doEncoderA, CHANGE);

  // encoder pin on interrupt 1 (pin 3)
  //attachInterrupt(1, doEncoderB, CHANGE);

  Serial.begin (9600);
}

void loop() {


  motor.setSpeed(128);  // Run forward at 50% speed.
  delay(1000);

  Serial.println ("First part Done");

  //motor.setSpeed(255);  // Run forward at full speed.
  //delay(1000);



  motor.setSpeed(0);    // Stop.
  delay(1000);

   noInterrupts();
    uint16_t nowEnc = encoder0Pos;
    interrupts();   
    Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

  motor.setSpeed(-128);  // Run backward at 50% speed.
  delay(1000);

  Serial.println ("Second Part Done");

  //motor.setSpeed(-255);  // Run backward at full speed.
  //delay(1000);

  motor.setSpeed(0);    // Stop.
  delay(1000);

noInterrupts();
    nowEnc = encoder0Pos;
    encoder0Pos = 0;
    interrupts();   
    Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

}

void doEncoderA() {
  // look for a low-to-high on channel A
  if (digitalRead(encoder0PinA) == HIGH) {

    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == LOW) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  else   // must be a high-to-low edge on channel A
  {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == HIGH) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
  //Serial.println (encoder0Pos, DEC);
  // use for debugging - remember to comment out
}

void doEncoderB() {
  // look for a low-to-high on channel B
  if (digitalRead(encoder0PinB) == HIGH) {

    // check channel A to see which way encoder is turning
    if (digitalRead(encoder0PinA) == HIGH) {
      encoder0Pos = encoder0Pos + 1;         // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;         // CCW
    }
  }

  // Look for a high-to-low on channel B

  else {
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinA) == LOW) {
      encoder0Pos = encoder0Pos + 1;          // CW
    }
    else {
      encoder0Pos = encoder0Pos - 1;          // CCW
    }
  }
}

cosmicone · November 13, 2020, 5:07am

Blackfin:
OK. I think you erased both instances of the "uint16_t"; I think this works now (it compiles here...)
#include "CytronMotorDriver.h"

#define encoder0PinA 2
#define encoder0PinB 3

// Configure the motor driver.
CytronMD motor(PWM_DIR, 4, 7); // PWM = Pin 4, DIR = Pin 7.

volatile unsigned int encoder0Pos = 0;

void setup() {
pinMode(encoder0PinA, INPUT);
pinMode(encoder0PinB, INPUT);

// encoder pin on interrupt 0 (pin 2)
attachInterrupt(0, doEncoderA, CHANGE);

// encoder pin on interrupt 1 (pin 3)
//attachInterrupt(1, doEncoderB, CHANGE);

Serial.begin (9600);
}

void loop() {

motor.setSpeed(128); // Run forward at 50% speed.
delay(1000);

Serial.println ("First part Done");

//motor.setSpeed(255); // Run forward at full speed.
//delay(1000);

motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
uint16_t nowEnc = encoder0Pos;
interrupts();
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

motor.setSpeed(-128); // Run backward at 50% speed.
delay(1000);

Serial.println ("Second Part Done");

//motor.setSpeed(-255); // Run backward at full speed.
//delay(1000);

motor.setSpeed(0); // Stop.
delay(1000);

noInterrupts();
nowEnc = encoder0Pos;
encoder0Pos = 0;
interrupts();
Serial.print( "Encoder value after part 2: " ); Serial.println( nowEnc );

}

void doEncoderA() {
// look for a low-to-high on channel A
if (digitalRead(encoder0PinA) == HIGH) {

// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinB) == LOW) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}

else // must be a high-to-low edge on channel A
{
// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinB) == HIGH) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}
//Serial.println (encoder0Pos, DEC);
// use for debugging - remember to comment out
}

void doEncoderB() {
// look for a low-to-high on channel B
if (digitalRead(encoder0PinB) == HIGH) {

// check channel A to see which way encoder is turning
if (digitalRead(encoder0PinA) == HIGH) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}

// Look for a high-to-low on channel B

else {
// check channel B to see which way encoder is turning
if (digitalRead(encoder0PinA) == LOW) {
encoder0Pos = encoder0Pos + 1; // CW
}
else {
encoder0Pos = encoder0Pos - 1; // CCW
}
}
}

Now the motor won't move. The serial output is like this:

First part Done
Encoder value after part 2: 0
Second Part Done
Encoder value after part 2: 0
First part Done
Encoder value after part 2: 0
Second Part Done
Encoder value after part 2: 0
First part Done
Encoder value after part 2: 0
Second Part Done
Encoder value after part 2: 0
First part Done
Encoder value after part 2: 0
Second Part Done

Blackfin · November 13, 2020, 5:11am

Does it spin with your original code?

You can see all I added was a few lines after each stop to grab the encoder count and serial-print it.

The motor PWM stuff hasn't changed.

JCA34F · November 13, 2020, 7:55am

If the encoder uses Hall effect sensors with NPN open collector outputs, you may need:

  pinMode(encoder0PinA, INPUT_PULLUP);
  pinMode(encoder0PinB, INPUT_PULLUP);

Can you turn the motor shaft by hand?

Topic		Replies	Views
Removing Delays from Motor control Code Motors, Mechanics, Power and CNC	12	2674	May 6, 2021
Need delay in loop() to work with timer interrupt Mega ADK Programming	5	2233	May 5, 2021
encoder with interrupt problem Programming	13	2349	May 5, 2021
Problem with motor control Programming	4	1058	May 6, 2021
Using Millis() instead of delay not working Programming	5	640	May 5, 2021

Delay function doesn't work

Related topics