Audomatic Garage Door Close

rocketboy001:
I've tried a few approaches but can't get any to work.

...

rocketboy001:
A viable solution is one that works - this does.

Well, I'm confused. Does it work or not?

rocketboy001:
I'm sorry I have to laugh. You start off by criticizing my use of goto when you don't seem to understand the basics.

In a few years time you'll look back on this arrogant nonsense and laugh. In case you're in any doubt, every one of Nick Gammon's comments are absolutely spot on.

How not to request help - 101.

As I said in my first and subsequent posts (and as demonstrated in the linked video) the code, as it is written, works. The only thing that I cannot get working is when the override is activated is having a beep go off every x number of minutes.

I'm sorry if I misunderstood you but it seemed as if you were criticizing me, and confused, by my defining the Arduino pins as names (like DoorShutSW). This is what I meant by my "basic stuff" quip. I'm not sure why you'd criticize me doing this when it seems to be SOP in every single example I've ever seen, including the ones on the official Arduino example page.

If I've defined pin 6 as DoorShutSW then it stands to reason that I could then reference pin 6 as DoorShutSW in the rest of my code. Am I wrong?

while(PIR1, PIR2 == HIGH);

All I can say is that this works. Please understand that I'm not a programmer so my approach has basically been throwing things at the wall and using what sticks. This stuck. I saw the use of the comma in this way in some other code (can't remember where at this point) so I tried it and it worked.

while(digitalRead(PIR1) == HIGH || digitalRead(PIR2) == HIGH);

You'll note that I used the || (or) operator in other parts of my code but in this particular statement could not get it to work, I was probably writing it wrong. You seem convinced that my use of the comma does not work, all I'm saying is that it does or at least seems to as far as I can tell. If it is in fact wrong then it is wrong in form and not functionality. I'll try your suggestion when I get back in front of my Arduino.

You also pointed out

while (DoorShutSW == HIGH);

and said

HIGH is 1, DoorShutSW is 6, so that condition will never be true.

DoorShutSW is defined as PIN 6, I'm saying while PIN 6 is HIGH do this. As I understand it a statement like (digitalRead(DoorShutSW) is basically shorthand (or another of writing) (DoorShutSW == HIGH) this is what I remember being told in by someone here on another project I was working on anyway and I've seen both ways in use. In any case it works, it seems to be SOP, so again your criticism/confusion is... confusing.

As for the goto, I know that using goto is frowned upon. In this case, with something as uncomplicated as this, I'm not sure I see what the problem is but again - I'm just throwing at the wall and seeing what sticks. In thinking about this more I'm wondering if having a sub-routine for the override would allow me to accomplish the same result without using the goto. I'm open to suggestions...

Again, as it's written the code works. The only thing that I cannot work out is how to have a beep go every x number of minutes while the override is active.

I'm sorry for my crass responses. You came off a bit crass and it felt like your first post was dedicated to detailing how stupid I am. I was in a mood last night, sorry if I misunderstood. I really am wanting to learn and improve where necessary.

As I understand it a statement like (digitalRead(DoorShutSW) is basically shorthand (or another of writing) (DoorShutSW == HIGH)

This is a problem - it simply isn't correct. You're going to need to use digitalRead as suggested above.

As I understand it a statement like (digitalRead(DoorShutSW) is basically shorthand (or another of writing) (DoorShutSW == HIGH)

Then you misunderstand, digitalRead() is function that returns the state of the pin. (DoorShutSW == HIGH) evaluates the pin number, not its state. I explained this in my earlier post.

http://arduino.cc/en/Reference/DigitalRead

dxw00d:

As I understand it a statement like (digitalRead(DoorShutSW) is basically shorthand (or another of writing) (DoorShutSW == HIGH)

Then you misunderstand, digitalRead() is function that returns the state of the pin. (DoorShutSW == HIGH) evaluates the pin number, not its state. I explained this in my earlier post.

http://arduino.cc/en/Reference/DigitalRead

Ok, good to know. You'll excuse my initial confusion (and perhaps overly overaggressive defense of what I was doing) as what I have does seem to work.

Looking back through where I read that (DoorShutSW == HIGH) was the same as (digitalRead(DoorShutSW) I realize that I misinterpreted what the person was saying. Again, confusion set in because I was being told that what I had would not work when it does, that's all.

I will adjust the code and retest when I get back in front of my Arduino... are there any suggestions as two how to deal with the beep every x number of minutes during the override? Also, any suggestions as to how to eliminate the goto would be welcome.

Whatever others pointed out, all being correct, is probably not helping you because you consider whatever "works" is correct already. You need to read a few sample programs that come with arduino. You can't just invent how to speak the C language. Read a few well written short samples and you might be able to mimic, until you truly understand. If you have some experience speaking a second language, you would agree with me. You can't just invent your own way to speak it. You mimic how native speakers talk until you can talk on your own. Those that pointed out your mistakes are the native speakers of the C language. Fighting them won't get your ideas understood by arduino. Read the sample code and hit the books.

liudr:
Whatever others pointed out, all being correct, is probably not helping you because you consider whatever "works" is correct already. You need to read a few sample programs that come with arduino. You can't just invent how to speak the C language. Read a few well written short samples and you might be able to mimic, until you truly understand. If you have some experience speaking a second language, you would agree with me. You can't just invent your own way to speak it. You mimic how native speakers talk until you can talk on your own. Those that pointed out your mistakes are the native speakers of the C language. Fighting them won't get your ideas understood by arduino. Read the sample code and hit the books.

I'm not fighting anyone. I'm genuinely looking for help. There was some confusion because people were pointing out errors - things that should not work - while I was seeing them as working. I was also misinformed on a particular aspect of how read the state of a pin. You can also understand my... dismay and frustration when I've spent the past couple of weeks writing this to come here and be told right off the bat that everything I've done is wrong.

With all that said:

  1. I understand that I was reading the state of the pin wrong and will work in the suggested fix for that. Thank you.

  2. I understand that that the use of the goto is frowned upon (though I did not know hoe vehement people were about it :)). I used it 'cause it worked and as mentioned here (http://arduino.cc/en/Reference/Goto) I did not use in "unrestrained." I'm open to suggestions on how to eliminate this.

  3. The original reason for my post: suggestions, ideas, on how to get a beep every x number of minutes while the override is active.

rocketboy001:
while(PIR1, PIR2 == HIGH);
All I can say is that this works.

...

I'm saying is that it does or at least seems to as far as I can tell. If it is in fact wrong then it is wrong in form and not functionality.

No, it doesn't work, for two reasons, both of which have been explained. But presumably your testing has not been sufficient to expose the bug yet. I suggest it would benefit you to be far less eager to dismiss people's criticisms of your code.

You asked about how to output a regular beep while the override was active. Personally, I suggest you get rid of the existing sequential blocking control structure and the fugly goto hack, and implement it as a state machine. Code this to execute asynchronously using the techniques shown in the 'blink without delay' example sketch and it would be easy to add in any other background processing you want in whichever state you want - such as blinking lights and making beeps. The design would be much simpler and clearer.

PeterH:

rocketboy001:
while(PIR1, PIR2 == HIGH);
All I can say is that this works.

...

I'm saying is that it does or at least seems to as far as I can tell. If it is in fact wrong then it is wrong in form and not functionality.

No, it doesn't work, for two reasons, both of which have been explained. But presumably your testing has not been sufficient to expose the bug yet. I suggest it would benefit you to be far less eager to dismiss people's criticisms of your code.

Ok, I get that. Which is why I've explained ad nauseum that my basis for defending the way it's written is because as far as I can tell it works. That's all I'm saying, I'm not trying to dismiss anyone, I'm not trying to start a fight, or pee on anyone's fun-fire, I'm just saying "hum, odd because in all the testing I've done this seems to work swimmingly."

See: Automatic Garage Door Closer on Vimeo

rocketboy001:
I'm just saying "hum, odd because in all the testing I've done this seems to work swimmingly."

I'm just saying that code doesn't achieve what you expect. I'd suggest that you could remove that line code and get the same behaviour. So, either what you're trying to do there is not required, or you haven't tested it sufficiently to realise that it's not doing what you intend.

PeterH:

rocketboy001:
I'm just saying "hum, odd because in all the testing I've done this seems to work swimmingly."

I'm just saying that code doesn't achieve what you expect. I'd suggest that you could remove that line code and get the same behaviour. So, either what you're trying to do there is not required, or you haven't tested it sufficiently to realise that it's not doing what you intend.

Noted.

rocketboy001:
Ok, I get that. Which is why I've explained ad nauseum that my basis for defending the way it's written is because as far as I can tell it works. That's all I'm saying, I'm not trying to dismiss anyone, I'm not trying to start a fight, or pee on anyone's fun-fire, I'm just saying "hum, odd because in all the testing I've done this seems to work swimmingly."

Which means 1 or 2 things:

  1. You haven't sufficiently tested enough to expose the error.
  2. The code is redundant/pointless and is handled in a different portion of the code.

If it works, that's fine for you. The problem is you're on the forum, asking for people for help with your code. People here don't want to weed through pointless code, or code that will cause issues as you make changes to your program. That's why it's pointed out.

The problem with GoTo is it's hard to follow/debug. So again, if it works for you, great. If not, you have to take into account how others feel about the code that you've written if you're asking for your help, because GoTo complicates things for those trying to read the code.

As far as your original question, I haven't read your code, but the fact that GoTo is used (delay(), and while statement could also cause issues), may make the normal way of doing this not work, but that's something you can experiment yourself:

if something that causes you to set a condition that will blink the led occurs
   set the condition
   note the time
end if

if the blinking condition is set and it's been 5 minutes
  turn the LED on
  note the time
end if

if it's been on long enough
  turn the led off
end if

Take a look at the Blink Without Delay example for how to handle the "note the time" and "it's been..." portions of the psuedocode.

Arrch:

rocketboy001:
Ok, I get that. Which is why I've explained ad nauseum that my basis for defending the way it's written is because as far as I can tell it works. That's all I'm saying, I'm not trying to dismiss anyone, I'm not trying to start a fight, or pee on anyone's fun-fire, I'm just saying "hum, odd because in all the testing I've done this seems to work swimmingly."

Which means 1 or 2 things:

  1. You haven't sufficiently tested enough to expose the error.
  2. The code is redundant/pointless and is handled in a different portion of the code.

If it works, that's fine for you. The problem is you're on the forum, asking for people for help with your code. People here don't want to weed through pointless code, or code that will cause issues as you make changes to your program. That's why it's pointed out.

The problem with GoTo is it's hard to follow/debug. So again, if it works for you, great. If not, you have to take into account how others feel about the code that you've written if you're asking for your help, because GoTo complicates things for those trying to read the code.

As far as your original question, I haven't read your code, but the fact that GoTo is used (delay(), and while statement could also cause issues), may make the normal way of doing this not work, but that's something you can experiment yourself:

if something that causes you to set a condition that will blink the led occurs

set the condition
  note the time
end if

if the blinking condition is set and it's been 5 minutes
 turn the LED on
 note the time
end if

if it's been on long enough
 turn the led off
end if




Take a look at the Blink Without Delay example for how to handle the "note the time" and "it's been..." portions of the psuedocode.

If it helps, I've looked at the blink without delay example and I can successfully get the override LED to blink during the override. I cannot however get a beep out of the speaker.

Can you get anything out of the speaker elsewhere in your code? Such as the doorclose1 section?

wildbill:
Can you get anything out of the speaker elsewhere in your code? Such as the doorclose1 section?

Yes, elsewhere it seems to work fine. I can get it to beep during the override too, a steady beep (I think I can even get it to do an alternating beep). The issue I'm having is getting it to only beep every 5 minutes (for example) while the override is active.

I don't suspect, but I could be wrong, that the goto is causing any issues with this simply because during the override the goto isn't read... if it helps to eliminate the goto then I will... I'm just at a loss as to how. Would a sub-routine outside of the for(loop) work? Something like:

if (digitalRead(OverrideSW) && digitalRead(DoorOpenedSW) || CloseAttemptsVal == CloseAttempts) {
    OverrideState = true;
  }

  if (OverrideState == true) {
    override()
  }

With the sub-routine being:

void override(){

	digitalWrite(OverrideLED, HIGH);
    DoorOpenTime = 0;
    DoorState = false;
    DoorClose1 = false;
    DoorClose2 = false;
    DoorReset = false;
  } 
}

Then whenever I need the override, instead of using goto I could just call the sub-routine override()?

I'd be inclined to refactor the thing into a state machine (search the forums). But for less work, when you set OverrideState to be true, store the current value of millis in a global unsigned long. In the section you run when OverrideState is true do something like this:

if(millis()-TimeOverrideWasInvoked > 30000UL)
  {
  tone(Speaker,100);
  TimeOverrideWasInvoked =millis();
  }

Edit: Need to reset TimeOverrideWasInvoked

wildbill:
I'd be inclined to refactor the thing into a state machine (search the forums). But for less work, when you set OverrideState to be true, store the current value of millis in a global unsigned long. In the section you run when OverrideState is true do something like this:

if(millis()-TimeOverrideWasInvoked > 30000UL)

{
  Tone(Speaker,100);
  }

Ok, I'll try this out when I'm back in front of my Arduino

rocketboy001:
2) I understand that that the use of the goto is frowned upon (though I did not know hoe vehement people were about it :)). I used it 'cause it worked and as mentioned here (http://arduino.cc/en/Reference/Goto) I did not use in "unrestrained." I'm open to suggestions on how to eliminate this.

That was popular before structured programming paradigm came along to save the day when code became hundreds of lines long that resembles a certain type of poorly cooked Italian noodle. You have to have well-defined entrance and exit points in your code for debugging. It's not an optional writing style. I'll use language analogy once again. Goto was like middle English :slight_smile:

Object-oriented programming didn't come out of a fad either. It was absolutely necessary to manage even larger projects and make code stay independent and reusable like modules. Then more paradigm that I don't know for even larger and more flexible projects.