Pages: [1] 2 3   Go Down
Author Topic: Audomatic Garage Door Close  (Read 3573 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'm working on a project that will automatically close a garage door after a set period of time. I'm nearly finished but have one lingering problem - when the override is active I want there to be a beep every x number of minutes (say every 5 minutes) until the override is deactivated. I've tried a few approaches but can't get any to work. I'm hoping that someone can look over what I have and suggest a solution. The code is heavenly commented because I'm writing this for my uncle and I want him to know what's going on in the code.

Also, I'm not a programmer and this is only my second attempt at making something with the Arduino so... I'm open to suggestion to improve the code.

Thanks!

I've attached the code 'cause it was too long to post directly.

* gdoor-code.txt (9.69 KB - downloaded 33 times.)
Logged

Global Moderator
Melbourne, Australia
Online Online
Brattain Member
*****
Karma: 511
Posts: 19315
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Sorry, I'm unable to look at code with goto in it. Perhaps someone else can.

Seriously, refactor without it, and without so much confusing code.

As for this:

Code:
    while (DoorShutSW == HIGH);

Code:
const int DoorShutSW = 6; // Arduino PIN that the switch for detecting a closed door is connected to

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



Code:
    while(PIR1, PIR2 == HIGH);

That almost certainly doesn't do what you think it does.
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

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

Lol.

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.
There is no reason to refractor my use of goto, it works and does not seem to be causing any issues that would cause me to not use it (like an endless loop). A viable solution is one that works - this does. In any case with my limited knowladge, I was unable to come up with an alternative solution. If you are able to offer a better solution - rather than just curt criticism - then I'm all ears.

As noted in the code comments DoorShutSW is a variable that is assigned to pin 6 on the Arduino. Attached to pin 6 is a switch which I’m monitoring to see if it goes high or low (on/off). This is basic stuff and I’m not sure why it’s confusing.

while(PIR1, PIR2 == HIGH);

Again, as noted in the comments I’m monitoring the state of two PIR sensors this is effectively saying if either PIR1 or PIR2 is high then continue to execute the if statement.

I assure you that the code works as I have tested it with the hardware (switches, pir sensor, etc) connected to the Arduino. The only issue I’m having is how to get a beep every x number of minutes when the override is active.

I’m open to suggestions as to how to improve the code. Criticizing my work (when I’ve openly admitted I’m not a programmer) while offering no solutions seems like a waste of both your time and mine.

If it helps here is a video of the attached code in action. It’s a bit lengthy and dry but I explain what I’m trying to do and show how it’s currently working.

https://vimeo.com/50017640

Thank you,

Adam
Logged

Gosport, UK
Offline Offline
Faraday Member
**
Karma: 21
Posts: 3113
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
  while (DoorShutSW == HIGH);
Quote
As noted in the code comments DoorShutSW is a variable that is assigned to pin 6 on the Arduino. Attached to pin 6 is a switch which I’m monitoring to see if it goes high or low (on/off). This is basic stuff and I’m not sure why it’s confusing.

Yes, it is basic stuff, but it's confusing because you are evaluating the pin number (which is fixed at 6), not the state of the pin.


Code:
   while(PIR1, PIR2 == HIGH);
Quote
Again, as noted in the comments I’m monitoring the state of two PIR sensors this is effectively saying if either PIR1 or PIR2 is high then continue to execute the if statement.

No, that isn't saying that (you can look up the C comma operator if you wish - http://en.wikipedia.org/wiki/Comma_operator). You are also evaluating pin numbers, not states, again. You would need something like:

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

It wouldn't exactly 'continue to execute the if statement', it would sit in this while loop until both of the PIRs evaluated to LOW.
« Last Edit: September 27, 2012, 02:00:11 am by dxw00d » Logged

Global Moderator
Melbourne, Australia
Online Online
Brattain Member
*****
Karma: 511
Posts: 19315
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Sorry about that 99.

I'll have to re-read the basics.

Quote
Again, as noted in the comments I’m monitoring the state of two PIR sensors this is effectively saying if either PIR1 or PIR2 is high then continue to execute the if statement.

That's utter nonsense, but believe it if you want to.
Logged

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

Global Moderator
Melbourne, Australia
Online Online
Brattain Member
*****
Karma: 511
Posts: 19315
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

...

A viable solution is one that works - this does.

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

http://www.gammon.com.au/electronics

Please post technical questions on the forum - not to me by personal message. Thanks a lot.

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

I only provide help via the forum - please do not contact me for private consultancy.

Maine
Offline Offline
Sr. Member
****
Karma: 14
Posts: 418
Caution: Explosives in use.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

How not to request help - 101.
Logged

"Anyone who isn't confused really doesn't understand the situation."

Electronic props for Airsoft, paintball, and laser tag -> www.nightscapetech.com

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

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

...

A viable solution is one that works - this does.

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

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

New Jersey
Offline Offline
Faraday Member
**
Karma: 72
Posts: 3761
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Gosport, UK
Offline Offline
Faraday Member
**
Karma: 21
Posts: 3113
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

Quote
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.
« Last Edit: September 27, 2012, 12:54:00 pm by rocketboy001 » Logged

Central MN, USA
Offline Offline
Tesla Member
***
Karma: 75
Posts: 7303
Phi_prompt, phi_interfaces, phi-2 shields, phi-panels
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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


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

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

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
« Last Edit: September 27, 2012, 01:38:27 pm by PeterH » Logged

I only provide help via the forum - please do not contact me for private consultancy.

Pages: [1] 2 3   Go Up
Jump to: