I want to have a loop run for ten seconds and then execute another section of code. This timing is done by a delay and increases a value every time the loop is ran. An if statement checks the counter until it reaches the limit I set. My problem is that the count increases once and stops.
I'm using an rfid card reader. The system checks to see if the card is within range and when it is, it turns a servo. It actively checks all the time, but the problem is that the count will only increase when there's a card within range. That's not supposed to happen. I want the motor to turn when a card is read, not increase the count while a card is read. The point is to have the count increase the whole ten seconds while looking for a card to read.
How do I make the counter increase every time the loop is ran?
*the delay is important, and it's small so that the rfid card reader can be looking, longer delays (I can increase mine) will not let the card reader search for a card until the delay is over
Code:
void loop(void) {
delay(20);
count++;
Serial.println(count); //count prints, but only when a MiFare card is within range
pos = deadbolt.read();
byte j_card[4] = { 0xCD, 0xBF, 0xC1, 0x78 };
volatile boolean valid;
uint8_t success; // Flag to check if there was an error with the PN532
uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 }; // Buffer to store the returned UID
uint8_t uidLength; // Length of the UID (4 or 7 bytes depending on ISO14443A card type)
uint8_t currentblock; // Counter to keep track of which block we're on
bool authenticated = false; // Flag to indicate if the sector is authenticated
uint8_t data[16]; // Array to store block data during reads
// Keyb on NDEF and Mifare Classic should be the same
uint8_t keyuniversal[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
//useful things vvv
// Wait for an ISO14443A type cards (Mifare, etc.). When one is found
// 'uid' will be populated with the UID, and uidLength will indicate
// if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);
//useful things ^^^
if (success) { //upon any 4k card within range
//const int locked = 90;
//const int unlocked = 180;
if ( 50 < pos < 130 ) { // depends on deadbolt: switch 90 and 180 in the declartions of the poisitions above ^^^ before void setup(){}
deadbolt.write(unlocked);
delay(100);
return;//commenting these out so the loop runs continuously
}
else if ( 150 < pos < 190 ) {
deadbolt.write(locked);
delay(100);
return; //commenting these out so the loop runs continuously
}
}
//loop runs every 20ms for 500 times = 10 seconds
if ( count >= 500) {
Serial.println("Timer: Entering Sleep Mode");
count = 0;
sleepNow();
}
}
The delays count the times the loop has ran. A little guidance on fixes and/or changes would be nice. Regarding my comment of switching positions, there are five different sections, potentially confusing to inexperienced users.
No, they don't. YOU are counting the delay()s. Stop that. Note when something of interest happens (by calling millis()). Note when something else of interest happens (hint: it's the same way). The difference between the times is the interval of interest.
Regarding my comment of switching positions, there are five different sections, potentially confusing to inexperienced users.
You think that comments that don't match the (incorrect) code will make it clearer? Wanna buy a bridge?
Thanks, the Arduino example is counting the way you said it would. All the examples I found iabout using sleep modes and interrupts used delays to do timing. To my understanding, when using sleep modes and power saving coding, delays and timing can be altered and misinterpreted caused by the start-up delay on the microprocessor.
Same problem, the timing code is not working with the loop() that I have. Nothing prints to the serial monitor. BlinkWithoutDelay prints the value though.
bullroarer:
The delays count the times the loop has ran. A little guidance on fixes and/or changes would be nice. Regarding my comment of switching positions, there are five different sections, potentially confusing to inexperienced users.
It will be hard to tell if your code is doing what you want, when your if() statements are not syntactically correct.
if ( 50 < pos < 130 ) {
}
This should be re-written as:
if (50 < pos && pos < 130) {
}
Please also consider that you may want to use <= instead, depending on whether you mean to include the values 50 and 130 in your range or not.
You may wonder why your original statement compiles. Here is how C++ interprets it:
if( 50 < (pos < 130))
Then it evaluates the innermost parentheses: (pos < 130). Let's say that this statement is true. So that evaluates to the boolean value "true". C++ allows you to use boolean values as numerical values and vice versa. Boolean "true", when treated as a number, evaluates to 1.
So what happens when we get to the next set of parentheses: (50 < (true)). Since the less-than operator requires a number, the boolean value "true" is converted to the numerical value 1. The expression (50 < 1) evaluates to false and the if() statement does not enter.
As you can see, in C++ just because code compiles doesn't mean it is doing what you want.
//loop runs every 20ms for 500 times = 10 seconds
if ( count >= 500) {
Not true. Your code takes some time to execute, so each loop takes more than then 20 ms delay you put in the beginning.
Because you only posted your loop() function, it's not obvious what conditions will wake your Arduino from sleep, making it harder for us to evaluate your code and help you. You should almost always post the entirety of your code.
Constants, such as your array of key-code values, should be declared outside of loop(). It's possible that a compiler will optimize this so that it isn't re-allocated every loop, but why take the chance. You can additionally help the compiler out by using the const keyword, such as:
As has been said already, you really need to understand BlinkWithoutDelay so that you can get rid of the delay() functions. Delay() is an incredible kludge that has almost no place in a piece of production code. It's just there for n00bs who want to get their feet wet.
Mark the time that something happened.
Set a boolean value to indicate that the "trigger" event has occurred and you are now waiting for the "response" behavior.
Every time through the loop, check what time it is now.
If the difference between "now" and "trigger time" is greater than the desired interval, then do the "response" behavior and clear the boolean flag.
#include <avr/sleep.h>
#include <Wire.h>
#include <Adafruit_NFCShield_I2C.h>
#include <Servo.h>
#include <SPI.h>
#define IRQ (2)
#define SS_PIN (6) // Not connected by default on the NFC Shield
Adafruit_NFCShield_I2C nfc(IRQ, SS_PIN);
Servo deadbolt;
const int locked = 90;
const int unlocked = 180;
int pos;
int wakePin = 3;
int sleepStatus = 0;
//int count = 0; //delays do not work with timing when running a loop
/*blinkWithoutDelay*/
const int ledPin = 13;
int ledState = LOW;
long previousMillis = 0;
long interval = 1000;
void wakeUpNow() {
}
void setup(void) {
pinMode(wakePin, INPUT);
pinMode(ledPin, OUTPUT);
attachInterrupt(1, wakeUpNow, HIGH);
Serial.begin(9600); //originally set to 115200
Serial.println("Looking for PN532...");
deadbolt.attach(9);
deadbolt.write(locked);
nfc.begin();
uint32_t versiondata = nfc.getFirmwareVersion();
if (! versiondata) {
Serial.print("Didn't find PN53x board");
while (1); // halt
}
// Got ok data, print it out!
Serial.println("Everything checks out."); //Serial.println((versiondata>>24) & 0xFF, HEX);
//Serial.print("Firmware ver. "); Serial.print((versiondata>>16) & 0xFF, DEC);
//Serial.print('.'); Serial.println((versiondata>>8) & 0xFF, DEC);
// configure board to read RFID tags
nfc.SAMConfig();
Serial.println("Waiting for an Identification Card ...");
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
void sleepNow() {
set_sleep_mode(SLEEP_MODE_PWR_DOWN);
sleep_enable();
attachInterrupt(1, wakeUpNow, HIGH);
deadbolt.detach();
sleep_mode();
sleep_disable();
detachInterrupt(1);
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
void loop(void) {
//Serial.println(count); //count prints, but only when a MiFare card is within range
pos = deadbolt.read();
byte j_card[4] = { 0xCD, 0xBF, 0xC1, 0x78 };
volatile boolean valid;
uint8_t success; // Flag to check if there was an error with the PN532
uint8_t uid[] = { 0, 0, 0, 0, 0, 0, 0 }; // Buffer to store the returned UID
uint8_t uidLength; // Length of the UID (4 or 7 bytes depending on ISO14443A card type)
uint8_t currentblock; // Counter to keep track of which block we're on
bool authenticated = false; // Flag to indicate if the sector is authenticated
uint8_t data[16]; // Array to store block data during reads
// Keyb on NDEF and Mifare Classic should be the same
const uint8_t keyuniversal[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
// Wait for an ISO14443A type cards (Mifare, etc.). When one is found
// 'uid' will be populated with the UID, and uidLength will indicate
// if the uid is 4 bytes (Mifare Classic) or 7 bytes (Mifare Ultralight)
success = nfc.readPassiveTargetID(PN532_MIFARE_ISO14443A, uid, &uidLength);
if (success) { //upon any 4k card within range
//const int locked = 90;
//const int unlocked = 180;
if ( pos == locked ) { // depends on deadbolt: switch 90 and 180 in the declartions of the poisitions above ^^^ before void setup(){}
deadbolt.write(unlocked);
delay(100);
return;//commenting these out so the loop runs continuously
}
else if ( pos == unlocked ) {
deadbolt.write(locked);
delay(100);
return; //commenting these out so the loop runs continuously
}
}
//following code is from the blinkWithoutDelay example from Arduino
///////////////////////////////////////////////////////////////////
// check to see if it's time to blink the LED; that is, if the
// difference between the current time and last time you blinked
// the LED is bigger than the interval at which you want to
// blink the LED.
unsigned long currentMillis = millis();
if(currentMillis - previousMillis >= interval) {
// save the last time you blinked the LED
previousMillis = currentMillis;
Serial.println(previousMillis);
// if the LED is off turn it on and vice-versa:
if (ledState == LOW)
ledState = HIGH;
else
ledState = LOW;
// set the LED with the ledState of the variable:
digitalWrite(ledPin, ledState);
}
//loop runs every 20ms for 500 times = 10 seconds
//if ( count >= 500) {
// Serial.println("Timer: Entering Sleep Mode");
// count = 0;
// sleepNow();
//}
}
Paul, you are making this easier and easier. The only reason I put the delay with the servo controls was to help with write functions. Any time the card was in range, my servo would vibrate in position without going to 180 or 90. I got rid of all the delay() 's and now the rfid is sensitive to the point I can hardly get a card close to it. If that's the way the water flows, then I guess I'm going down river.
@PaulS
PaulS:
Serial.println("Looking for PN532...");
deadbolt.attach(9);
deadbolt.write(locked);
Somehow, I fail to see the relationship between what you printed and what you do.
Don't use two separate reads in an if/else combination. The second one might give a different answer and cause all sorts of strange behaviour.
Don't bother with servo.read() - it doesn't do anything useful. Just use a variable that records the last value used with servo.write().
Since your code should "know" what it has already told the servo to do I suspect the entire if/else is superfluous.
The delay which you have programmed with delay(100) could better be programmed using the BWOD technique.
To my way of thinking you have far too much code in loop(). The logic of the system would be much easier to follow if the different activities are put into their own functions. And because the code in each function would be short it would also be easy to understand and debug.
joshuabardwell:
You can additionally help the compiler out by using the const keyword
Uploading the new code in the next reply. Still unsure where to put the constants, the setup() may be my best shot.
Declare constants and global variables outside of setup() or loop(). If you declare a variable or constant inside a function, it is only accessible within that function, and its value is reset when the function exits.
const int foo = 1;
const int bar = 2;
void setup()
{
}
void loop()
{
}
Robin2:
Don't use two separate reads in an if/else combination. The second one might give a different answer and cause all sorts of strange behaviour.
To generalize this principle, any time a condition may change over time, it is best to read the condition once and store the result, then do conditional tests on the variable. The most obvious example of this is millis(), which is constantly changing every millisecond.
if (millis() > testVar + 1000)
// do something
else if (millis() > testVar + 2000)
// do something else
That is bad because millis() will have advanced between the if() and the else() check, and so there are cases that might "slip through" the logic. Better to do:
unsigned long now = millis();
if (now > testVar + 1000)
// do something
else if (now > testVar + 2000)
// do something else
Now a new function will be called when a card is within range and the position of the servo will be verified. I put a variable named 'pos' in before setup() to store the value of servo.write() .