Pages: [1]   Go Down
Author Topic: Help with millis!!!!  (Read 801 times)
0 Members and 1 Guest are viewing this topic.
Venezuela
Offline Offline
Jr. Member
**
Karma: 2
Posts: 81
The less You explain, the more I understand
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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 );
  }
}






Logged

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

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

Switzerland
Offline Offline
Faraday Member
**
Karma: 111
Posts: 5215
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Poole, Dorset, UK
Offline Offline
Edison Member
*
Karma: 52
Posts: 2359
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Take a look a FSM's in the playground.

Mark
Logged

Finland
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 543
Posts: 27348
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Logged

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.

Venezuela
Offline Offline
Jr. Member
**
Karma: 2
Posts: 81
The less You explain, the more I understand
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset





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

Poole, Dorset, UK
Offline Offline
Edison Member
*
Karma: 52
Posts: 2359
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 543
Posts: 27348
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Using a flag is one of the easiest ways.
Logged

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.

Poole, Dorset, UK
Offline Offline
Edison Member
*
Karma: 52
Posts: 2359
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

One flag ok

two flags ok Mmm well

three flags Mmmm

four flags - redesign it

Mark
Logged

Venezuela
Offline Offline
Jr. Member
**
Karma: 2
Posts: 81
The less You explain, the more I understand
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

DELHI
Offline Offline
God Member
*****
Karma: 8
Posts: 925
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
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.
« Last Edit: March 13, 2013, 12:56:28 am by Coding Badly » Logged

AMPS

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 543
Posts: 27348
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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.

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

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

Pages: [1]   Go Up
Jump to: