Pages: [1] 2   Go Down
Author Topic: Delay() works, but millis() doesn't?  (Read 1866 times)
0 Members and 1 Guest are viewing this topic.
uk
Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
#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   smiley-lol
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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() ?
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

The Udaho Border
Offline Offline
Newbie
*
Karma: 2
Posts: 18
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Sweden
Offline Offline
Full Member
***
Karma: 11
Posts: 237
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

But if you really need to use millis():

Code:
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:
int timeToWait = 1000 * 1000;
delayMicroseconds(timeToWait);

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

Hope this helped,
Jan
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 212
Posts: 13085
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@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.
Logged

Sweden
Offline Offline
Full Member
***
Karma: 11
Posts: 237
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@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:
unsigned long timeToWait = millis() + 1000; //changed to unsigned long because millis gives back an unsigned long
while(millis() =< timeToWait);

JanD
Logged

uk
Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
//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:
//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.
 smiley-grin


Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 654
Posts: 50931
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Sweden
Offline Offline
Full Member
***
Karma: 11
Posts: 237
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Fallguy: Use something like this:
Code:
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:
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
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 654
Posts: 50931
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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.

Logged

Sweden
Offline Offline
Full Member
***
Karma: 11
Posts: 237
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

uk
Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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. smiley-confuse


Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 654
Posts: 50931
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
      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.
Logged

Leighton Buzzard, UK
Offline Offline
Edison Member
*
Karma: 21
Posts: 1339
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
     if(currentMillis - previousMillis > interval){  //test that timing interval has passed
     
      previousMillis = CurrentMillis;                    // ready for next time

      digitalWrite(stoprec, HIGH);                    //trigger shutter relay coil
Logged

there are only 10 types of people
them that understands binary
and them that doesn't

Pages: [1] 2   Go Up
Jump to: