Go Down

Topic: Help with millis!!!! (Read 835 times) previous topic - next topic

alah

Hi, everybody. Millis is driving me crazier ( or maybe those curly braces? )
What I´m trying to do is start a timer when a swith comes in, wait a certain time and then " do that ".I googled millis,found tronicstuff, John Boxall and quite a few threads on the forum; can´t understand.
If I change a few things it LED" won´t go ON, or won´t go OFF or go ON immediately.
Just need explanation,not solution; Thanks.
Here´s my code:

Quote

int startPin = 2;
int LED1Pin = 3;
int openedPin = 4;
int LED2Pin = 5;
int closedPin = 6;
unsigned long openedtime;  // when did switch go HIGH?
unsigned long opentime;  // time delay
unsigned long currenttime; 

void setup( )
{
  pinMode ( startPin, INPUT  ); 
  pinMode ( LED1Pin, OUTPUT  ); 
  pinMode ( openedPin, INPUT ); 
  pinMode ( LED2Pin, OUTPUT  );
  pinMode ( closedPin, INPUT );
}

void loop ( )
{
  currenttime = millis ( );
  if ( digitalRead ( startPin ) == HIGH )
    digitalWrite ( LED1Pin, HIGH );
  if ( digitalRead ( openedPin ) == HIGH )


  {
    digitalWrite ( LED1Pin, LOW );
    openedtime = millis();  // when did it open
    opentime = currenttime - openedtime;
  }
  if ( (opentime) >= 5000 )
  {
    digitalWrite ( LED2Pin, HIGH );  // turn LED2 ON
    if ( digitalRead ( closedPin ) == HIGH )
      digitalWrite ( LED2Pin, LOW );
  }
}







PaulS

Properly posted code is easier to read.

Code that has had Tools + Auto Format run on it is easier to read.

Code that consistently uses { and } after an if statement is easier to understand.

None of your INPUT pins have the internal pullup resistors enabled. Therefore, the switches MUST have external pullup or pulldown resistors. How ARE the switches wired?

pylon

Take a look at your loop(). From the time currenttime is set to millis() to the time openedtime is set to the value of millis() there are a few microseconds at most (there are just a few ifs, digitalRead()s and digitalWrite()s in between). So opentime is always 0 or maybe seldom 1 but never 5000.

holmes4

Take a look a FSM's in the playground.

Mark

Chaul

Pylon pointed out something, that makes me think the code might actually work like that if you moved the
"openedtime = millis();  // when did it open"
outside of the if and into the else branch.
Code: [Select]

  if ( digitalRead ( openedPin ) == HIGH )
  {
    digitalWrite ( LED1Pin, LOW );
    opentime = currenttime - openedtime;
  }
  else
  {
    openedtime = millis();  // when did it open
  }

However, it may be a little too late in the evening for me to think clearly.. Hmm, it's also a little scary that you don't initialize the unsigned long variables. So, it might be that you actually get LED2 to light up in some random cases when the program has just started. And the LED2 might be dimly lit in some case.

CrossRoads

The variables are initialized:

unsigned long openedtime;  // when did switch go HIGH?
unsigned long opentime;  // time delay
unsigned long currenttime;

All will be 0 to start.

This line:
opentime = currenttime - openedtime;

currenttime will always be just less than openedtime because you are always resetting openedTime.
What you need is a flag to indicate that openedtime has been captured and you are waiting for time to pass; do not capture openedTime again until the flag has been cleared.

Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

alah





Quote
Code that has had Tools + Auto Format run on it is easier to read.

Almost sure I did that.
Quote
Code that consistently uses { and } after an if statement is easier to understand.

Does this mean that I should ALWAYS use curly braces with IF statements?
I´ve noticed that the code changes if I include or delete them.
Quote
How ARE the switches wired?

I´m more comfortable with positive digital electronics; pullup resistors,no problem.
Quote
Take a look a FSM's in the playground.

Hope I find it; don´t know what that means.
Quote
the code might actually work like that if you moved the "openedtime = millis();  // when did it open"outside of the if and into the else branch. 

Tried that a thousand times.
Quote
currenttime will always be just less than openedtime because you are always resetting openedTime.

Is it an error to assign both currenttime and openedtime = millis?
Should I take openedtime outof the main loop? Maybe call a function?
Is setting a flag the only way to solve this?

holmes4

FSM Finite State Machine.

Also take a look a the blink without delay example which shows you the correct use of mills and is its self a small FSM.

Mark

CrossRoads

Using a flag is one of the easiest ways.
Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

holmes4

One flag ok

two flags ok Mmm well

three flags Mmmm

four flags - redesign it

Mark

alah

Don´t know if I´m going in the right direction but I´m into interrupts; if there´s something easier I´d appreciate you let me know. I only need one flag; when that switch actuates, count 5s.
then " do that ".I´m working on it, just need directives, thanks.

AMPS-N

#11
Mar 13, 2013, 06:43 am Last Edit: Mar 13, 2013, 06:56 am by Coding Badly Reason: 1
Code: [Select]
unsigned long getDeltaT(unsigned long st){

 unsigned long deltaT=0;
 unsigned long currTime=millis();
      // Serial.println("inside utils");

 if(currTime < st)
   deltaT = (2^32-1) - st + currTime;
 else
   deltaT = currTime - st;
       
 return deltaT;


}

use above function it might solve your issue


Moderator edit: [code] [/code] tags added.
Moderator edit: excessive over-quote removed.
AMPS

CrossRoads

Code: [Select]

void loop(){
currentTime = millis();
if ( (digitalRead(startPin) == 0) && (timeRunning == 0) ) { // time not running, and startPin button is pressed to Gnd
startTime = currentTime;
timeRunning = 1;
digitalWrite (LED1, HIGH);
}
elapsedTime = currentTime - startTime;
if ( (timeRunning == 1) && elapsedTime >= 5000){
timeRunning = 0;
digitalWrite(LED1, LOW);
}

and add what ever you're doing with LED2
Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

PaulS

Quote
Does this mean that I should ALWAYS use curly braces with IF statements?

Yes, even if it's just one line of code. Adding an extra statement to the if block is easier if the block is already there.

Plus, it makes it very clear that you KNOW the amount of code that will be executed if a statement is true.

Without them, I know, and the compiler knows, but it isn't clear that you know how much code will be executed.

Quote
I´ve noticed that the code changes if I include or delete them.

This makes it seem like you really don't know the effect of the curly braces, or the amount of code that will be executed as a result of the evaluation of the if statement (or while statement or for loop, etc.).

Go Up