Pages: [1]   Go Down
Author Topic: help with millis()  (Read 617 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 44
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I am trying to make an rfid safe. I am using a finite state machine to have 3 states: locked, unlocked, and reprogramming. In the locked state, the arduino scans for either a button press, which changes the state to reprogramming, or a card read by the rfid reader, which changes the state to unlocked if it matches the authorized code stored in eeprom. In the unlocked state, the arduino waits 3 seconds, then relocks the safe and changes its state to locked. In the reprogramming state, the ardsuino stores the next card to be scanned in eeprom (this feature changes the card authorized to open the safe). If no card is scanned within 5 seconds, the state is changed back to locked. To execute the 5 second timeout, i am using millis(). In the code, i had the arduino print "reprograming..." to the serial monitor when the button is pressed, and "locked" when the 5 second timeout is over. The problem is, when the button is pressed, the serial monitor prints "reprogramming...", then "locked" in a cycle that runs 4-5 times in about a second. If the button is held down, it continues the cycle until the button is released.

Quote
#include <EEPROM.h>
#include "EEPROMAnything.h"
int  val = 0;
char code[11];                    //the code read by the RFID reader
char validCode [11];             //code read from EEPROM is stored here
int bytesread = 0;
#define buttonPin 10
int lockState = 0;
unsigned long time;





void setup(){
  Serial.begin (2400);
  pinMode(2,OUTPUT);
  pinMode (buttonPin, INPUT); 
  digitalWrite(2, LOW);
  EEPROM_readAnything (0, validCode);
}

bool readRFID(){
  if(Serial.available() > 0) {          // if data available from reader
    if((val = Serial.read()) == 10) {   // check for header
      bytesread = 0; 
      while(bytesread<10) {              // read 10 digit code
        if( Serial.available() > 0) {
          val = Serial.read();
          if((val == 10)||(val == 13)) { // if header or stop bytes before the 10 digit reading
            break;                       // stop reading
          } 
          code[bytesread] = val;         // add the digit           
          bytesread++;                   // ready to read next digit 
        } 
      } 
      if(bytesread == 10) {           // if 10 digit read is complete
        bytesread = 0; 
        digitalWrite(2, HIGH);                  // deactivate the RFID reader for a moment so it will not flood
        delay(1500);                       // wait for a bit
        digitalWrite(2, LOW);                  // Activate the RFID reader
        return true;
      }
    }
  } 
  return false;


void loop() {
  switch (lockState)
  {
  case 0:                                         //Locked State
    if (digitalRead (buttonPin) == LOW)           // check if the button is pressed
    {        
      time = millis();
      lockState = 2;
      Serial.println ("Reprogramming...");
      break;
    }
    else if(readRFID()== true)
    {
      if (strcmp (code, validCode) == 0)
      {
        //unlock...
        lockState = 1;
        break;   
      }
    }

    break;
  case 1:                                    //Unlocked State
    digitalWrite (2, HIGH);
    Serial.println ("Unlocked");
    delay (3000);
    Serial.println ("Locked");
    digitalWrite (2, LOW);
    //lock...
    lockState = 0;
    break;
  case 2:                                    //Reprogram
    if (time + 5000 >= millis())
    {
      Serial.println ("Locked1");
      lockState = 0;
      break;
    }

    else if (readRFID())
    {
      //EEPROM_writeAnything (0, code);
      Serial.println ("Locked");
      lockState = 0;
      break;
    }
    //break;
  }
}

the nature of the problem suggests (to me) that the code is performing the actions inside the

if (time + 5000 >= millis())

statement even when the condition is not true. How can I fox this problem? Is it my code, or is something wrong with my chip? please help...if you have any questions, please ask.

Logged

UTC-5
Offline Offline
Jr. Member
**
Karma: 0
Posts: 99
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I would write it as:



if (time - buttonPressTime >= 5000) {
    ....


time should be updated every time through the loop, and buttunPressTime should be updated when the button is pressed.
Logged

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 538
Posts: 27118
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

This
if (digitalRead (buttonPin) == LOW)           // check if the button is pressed
suggests your button connects to Gnd when pressed.

This
pinMode (buttonPin, INPUT);
says you are not utilizing the internal pullup.
Change to
pinMode (buttonPin, INPUT_PULLUP);
to ensure the pin is not floating and leading to what would appear as false low readings and the repeated actions you see.
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.

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

I already have a pull up resistor. And could someone please elaborate on Canyonero's response?
Logged

Offline Offline
God Member
*****
Karma: 19
Posts: 781
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Canyonero's response is a better way to write time comparisons so that you won't have any trouble when millis hits the rollover point. 

But the failure in your logic is here.

if (time + 5000 >= millis())

This will be true until 5 seconds have passed.  I think you want it to be false until 5 seconds have passed.

Let's think about some real world numbers.

Let's say millis() returns 1000 when you set time.  So when you do this check, millis has gone up a little, let's say to 1001.  Now 1000 + 5000 is 6000.  That will be greater than what is returned by millis until millis finally get's over 6000 in about 5 seconds. 

I would say to rewrite this with a less than, but in reality you should do what canyonero posted and do it with subtraction. 

Logged

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

So for Canyonero's fix, I know I set "buttonPressTime" when the button is pressed, but when do I set "time"?
Logged

UTC-5
Offline Offline
Jr. Member
**
Karma: 0
Posts: 99
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

do a time = millis(); that gets called every time you go through the loop, and stays (relatively) current, make it the first line in your loop.

Then you can do a buttonPressTime = time; when the button is pressed.
« Last Edit: August 23, 2013, 12:32:27 pm by Canyonero » Logged

New Jersey
Offline Offline
Faraday Member
**
Karma: 67
Posts: 3702
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Or just use millis() instead of time.
Logged

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

Thanks everyone. I got it to work.
Logged

Global Moderator
Boston area, metrowest
Offline Offline
Brattain Member
*****
Karma: 538
Posts: 27118
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

What did you change?
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.

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

I used the easier method and changed the greater than to a less than
Logged

Pages: [1]   Go Up
Jump to: