Go Down

Topic: Delay() works, but millis() doesn't? (Read 1 time) previous topic - next topic

Fallguy

Hi all,  guess what, I'm new to all this!

I have written a sketch (see below) to control the starting and stopping of a digital SLR camera in video mode built into a trap.  The sketch works fine if I use delay() in the final function to control the stopping of the recording after a set time period, but if I try to use the millis() function instead it just doesn't seem to carry out the stop rec function.  I have tried all manner of things to get it working but after days of going round in circles I thought it best to ask for help from those who know better.
The original sketch I used as a starting block was for an Intervalometer, http://www.highonsolder.com/blog/2009/7/20/arduino-intervalometer-for-time-lapse-photography.html, my thanks to Joe!

Code: [Select]
#include <LiquidCrystal.h>                      //library for LCD control

// LiquidCrystal display with:
// RS, EN, D4, D5, D6, D7
LiquidCrystal lcd( 8, 9, 10, 11, 12, 13);       //Set which LCD pins are used for data

//I-O setup
const int potentiometer = 0;                    //Potentiometer analog input pin
const int mode = 16;                            //Mode button input pin
const int shutter = 2;                          //Shutter relay output pin
const int trig = 18;                            //trigger switch input pin
const int stoprec = 3;                          //stop record output pin

//Variables
long interval = 0;                              //time between shutter presses
long previousMillis = 0;                        //previous millisecond value
int hold = 1000;                                //time shutter should be held open
int M = 0;                                      //Mode button state
int Mstate = 0;                                 //previous Mode button state
int StartSet = 0;                               //Mode state
int pot;                                        //potentiometer reading
long range;                                     //potentiometer position
int count = 0;                                  //picture count
int ModeSW = 0;                                 //Low-High set mode toggle
int T = 0;                                      //trigger state
int prevT = 0;                                  // previous trigger state
int ModeT = 0;                                  //low high trig mode toggle

//Press & Hold variables
#define debounce 50                             // ms debounce period to prevent flickering when pressing or releasing the button
#define holdTime 1000                           // ms hold period: how long to wait for press+hold event
long btnDnTime;                                 // time the button was pressed down
long btnUpTime;                                 // time the button was released
boolean ignoreUp = false;                       // whether to ignore the button release because the click+hold was triggered
long trigDnTime;                                // time the trigger was pressed
long trigUpTime;                                // time the trigger was released
boolean ignoreTrigUp = false;                   // whether to ignore the trigger release because the click+hold was triggered

//Initialize I-O
void setup() {                    
 pinMode(shutter, OUTPUT);
 pinMode(stoprec, OUTPUT);  
 pinMode(mode, INPUT);
 digitalWrite(mode, HIGH);
 pinMode(trig, INPUT);
 digitalWrite(trig, HIGH);
 lcd.begin(16, 2);                              //set LCD for 16 columns & 2 rows of display
 
}

void loop(){
 M = digitalRead(mode);                         //read Mode button state    
 
 //Test for button pressed and store the down time
 
 if (M == HIGH && Mstate == LOW && (millis() - btnUpTime) > long(debounce))
 {
   btnDnTime = millis();
 }
 
 //Test for button release and store the up time
 
 if (M == LOW && Mstate == HIGH && (millis() - btnDnTime) > long(debounce))
 {
   btnUpTime = millis();
 }
 
 //Test for button held down for longer than the hold time
 
 if (M == HIGH && (millis() - btnDnTime) > long(holdTime))
 {
   StartSet = !StartSet;                         //toggle Set mode to Timing mode or vice versa
   ignoreUp = true;
   btnDnTime = millis();
 }
 Mstate = M;
 
 //Test for Set Mode
 if(StartSet == LOW){          
   lcd.home();                                   //set cursor to LCD column 1, row 1
   lcd.print("Set Mode   ");                     //display "Set Mode" on LCD  
   count = 0;                                    //reset picture count
   LowRange();                                   // run set mode function
   
 
 }
 
 //Test for Timing Mode
 
 else{
   
   lcd.home();                                   //set cursor to LCD column 1, row 1
   lcd.print("Timing Mode");                     //display "Timing Mode" on LCD          
   lcd.setCursor(0, 1);                          //set cursor to LCD column 1, row 2
   lcd.print(count);                             //display picture count number on LCD
   lcd.print(" Pictures   ");                    //display "Pictures" on LCD
   TestTrig();                                   // call test trig function
 }
 
 


       
}

// Set timing function
long LowRange(){
     long seconds;                                 //shutter timing interval in seconds
     pot = analogRead(potentiometer);              //read potentiometer value
     range = map(pot, 0, 1023, 24, 1);             //divide potentiometer range into 5 second steps
     interval = 5000 * range;                      //convert steps into 60 second range in milliseconds
     seconds = interval/1000;                      //convert milliseconds to seconds
     lcd.setCursor(0, 1);                          //set cursor to LCD column 1, row 2
     lcd.print(seconds);                           //display shutter timing interval in seconds on LCD
     lcd.print(" seconds    ");                    //display "seconds" on LCD
     return interval;                              //return shutter timing interval value to loop
 
}

//Test for trigger function
void TestTrig(){
    T = digitalRead(trig);                         //read trigger button state    
 
 //Test for trigger pressed and store the down time
 
 if (T == HIGH && prevT == LOW && (millis() - trigUpTime) > long(debounce))
   {
     trigDnTime = millis();
   }
 
  //Test for button release and store the up time
   
   if (T == LOW && prevT == HIGH && (millis() - trigDnTime) > long(debounce))
   {
      trigUpTime = millis();
   }
 
  //Test for button held down for longer than the hold time
   
    if (StartSet != LOW && T == HIGH && (millis() - trigDnTime) > long(holdTime))
   {
     
     StartRec();                                    //Call the start rec function
     ignoreTrigUp = true;
     trigDnTime = millis();
   }
   prevT = T;
}  



//Start recording function
int StartRec(){
 
      previousMillis = millis();                    //record the time the recording was triggered
      digitalWrite(shutter, HIGH);                  //trigger recording start relay
      lcd.clear();                                  //clear the lcd
      lcd.home();                                   //set cursor to LCD column 1, row 1
      lcd.print("Start Recording");                 //display "Start Recording" on LCD
      delay(hold);                                  //delay for the set delay time
      lcd.clear();                                  //clear the lcd
      lcd.home();                                   //set cursor to column 1, row,1
      lcd.print("Recording!");                      //display "Recording! " on the lcd
      digitalWrite(shutter, LOW);                  //turn off shutter relay
      delay(3000);                                  //delay 3 seconds                      
      StopRec();                                    //call the stop rec function
       
 }

//Stop recording function
int StopRec(){        
     unsigned long currentMillis = millis();        //define currentMillis as unsigned long
     
    if(currentMillis - previousMillis > interval){  //test that timing interval has passed
     
     digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
     lcd.clear();
     lcd.home();                                   //set cursor to LCD column 1, row 1
     lcd.setCursor(0, 1);
     lcd.print("Stop Recording");                   //display "Stop Recording" on LCD
     Serial.print("Stop Recording");                //print "stop recording" to serial monitor
     delay(hold);                                   //wait preset shutter hold time
     lcd.clear();
     digitalWrite(stoprec, LOW);                    //release shutter relay coil
     ++count;                                       //increment picture count
     return count;                                  //return picture count value to loop
   }
   else return count;
}
 
 


 





I'm sure its something bleedin obvious but any help would be greatly appreciated.

Thanks   XD

mmcp42

I glared at the code for a while
not sure I understand the question
care to point out which line you changed so that it now works using delay() ?
there are only 10 types of people
them that understands binary
and them that doesn't

cachehiker

StopRec only being called once at the end of the StartRec function. A delay() would cause StopRec to wait. The millis() comparison is only being performed once. It comes up false, doesn't stop the recording, the function finishes executing, and never gets called again.

mmcp42

er ok
can you show the actual code that fails?
or
tha actual code that works if the code there already fails?

that way we can compare good with bad
there are only 10 types of people
them that understands binary
and them that doesn't

JanD

For the first I don't know why you don't want to use delay().

But if you really need to use millis():

Code: [Select]

int timeToWait = millis() + 1000;
while(millis() != timeToWait);


This will do the exact same thing as delay(1000) but use millis() instead.

If you don't want to use delay() because you need to disable the timer used by delay() you can still use delayMicroseconds():
Code: [Select]

int timeToWait = 1000 * 1000;
delayMicroseconds(timeToWait);


That also does the exact same thing as delay(1000) but uses another timer.

Hope this helped,
Jan

Coding Badly

@Fallguy and @JanD:

millis returns an UNSIGNED data-type.  Mixing SIGNED and UNSIGNED values when working with millis is a bad idea.

@JanD:

Checking for an exact value...

while(millis() != timeToWait);

...is going to fail from time-to-time.  millis occasionally skips a value.

JanD


@Fallguy and @JanD:

millis returns an UNSIGNED data-type.  Mixing SIGNED and UNSIGNED values when working with millis is a bad idea.


@JanD:

Checking for an exact value...

while(millis() != timeToWait);

...is going to fail from time-to-time.  millis occasionally skips a value.

[/quote]
We all live and learn.

Ok, then you should use:
Code: [Select]

unsigned long timeToWait = millis() + 1000; //changed to unsigned long because millis gives back an unsigned long
while(millis() =< timeToWait);


JanD

Fallguy

Wow!  This is going to sound sad but this is my first experience of posting on a forum and I am blown away by the speed and help of the responses, thanks.

mmcp42, I should have posted both sets of code in the first place but not being up to speed with the protocol I kind of ran out of space.  Here are the two versions of the relevant section.
Working version:

Code: [Select]
//Stop recording function
int StopRec(){         
         
     
      delay(interval);
     
      digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
      lcd.clear();
      lcd.home();                                   //set cursor to LCD column 1, row 1
      lcd.setCursor(0, 1);
      lcd.print("Stop Recording");                   //display "Stop Recording" on LCD
      delay(hold);                                   //wait preset shutter hold time
      lcd.clear();
      digitalWrite(stoprec, LOW);                    //release shutter relay coil
      ++count;                                       //increment picture count
      return count;                                  //return picture count value to loop
   
}


And here is the non working version:


Code: [Select]
//Stop recording function
int StopRec(){         
    ] unsigned long currentMillis = millis();        //define currentMillis as unsigned long
     
     if(currentMillis - previousMillis > interval){  //test that timing interval has passed
     
      digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
      lcd.clear();
      lcd.home();                                   //set cursor to LCD column 1, row 1
      lcd.setCursor(0, 1);
      lcd.print("Stop Recording");                   //display "Stop Recording" on LCD
      Serial.print("Stop Recording");                //print "stop recording" to serial monitor
      delay(hold);                                   //wait preset shutter hold time
      lcd.clear();
      digitalWrite(stoprec, LOW);                    //release shutter relay coil
      ++count;                                       //increment picture count
      return count;                                  //return picture count value to loop
    }
    else return count;
}


The delay is for the variable duration determined by the user.  I could get by using delay()  but it means once the unit is triggered and goes into a recording cycle it can't be stopped other than by resetting the whole thing, probably by powering down and back up, (unless its possible to fit an external reset button).  It would be nice to be able to exit a recording cycle by pushing the mode button and going back to Set Mode.

Cachehiker,  I think what you have pointed out makes total sense and is probably the obvious bit I was missing.  However I still cant see how to get round the problem without the stop rec function being in the loop.  any pointers as to how to achieve this would be very welcome.  I will sleep on it and try to figure it out on my own in the fresh light of day.

JanD,  I see what you are saying but as I hope the extra information above will explain, the delay in a variable governed by the input from the pot and adjusted by the user so setting it to a fixed value is no good, unless I have totally misunderstood what you are suggesting. 

Thanks again all for you help.
:D



PaulS

In the working code, you explicitly wait for a period of time to elapse.

In the non-working code, you check to see if the required period of time has elapsed. If not, you return.

Are you calling StopRec() in a loop, so that eventually, StepRec() will discover that sufficient time HAS elapsed?

JanD

Fallguy: Use something like this:
Code: [Select]

int timeToWait = map(analogRead(myPot), 0, 1023, 0, 10000);
delay(timeToWait);

This reads the value of the pot connected to "myPot", maps the value so what you have a delay of 0-10s depending on how the pot stands. It then delays the given time.

If your problem is what you want to be able to use a button while the delay to stop the recording, try something like this:
Code: [Select]

boolean stopped = false;
int timeToWait = map(analogRead(myPot, 0, 1023, 0, 10000) + millis();
while(millis() =< timeToWait){
  if(digitalRead(myButton) == HIGH){
    stopped = true;
    break;
}

if(!stopped){
  //this code is executed if the process was NOT stopped by myButton
}
else if(stopped){
  //this code is executed if the process WAS stopped by myButton
}


I think the code explains it self, but what happens is what the time to delay is read and mapped, then in the while loop, it loops until either myButton is driven HIGH or the time to delay has ended. If myButton was pressed to cancel the delay, stopped is set true, and you can ask if it was ended by the time or by the button later.

I hope this helped more,
Jan

PaulS

The map function has a fair amount of overhead. If you can live with the mapping being from 0 to 1023 to 0 to 10230 (instead of 10000), just multiply by 10. Far faster...

Code: [Select]
int timeToWait = map(analogRead(myPot, 0, 1023, 0, 10000) + millis();
The time to wait is not what this assignment is producing. This assignment is producing time to stop waiting. This is really not a good idea. Because millis() returns an unsigned long value that will eventually roll over when the Arduino has been running long enough, ADDing to the value returned by millis() could result in a value that rolls over sooner.

Subtraction involving unsigned longs is guaranteed to produce correct results. Addition involving unsigned longs is not.


JanD

Ok, sorry if I misslead you, Fallguy, I think I can't help now. After all, this is the third time or so trying to help someone, but I have lost track of how many times I have needed help.

Jan

Fallguy

Hi PaulS, think you have isolated the problem:
Quote
Are you calling StopRec() in a loop, so that eventually, StepRec() will discover that sufficient time HAS elapsed?


In the first version I was calling StopRec outside the loop, in fact from the end of the StartRec function which I now see meant that it would only be tested once and so never reach the point where currentMillis - previousMillis > interval.
As I understand it then I need to either call StopRec from within the loop or move the line which checks currentMillis - previousMillis against interval to within the loop.

So far my attempts to do this have failed, I am obviously still doing something fundamentally wrong but I just can't see the wood for the trees.

This was my latest attempt:
Code: [Select]
void loop(){
  M = digitalRead(mode);                   //read Mode button state     
 
  //Test for button pressed and store the down time
 
  if (M == HIGH && Mstate == LOW && (millis() - btnUpTime) > long(debounce))
  {
    btnDnTime = millis();
  }
 
  //Test for button release and store the up time
 
  if (M == LOW && Mstate == HIGH && (millis() - btnDnTime) > long(debounce))
  {
    btnUpTime = millis();
  }
 
  //Test for button held down for longer than the hold time
 
  if (M == HIGH && (millis() - btnDnTime) > long(holdTime))
  {
    StartSet = !StartSet;                  //toggle Set mode to Timing mode or vice versa
    ignoreUp = true;
    btnDnTime = millis();
  }
  Mstate = M;
 
  //Test for Set Mode
  if(StartSet == LOW){           
    lcd.home();                             //set cursor to LCD column 1, row 1
    lcd.print("Set Mode   ");               //display "Set Mode" on LCD   
    count = 0;                              //reset picture count
    LowRange();                             // run set mode function
   
   
  }
 
  //Test for Timing Mode
 
  else{
   
    lcd.home();                             //set cursor to LCD column 1, row 1
    lcd.print("Timing Mode  ");               //display "Timing Mode" on LCD       
    lcd.setCursor(0, 1);                    //set cursor to LCD column 1, row 2
    lcd.print(count);                       //display picture count number on LCD
    lcd.print(" Shots      ");              //display "Pictures" on LCD
    TestTrig();                            // call test trig function
  }
  //Test for recording State
      long endTime;
      while (millis() < endTime && millis() > startTime){
      lcd.home();
      lcd.print("Recording! ");
      recState = true;
    }
    //test for interval passed
      while (recState == true)
      if((millis() - startTime) > interval){
      StopRec();
    }
         
}

//Functions

// Set timing function
long LowRange(){
      long seconds;                             //shutter timing interval in seconds
      pot = analogRead(potentiometer);          //read potentiometer value
      range = map(pot, 0, 1023, 24, 1);         //divide potentiometer range into 5 second steps
      interval = 5000 * range;                  //convert steps into 60 second range in milliseconds
      seconds = interval/1000;                  //convert milliseconds to seconds
      lcd.setCursor(0, 1);                      //set cursor to LCD column 1, row 2
      lcd.print(seconds);                       //display shutter timing interval in seconds on LCD
      lcd.print(" seconds    ");                //display "seconds" on LCD
      return interval;                          //return shutter timing interval value to loop
 
}

//Test for trigger function
int TestTrig(){
     T = digitalRead(trig);                   //read trigger button state     
 
  //Test for trigger pressed and store the down time
 
  if (T == HIGH && prevT == LOW && (millis() - trigUpTime) > long(debounce))
    {
      trigDnTime = millis();
    }
 
   //Test for button release and store the up time
   
    if (T == LOW && prevT == HIGH && (millis() - trigDnTime) > long(debounce))
    {
       trigUpTime = millis();
    }
 
   //Test for button held down for longer than the hold time
     
     if (StartSet != LOW && T == HIGH && (millis() - trigDnTime) > long(holdTime))
    {
     
      StartRec();                                //Call the start rec function
      ignoreTrigUp = true;
      trigDnTime = millis();
    }
    prevT = T;
}   



//Start recording function
int StartRec(){
        long startTime = millis();
        long endTime = startTime + interval;
        digitalWrite(shutter, HIGH);        //trigger shutter relay
       lcd.clear();
       lcd.home();                             //set cursor to LCD column 1, row 1
       lcd.print("Start Recording");               //display "Start Recording" on LCD
       delay(hold);
       lcd.clear();
       digitalWrite(shutter, LOW);          //turn off shutter relay
        return startTime;                           
       
  }

//Stop recording function
int StopRec(){         
         
      digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
      lcd.clear();
      lcd.home();                                   //set cursor to LCD column 1, row 1
      lcd.setCursor(0, 1);
      lcd.print("Stop Recording");                   //display "Stop Recording" on LCD
      delay(hold);                                   //wait preset shutter hold time
      lcd.clear();
      digitalWrite(stoprec, LOW);                    //release shutter relay coil
      ++count;                                       //increment picture count
      return count;                                  //return picture count value to loop
   
    }


The new bits are at the end of the loop: these are "test for recording state' and 'test for interval passed'
This gets as far as Starting recording and then flips straight back to Timing mode without apparently calling the StopRec function at all.
Surely I'm just missing something simple due to my lack of coding experience!
I do appreciate all attempts at help, even if I don't really understand them yet. :~



PaulS

Code: [Select]
      long endTime;
      while (millis() < endTime && millis() > startTime){

Since endTime is not explicitly defined, you have no idea whether millis() will, or will not return a value less than endTime.

Presumably, endTime is not supposed to be an un-initialized variable. Quit whenever you feel like it is probably not what you want to have happen.

mmcp42

are you setting previousMillis somewhere?
it looks like it's never changing

i would reset it to current time once your test has passed, something like this

Code: [Select]

     if(currentMillis - previousMillis > interval){  //test that timing interval has passed
     
      previousMillis = CurrentMillis;                    // ready for next time

      digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
there are only 10 types of people
them that understands binary
and them that doesn't

Go Up