Trouble with millis()

Hello,

What I want to happen is my robot is going along a pre-programmed course but when a button on the front of jabduino gets pressed (bumped into a wall) how can I make the robot then “DO THIS.” The problem is I want it to be able to sense a press and “DO THIS” at any time, not just at certain times… I have tried using millis() but can’t work it out. Could I please have some pointers with my code?

Thanks in Advanced.

This is my current code:

//Setup our motors and speed control plus define our pins
// Motor 1
#define pwma 11
#define ain2 10
#define ain1 9

//Motor 2
#define pwmb 7
#define bin1 6
#define bin2 5

long previousMillis = 0;    // will store last time LED was updated
long previousMillis2 = 0;
long previousMillis3 = 0;

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 200;
long interval2 = 750;
long interval3 = 2000;

void button3() {
  //Go backwards
    //M1
    digitalWrite(pwma, 50);
    digitalWrite(ain2, LOW);
    digitalWrite(ain1, HIGH);
    //M2
    digitalWrite(pwmb, 100);
    digitalWrite(bin2, LOW);
    digitalWrite(bin1, HIGH);
    delay(1000);
  
  // === turn around === //
  //M1
  digitalWrite(pwma, 50);
  digitalWrite(ain2, LOW);
  digitalWrite(ain1, HIGH);
  //M2
  digitalWrite(pwmb, 100);
  digitalWrite(bin2, HIGH);
  digitalWrite(bin1, LOW);
  delay(250);
  
  }

void setup() {

// ==== Motor 1 ===== //
  //PWMA
  pinMode(pwma, OUTPUT);
  //AIN2
  pinMode(ain2, OUTPUT); 
  //AIN1 
  pinMode(ain1, OUTPUT);
  
// ==== Motor 2 ==== //
  //BIN1
  pinMode(bin1, OUTPUT);
  //BIN2
  pinMode(bin2, OUTPUT);
  //PWMB
  pinMode(pwmb, OUTPUT);
  
  //BUTTON STUFF
  #define button 2
  pinMode(button, INPUT);
        
}

void loop()
{
  //=================================================
  //Start of 1 thing//
  if (digitalRead(button) == HIGH) button3();
  
  unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval3) {
    // save the last time 
    previousMillis = currentMillis;   
// ===== Straight ===== //
  //M1
  digitalWrite(pwma, 50);
  digitalWrite(ain2, HIGH);
  digitalWrite(ain1, LOW);
  //M2
  digitalWrite(pwmb, 100);
  digitalWrite(bin2, HIGH);
  digitalWrite(bin1, LOW);
  }
  //End of 1 thing//
  //=================================================
  //Start of 2 thing//
  if(currentMillis - previousMillis2 > interval) {
    // save the last time 
    previousMillis2 = currentMillis;   
// === turn around === //
  //M1
  digitalWrite(pwma, 50);
  digitalWrite(ain2, LOW);
  digitalWrite(ain1, HIGH);
  //M2
  digitalWrite(pwmb, 100);
  digitalWrite(bin2, HIGH);
  digitalWrite(bin1, LOW);
  }
  //End of 2 thing//
  //=================================================
}

You need to be more specific in what you’re trying to accomplish. I don’t know why you think a timing function (millis) is needed, unless you’re trying to debounce the switch or “DO THIS” only for some length of time. It looks (from the code comments) that you want the robot to turn around. I’ll further guess that there are 2 motors that are controlled via PWM.

I guess even more that ain1,2 and bin1,2 are something to do with the direction controls (on an H bridge controller). I don’t understand why the PWMA command is always “50” and the PWMB command is always “100”. I would again guess that the code would have the robot always turning, unless you’ve come up with these numbers experimentally and those are what’s needed to make the motors run the robot in a straight line. Let me assume that’s the case.

Looking at your code it would seem that when the button is pushed (=HIGH) you’re trying to reverse for some period of time (1sec), then turn (for 0.25secs), then go back to straight ahead. Is that what you want ?

There are a number of things I see wrong in the syntax of your code but let’s understand what you’re trying to do first. I will say that this line in the loop() …

unsigned long currentMillis = millis();

… shouldn’t be there. It should be more like …

currentMillis = millis();

… with this in the setup() …

unsigned long currentMillis = 0;

I don’t know what you think you’re doing with this line …

if (digitalRead(button) == HIGH) button3();

Isn’t button3() the routine that does the backup and turn ? When the button is pushed, do you expect a HIGH or a LOW ? Isn’t all you need in the loop() is the command to “go straight” followed by reading the button, which if pushed calls the button3() routine ? I don’t understand why you’ve added all the other timing comparisons.

… with this in the setup() …

unsigned long currentMillis = 0;

It wouldn't go in the setup, else it wouldn't exist in the main loop. It would either need to be global (not inside any function), or static, e.g.
void loop()
{
  static unsigned long currentMillis = 0;

TCWORLD:

… with this in the setup() …

unsigned long currentMillis = 0;

It wouldn't go in the setup, else it wouldn't exist in the main loop. It would either need to be global (not inside any function), or static, e.g.
void loop()

{
static unsigned long currentMillis = 0;

Perhaps I messed up but I thought, where it is, it’s going to be declared over and over and over and … I agree that it needs to be a global variable, declared once and then used in the main loop(), if it’s to be used at all.

This all said, I think the code as is, is more complicated than it needs to be. His button3() function seems to do what I think he requested. All the other stuff in loop() about “Thing1” and “Thing2” and all the timing looks to be superfluous (though I’ve got the feeling I’ve not grasped his real requirements). Something like :

void loop()
{
  if (digitalRead(button) == HIGH) 
    {
    button3();
    }   
  // command straight ahead
  // commands for motor #1
  digitalWrite(pwma, 50);
  digitalWrite(ain2, HIGH);
  digitalWrite(ain1, LOW);
  // commands for motor #2
  digitalWrite(pwmb, 100);
  digitalWrite(bin2, HIGH);
  digitalWrite(bin1, LOW);
}

Personally I always use the internal pullups and make the button/switch activation be a LOW but …

The above, assuming the rest of his code is OK, should make the robot go forward until the button is pushed (via whatever) and then when it is, backup for 1 sec, then turn for 0.25 secs and then go forward again.

The intervals don’t need to be longs. I’m assuming the intervals don’t change within the execution. The ony time they change is when you alter the program and reload it. So, as long as you don’t anticipate having an interval longer than an int, there’s no need to make these longs.

The commenting on the program is pretty good and allows an outsider to understand the functions. You should get more creative with variable names. For example, previousMillis, etc… don’t really correspond to the function of the timer. If you only had one timer inside a function, previousMillis is fine. But, when you have several timers for different functions they should be named something like ledTimer or timerThing2 or milliCounterLed etc…

The same goes for function naming–button3 isn’t nearly as intiative as say, onCollisionReverseCourse or collideChangeDirection etc… And, assuming you will have a button in the front and back, you can rewrite the collisionReverseCourse to apply to both backward and forward direction. This can be done many ways. My preference is by sending an argument to the function. The argument then sets the variables within the reverseCourse function (direction and speed) depending on if the collision occurred while traveling backwards or forwards.

The reusable code principle also applies to the motor direction and movement. Why write 6 digitalWrite lines for each movement when you could create a function move() which takes 1 or 2 arguments (“back” or “forward”, with and overloaded function including amount of time) and a function turn() with 2 arguments (“left” or “right” and amount of time to turn). This would replace 6 lines of digitalWrites with move(direction) or move(direction, time)