If statement troubles..

First off…I would like to say i am new at this
I have only been messing with arduino and programming them for about a month

But I understand its concepts and some of things involved with programming it
How ever…I have been struggling with an “if” statement that I am working on

First I have an Arduino uno and and ethernet shield.

I am trying something basic with a program I found that
user name Colagrosso had on Github
its called AVVISO… its a sketch and set of libraries that send a push notification to you phone
when its ran…but the sketch he provides…does it one time after a count down

well i have it to work as he set it up for…
but I am trying to modify it to work with and "if statement and send the notification to my phone
lets say…
If front door is open send message to phone until door is shut
So I have pin 8 on the controller that im using to try to get it to work
(I have other pins setup for a window and lights…if I get the door to work…but only trying for pin 8 right now)

When I run it…it does not wait for Voltage to appear on that pin…it just goes through and sends and sends and sends
I know its something wrong with my use of “if” statement…or something
Maybe I Shouldn’t be using the “if” statement in this instance…but I’m really not sure what to do…
any help would be greatly appreciated .
The code is below

Thanks

nick

#include <SPI.h>
#include <Ethernet.h>
#include <HTTPClient.h>
#include <Ethernet.h>
#include <Avviso.h>
#define DEBUG 1
byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };

int counterValue;      //maybe used for a counter if needed
int Front = 8;      //front door switch
int Fopen = 7;     //front door open output if i add a light
int Window = A1;    //window switch
int Wopen = 6;    //window open output light if i add add a light

void setup() {
  counterValue = 10;
  pinMode(Front, INPUT);     //made front switch an input
  pinMode(Fopen, OUTPUT);     //made front output for light
  pinMode(Window, INPUT);     //made window switch an input
  pinMode(Wopen, OUTPUT);     //made front output for light

  Serial.begin(115200);                // open serial port
  if (DEBUG) Serial.println("Attempting to obtain a DHCP lease...");
  Ethernet.begin(mac);                //all this below gathers network info and prints it
  if (DEBUG) {
    Serial.print("My IP address is: ");
    Ethernet.localIP().printTo(Serial);
    Serial.println();
    Serial.print("Gateway IP address is ");
    Ethernet.gatewayIP().printTo(Serial);
    Serial.println();
    Serial.print("DNS IP address is ");
    Ethernet.dnsServerIP().printTo(Serial);
    Serial.println();
  }
  Avviso.begin();                           //begin the library for Avviso app
  Avviso.setApiKey("e6b175cac095e82b01e6050ecff22eba60f7d15c");//phone receiver key
  Avviso.setApplicationName("ALERT");      //app name saying alert
}

void loop() {
  int isFrontopen = digitalRead(Front);    //reading front door switch to see if open
  if (isFrontopen == HIGH)             //check to see if the door has been opened
  {
    Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened
    Serial.println(isFrontopen);                            //print on serial monitor so i can see what happens
    delay(500);                                            //small delay


  }
  else {    // do nothing if door isn't opened...i hope
  }
}

Maybe it's not the "if" that's not working, maybe it's the pin that is HIGH How is it wired? With a pull down resistor?

move this one line before the "if' Serial.print("isFrontopen = "); Serial.println(isFrontopen);

if it prints anything other than a 0 then the pin is being read as high. (it common for a pin to float which is why awol is asking about a pull-down resistor)

Before you post code here again, use Tools + Auto Format to fix that mess.

{Avviso.push("Hey DUDE","Front door is open again",1);  //message to send it the door is opened

NOTHING follows the {. Ever!

@nick: Please edit your post, select the code, and put it between [code][/code] tags.

You can do that by hitting the “Code” icon above the posting area. It is the first icon, with the symbol: </>

How to use this forum

PaulS: Before you post code here again, use Tools + Auto Format to fix that mess.

{Avviso.push("Hey DUDE","Front door is open again",1);  //message to send it the door is opened

NOTHING follows the {. Ever!

mess fixed

Can i ask why nothing follows {

It follows it on the next line..wouldn't it? Or do you mean just on that line..and if so why?

OK…i fixed the way the code appeared messy and so forth
i moved the “Avviso.push” on the line AFTER the “{” line
as PaulS said…
I added a pull down resistor and i move the line of code above the if statement that gpopl suggested
Well that got it working…as i need to start with…
Not sure which one made it work correctly
Now any insight as how to add a counter or something of the sort that
make it where
it only send once when the pin goes high…and doesn’t send again…
until the pin goes low again

Here is the new code I have that works…once the pin goes high…it sends…but keep sending
now I want to limit it…to like i said…
once if goes high…it sends once…until it goes low…and then if it reads high again…
it will send again

#include <SPI.h>
#include <Ethernet.h>
#include <HTTPClient.h>
#include <Ethernet.h>
#include <Avviso.h>
#define DEBUG 1
byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };

int counterValue;      //maybe used for a counter if needed
int Front = 8;      //front door switch
int Fopen = 7;     //front door open output if i add a light
int Window = A1;    //window switch
int Wopen = 6;    //window open output light if i add add a light

void setup() {
  counterValue = 10;
  pinMode(Front, INPUT);     //made front switch an input
  pinMode(Fopen, OUTPUT);     //made front output for light
  pinMode(Window, INPUT);     //made window switch an input
  pinMode(Wopen, OUTPUT);     //made front output for light

  Serial.begin(115200);                // open serial port
  if (DEBUG) Serial.println("Attempting to obtain a DHCP lease...");
  Ethernet.begin(mac);                //all this below gathers network info and prints it
  if (DEBUG) {
    Serial.print("My IP address is: ");
    Ethernet.localIP().printTo(Serial);
    Serial.println();
    Serial.print("Gateway IP address is ");
    Ethernet.gatewayIP().printTo(Serial);
    Serial.println();
    Serial.print("DNS IP address is ");
    Ethernet.dnsServerIP().printTo(Serial);
    Serial.println();
  }
  Avviso.begin();                           //begin the library for Avviso app
  Avviso.setApiKey("e6b175cac095e82b01e6050ecff22eba60f7d15c");//phone receiver key
  Avviso.setApplicationName("ALERT");      //app name saying alert
}

void loop() {
  int isFrontopen = digitalRead(Front);    //reading front door switch to see if open
  Serial.print("isFrontopen=");
  Serial.println(isFrontopen);

  if (isFrontopen == HIGH)  //check to see if the door has been opened
  {
    Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened
    Serial.println(isFrontopen);                            //print on serial monitor so i can see what happens
    delay(3500);                                            //small delay


  }
  else {    // do nothing if door isn't opened...i hope
  }
}

There are two basic styles regarding placement of the {. One is like so:

if(something) {

The other is like so:

if(something)
{

With the first style, the closing } does not line up with the {. With the second style, the } does line up with the {.

The ONLY argument that holds water for using the first style is that it uses fewer lines of code. That argument is shot to shit when you follow the statement with a blank line.

  else {    // do nothing if door isn't opened...i hope
  }

If there is nothing to do, it isn't necessary to even have an else statement with a useless comment.

Now any insight as how to add a counter or something of the sort that make it where it only send once when the pin goes high..and doesn't send again.. until the pin goes low again

You don't need a counter. You need to look at the state change detection example, to see how to write code that deals with a pin changing state. Send the message only when the switch BECOMES pressed, not when the switch IS pressed.

nickmthw: Can i ask why nothing follows {

It follows it on the next line..wouldn't it? Or do you mean just on that line..and if so why?

Readability, old chap. Having readable code helps you. Having a consistent style helps everyone, including the people who are trying to help you, now or later.

something like this might work. 


[codeif ((isFrontopen == HIGH) && (door_open_flag == 0)) //check to see if the door has been opened
    //if the flag is set avviso has already done the push
  {
    door_open_flag = 1; // flag to block the code repeating
    Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened
    Serial.println(isFrontopen);                            //print on serial monitor so i can see what happens
    delay(3500);                                            //small delay
  }
  else if (isFrontopen == LOW) {
    door_open_flag = 0;
  }
  // when door closed it resets the flag block so next time
  //the door opens the message will get sent again]

for 3.5 seconds it will not reset due to the delay

PaulS: There are two basic styles regarding placement of the {. One is like so:

if(something) {

The other is like so:

if(something)
{

With the first style, the closing } does not line up with the {. With the second style, the } does line up with the {.

The ONLY argument that holds water for using the first style is that it uses fewer lines of code. That argument is shot to shit when you follow the statement with a blank line.

  else {    // do nothing if door isn't opened...i hope
  }

If there is nothing to do, it isn't necessary to even have an else statement with a useless comment. You don't need a counter. You need to look at the state change detection example, to see how to write code that deals with a pin changing state. Send the message only when the switch BECOMES pressed, not when the switch IS pressed.

I understand some what.. The else statement..I wasn't sure if i needed it or not...the comment was there..for when I added what ever I thought might have been needed to have it NOT send a message..thanks for the help I will look into state change example..and see what I can gather from it..

Thanks for the help

gpop1: ``` something like this might work.

[codeif ((isFrontopen == HIGH) && (door_open_flag == 0)) //check to see if the door has been opened     //if the flag is set avviso has already done the push   {     door_open_flag = 1; // flag to block the code repeating     Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened     Serial.println(isFrontopen);                            //print on serial monitor so i can see what happens     delay(3500);                                            //small delay   }   else if (isFrontopen == LOW) {     door_open_flag = 0;   }   // when door closed it resets the flag block so next time   //the door opens the message will get sent again]




for 3.5 seconds it will not reset due to the delay

This one didn't work exactly like this.. once door is opened..the door open flag never seems to change..so im working with it to see if i can get it working

Use Serial.print to find out why its not working.

im guessing that the Hey DUDE", "Front door is open again", printed and now it will not reset. If this is correct serial.println (isFrontopen); to see if it goes to 0 when the door is shut.

Once the door is opened and the Avviso.push message sends notification to my phone
the open door flag never goes to 1 and stays there waiting until the door is shut
to return to 0
It will be 0 then to 1 as the door is open
the serial monitor shows the door HIGH(1)
the open door flag goes to 0…then next line right below it goes back to 1
and it never blocks the code from sending the Avviso.push again
so it keeps sending and sending until i close the door

#include <SPI.h>
#include <Ethernet.h>
#include <HTTPClient.h>
#include <Ethernet.h>
#include <Avviso.h>
#define DEBUG 1
byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };

int counterValue;      //maybe used for a counter if needed
int Front = 8;      //front door switch
int Fopen = 7;     //front door open output if i add a light
int Window = A1;    //window switch
int Wopen = 6;    //window open output light if i add add a light

void setup() {
  counterValue = 10;
  pinMode(Front, INPUT);     //made front switch an input
  pinMode(Fopen, OUTPUT);     //made front output for light
  pinMode(Window, INPUT);     //made window switch an input
  pinMode(Wopen, OUTPUT);     //made front output for light

  Serial.begin(115200);                // open serial port
  if (DEBUG) Serial.println("Attempting to obtain a DHCP lease...");
  Ethernet.begin(mac);                //all this below gathers network info and prints it
  if (DEBUG) {
    Serial.print("My IP address is: ");
    Ethernet.localIP().printTo(Serial);
    Serial.println();
    Serial.print("Gateway IP address is ");
    Ethernet.gatewayIP().printTo(Serial);
    Serial.println();
    Serial.print("DNS IP address is ");
    Ethernet.dnsServerIP().printTo(Serial);
    Serial.println();
  }
  Avviso.begin();                           //begin the library for Avviso app
  Avviso.setApiKey("e6b175cac095e82b01e6050ecff22eba60f7d15c");//phone receiver key
  Avviso.setApplicationName("ALERT");      //app name saying alert
}

void loop() {
  int isFrontopen = digitalRead(Front);    //reading front door switch to see if open
  int door_open_flag;                      //flag to block the code from repeating
  Serial.print("isFrontopen=");
  Serial.println(isFrontopen);

  Serial.print("flag set=");
  Serial.println(door_open_flag);
  delay(750);

  if ((isFrontopen == HIGH) && (door_open_flag == 0)) //check to see if the door has been opened
  {
    door_open_flag = 1;
    Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened
    Serial.println(door_open_flag);    //print on serial monitor so i can see what happens
    delay(500);                     //small delay


  }
  else(isFrontopen == LOW); {
    door_open_flag = 0;   // when door closes it resets the flag block
    //so the next time the door opens the message will send again
  }
}

move the flag to global and see what happens.

just before int counterValue;

add

byte door_open_flag=0;

  int door_open_flag;                      //flag to block the code from repeating
  Serial.print("isFrontopen=");
  Serial.println(isFrontopen);

  Serial.print("flag set=");
  Serial.println(door_open_flag);

door_open_flag is a local variable that will go out of scope (loose its value) at the end of loop. It is NOT initialized, so it contains whatever crap was in that memory location the last time some other code used that memory location. It was a complete waste of time printing that garbage.

PaulS: ```   int door_open_flag;                      //flag to block the code from repeating   Serial.print("isFrontopen=");   Serial.println(isFrontopen);

  Serial.print("flag set=");   Serial.println(door_open_flag);



door_open_flag is a local variable that will go out of scope (loose its value) at the end of loop. It is NOT initialized, so it contains whatever crap was in that memory location the last time some other code used that memory location. It was a complete waste of time printing that garbage.

ok that's the negative. At what point do you tell us newbys that have never had a programming class what the correct way of initializing it would be. Or let me guess everyone who has never learnt to program need to google the answer. I know that global will work but im also guessing that theres probably a word you can use to stop it loosing it value. As for "It was a complete waste of time printing that garbage" well that's where you are wrong sweetheart. If it wasn't printed then the op wouldn't have a clue that it was being reset with garbage so he would have to beg for crumbs on this web site in the hope that some one would gives him a answer that makes sense.

At what point do you tell us newbys that have never had a programming class what the correct way of initializing it would be.

One should ALWAYS provide an initial value.

I know that global will work but im also guessing that theres probably a word you can use to stop it loosing it value.

That word is static.

just before int counterValue;

add

byte door_open_flag=0; [/quote]

I tried that..and I tried byte door_open_flag;

It doesn't work either way it goes right past the

else if (isFrontopen == LOW); {

its like its not stopping to see the door LOW before it writes the door_open_flag = 0; so it will change from 1 right back to 0 and keep looping and sending my message Ive tried moving it around..and stuff to get it to wait to see low BEFORE changing back to 0 but It will not do it.

Isn't that the "else if" works.. don't go to this line..as long as the "if" line statements are true? as soon as one part of it goes false..it should continue on to the next line?

and with the door_open_flag = 1 part of this code..it should make it become false right before or as it sends.?

door_open_flag = 1;                 //flag to block the code from repeating
    Avviso.push("Hey DUDE", "Front door is open again", 1); //message to send it the door is opened
    Serial.print("Flag now");
    Serial.println(door_open_flag);    //print on serial monitor so i can see what happens
    delay(500);                     //small delay
else if (isFrontopen == LOW); {

What's that ; doing in there?