Problem using millis() with Adafruit Motorshield v2 & DC motor

Hello, this is my first time using “millis()” in my code and I have read the post on
“Demonstration code for several things at the same time” https://forum.arduino.cc/index.php?topic=223286.0 and adapted it to form the code below. I wish to sweep a DC motor backwards and forwards for a specified amount of time using an Adafruit motorshield v2, something that I achieved using “delay()” but when I run this code, my DC motor does not respond. I have read through many tutorials and have tried working through the code piece by piece but I do not know what the issue is. Any help would be greatly appreciated.

//LIBRARIES
#include <Wire.h>
#include <Adafruit_MotorShield.h>
#include "utility/Adafruit_MS_PWMServoDriver.h"
//CONSTANTS
Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Adafruit_DCMotor *motorL = AFMS.getMotor(2);
const int motorLInterval = 2500;
const int motorSweepDuration = 1000; 
//VARIABLES
byte motorLState = FORWARD;
unsigned long currentMillis = 0;  
unsigned long previousMotorLMillis = 0;
//RUN ONCE
void setup() {
 Serial.begin(9600);
 Serial.println("Program Start");
 AFMS.begin();
 motorL->setSpeed(20);
}
//RUN LOOP
void loop() {
 currentMillis = millis();
 updateMotorLState();
 switchMotor();
}
//STATE UPDATE
void updateMotorLState() {
 if (motorLState == FORWARD) {
   if (currentMillis - previousMotorLMillis >= motorLInterval) {
      motorLState = BACKWARD;
      previousMotorLMillis += motorLInterval;
   }
 }
 else {
   if (currentMillis - previousMotorLMillis >= motorSweepDuration) {
      motorLState = FORWARD;
      previousMotorLMillis += motorSweepDuration;
   } 
 }    
}
//MOTOR SWITCH
void switchMotor() {
 digitalWrite(motorL, motorLState);
}

but I do not know what the issue is.

What do your Serial.print()s tell you is happening?

Why not?

Calling millis() is free. There is no reason to store the value in a global variable.

PaulS:
Calling millis() is free. There is no reason to store the value in a global variable.

Often wondered why that is so common.

Does it work with this version - i.e. without the timing

//RUN LOOP
void loop() {
 currentMillis = millis();
 // updateMotorLState();
 switchMotor();
}

Calling millis() is free. There is no reason to store the value in a global variable.

It can be useful to save the value of millis() so that the exact same value is used in several tests in the same iteration of loop(). In this particular program that is probably not relevant.

I have not tested it, but I suspect it is faster to read a variable rather than call the millis() function when repeated uses of the same value are required.

...R

Storing the value of millis() in a local variable with a sensible name not only removes the overhead of reading it again, with the chance that it has changed since the previous read, it also allows code to be more readable as in

if (currentTime - buttonPressedTime > = shortPressPeriod)

neiklot:
Often wondered why that is so common.

For the same reason that the name is currentMillis. Somewhere, there is an example that did it that way, and people just blindly follow the examples without understanding them.

The value returned by millis() represents now. So, when not use that name? The fact that the value came from the millis() function is irrelevant. So, why does everyone think that the name of a variable holding the value returned by millis() needs to have millis in the name?

No one pretends that all variables holding values returned by analogRead() should have AnalogRead in the name.

So, why does EVERY example showing how to use millis() include Millis in the name of all variables that hold the values that millis() returns? Beats me. I prefer names that make sense, like ledOnTime.

Robin2:
Does it work with this version - i.e. without the timing

//RUN LOOP

void loop() {
currentMillis = millis();
// updateMotorLState();
switchMotor();
}

Thanks for the reply, I tried this, however, the motor does not move at all even without the state updating.

Thanks for the reply, I tried this, however, the motor does not move at all even without the state updating.

All that the code is now doing is calling digitalWrite() to set a fixed pin to a fixed state. If that doesn't work, the problem has NOTHING to do with millis() or any software. It is a hardware problem.

I also looked at the serial monitor as you suggested and it does show that every 1000ms the state of the motor does switch between FORWARD and BACKWARD as the program was meant to do so I will try and figure out my hardware issue. I think that the issue may be that I have used the motorshield to define the motor, rather than using an actual pin on the Arduino, but I am not sure how I would be able to specifically talk to the direction pin on the motorshield.

something that I achieved using “delay()”

What did THAT code look like? Something fundamental was changed between that code and this code, such as which pin to use to make the motor move.

PaulS:
What did THAT code look like? Something fundamental was changed between that code and this code, such as which pin to use to make the motor move.

Yep, the code I used before was in the Adafruit library and controlled the acceleration/speed of the DC motor. I wasnt sure how to incorporate millis() into this code so I tried to adapt the LED state change part of Robin2`s code on multitasking. There is also no mention of the pin, as the “AFMS.getMotor” just assigns the DC motor to 1 of the 4 preassigned motor objects in the motorshield.

#include <Wire.h>
#include <Adafruit_MotorShield.h>

Adafruit_MotorShield AFMS = Adafruit_MotorShield();

Adafruit_DCMotor *motorL = AFMS.getMotor(2);
Adafruit_DCMotor *motorR = AFMS.getMotor(1);

void setup() {
  AFMS.begin();

  motorL->setSpeed(20);
  motorL->run(FORWARD);
  motorL->run(RELEASE);
}

void loop() {
  uint8_t i;

  motorL->run(FORWARD);
  for (i = 0; i < 255; i++) {
    motorL->setSpeed(i);
    delay(2);
  }
  for (i = 255; i != 0; i--) {
    motorL->setSpeed(i);
    delay(2);
  }


  motorL->run(BACKWARD);
  for (i = 0; i < 255; i++) {
    motorL->setSpeed(i);
    delay(2);
  }
  for (i = 255; i != 0; i--) {
    motorL->setSpeed(i);
    delay(2);
  }
  motorL->run(RELEASE);
  delay(10);
}

There is also no mention of the pin, as the "AFMS.getMotor" just assigns the DC motor to 1 of the 4 preassigned motor objects in the motorshield.

The motor objects obviously know what to do to the hardware to make the appropriate motor move.

I notice that the code that makes the motor actually run uses motorL->run(), while the code that doesn't make the motor actually move does not. Coincidence? I think not.

why does EVERY example showing how to use millis() include Millis in the name of all variables that hold the values that millis() returns? Beats me. I prefer names that make sense, like ledOnTime.

Whilst I agree with you regarding the use of sensible names for variables, not everyone is tarred with the same brush. See reply #4 in this thread, for instance

UKHeliBob:
Whilst I agree with you regarding the use of sensible names for variables, not everyone is tarred with the same brush. See reply #4 in this thread, for instance

Mea culpa.

csw1:
Thanks for the reply, I tried this, however, the motor does not move at all even without the state updating.

Then the problem has nothing to do with updating the state. Get the motor running with the simplest possible program before worrying about timing.

...R