Go Down

Topic: Audomatic Garage Door Close (Read 3942 times) previous topic - next topic

rocketboy001


About the goto alternative, you need to pick a good working logic and then decide, such as:

in the while loop, if certain condition is met, call an override function, which returns to the loop and the code continue to loop

or if with a different logic, you may decide to end the loop and then call the override function.

In any case, since the override code is pretty independent from other code, it can stay together as a function you can call.

It's best if you start with a flow diagram to specify how the program will run, and then write the code according to the flow diagram.


Thanks, I'm working on this and the other errors that were pointed out and I'll post back when done.

liudr

Just a reminder: a general block diagram is independent from the fact you are using a "computer" to implement it. If I want to stand outside your garage holding all wires, I should be able to use the block diagram to carry out your project too. Once you have a sketch, try to get more specific in your next block diagram and then plan on how to write code to have arduino do it, instead of me.

rocketboy001

Ok, went back to the drawing board a bit. I got rid of the goto, replaced it with a function, and also fixed the issue with the way I was referencing the state of the PINs. Using the suggestions given as guidance I was able to set it up so that there is a beep every x seconds while the override is active.

I'd be interested to hear your comments. 

Nick Gammon

Does it work?

In general in cases like this:

Code: [Select]

if (DoorCloseWarn == true) {
    Warn(); // go to the Warn function


Booleans can only be true or false, so testing for "== true" is a bit redundant. How about:


Code: [Select]

if (DoorCloseWarn) {
    Warn(); // go to the Warn function
Please post technical questions on the forum, not by personal message. Thanks!

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

wildbill

Shouldn't the Calibrate function be called from setup rather than loop? Clearly either will work, but why bother checking for it in loop when it's a startup activity only?

rocketboy001


Does it work?

In general in cases like this:

Code: [Select]

if (DoorCloseWarn == true) {
    Warn(); // go to the Warn function


Booleans can only be true or false, so testing for "== true" is a bit redundant. How about:


Code: [Select]

if (DoorCloseWarn) {
    Warn(); // go to the Warn function



It does work... both my uncle (who I wrote it for) have tested it and it all seems to function correctly. Good point on the DoorCloseWarn == true.

Thanks,
Adam

Nick Gammon


Shouldn't the Calibrate function be called from setup rather than loop? Clearly either will work, but why bother checking for it in loop when it's a startup activity only?


I was thinking that, but it has to be called 40 times. A 'for' loop might look better though, called from setup.
Please post technical questions on the forum, not by personal message. Thanks!

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

rocketboy001



Does it work?

In general in cases like this:

Code: [Select]

if (DoorCloseWarn == true) {
    Warn(); // go to the Warn function


Booleans can only be true or false, so testing for "== true" is a bit redundant. How about:


Code: [Select]

if (DoorCloseWarn) {
    Warn(); // go to the Warn function



It does work... both my uncle (who I wrote it for) have tested it and it all seems to function correctly. Good point on the DoorCloseWarn == true.

Thanks,
Adam


True. Honestly, I hadn't considered moving it outside the loop because I was not aware of how to work things outside of the loop. I'll have to play with your suggestion - I guess the benefit is that it's one less thing that has to be checked in the loop.

rocketboy001



Shouldn't the Calibrate function be called from setup rather than loop? Clearly either will work, but why bother checking for it in loop when it's a startup activity only?


I was thinking that, but it has to be called 40 times. A 'for' loop might look better though, called from setup.


Would it cause issues running through it 40 times in setup?

wildbill

Quote
Would it cause issues running through it 40 times in setup?

No, it'll be do exactly the same as it was before, except it won't have to check to see whether to calibrate on every iteration of loop.

Come to that, the for loop should be in the calibrate routine itself, rather than calling it 40 times.

rocketboy001

Ok, changed it so that the calibration runs in setup - this was easy per recommendations I just did:

Code: [Select]

for (PIRCalTime = 0; PIRCalTime < 40; PIRCalTime++) {
    Calibrate(); // run the Calibrate function
  }


I also cleaned up the redundant if == true statements as suggested.

I know I got off to a rough start but thanks for all of you for your help. I've learned a lot. Appreciate your time.

Go Up