thanks for the replies… much appreciated.
wildbill:
This is odd:
photocellReadingRange = 'Dark';
Those single quotes should enclose a single character - you’re creating some weird multi-byte character - the compiler accepts it because it has to be able to process unicode.
I don’t think this is your issue, but consider replacing that and the others with a single character, enum, const int or a #define’d constant.
After that, I’d comment out all the calls in loop except the one that controls the door and start throwing in debugging serial.prints in there to get some more clues about what’s going on.
ah, ok…pretty sure i understand now. i was getting errors when i didn’t have quotes around those characters. and since i’m still learning about the aforementioned replacements… i was just trying anything that might work. i now have my homework to re-read about data types.
and yeah, good call with commenting out the calls. not really sure how to add debugging serial.prints (except to add what i have back in 1 by 1? or maybe that’s exactly what you’re saying i should do?) with all of the serial printing i currently have going on, i do know that only a few actually appear in the serial monitor when it’s all running.
thanks again, wild bill!
PeterH:
peter h…
This sort of thing should be an if/else, not two ifs:
if (topSwitchPinVal == 0) { // check if top switch cicuit is closed
digitalWrite(ledCoopDoor, LOW); // turn coop door led on
Serial.println(“circuit and door are open - danger”); // display “circuit and door are open - danger”
}
if (topSwitchPinVal == 1){ // check if bottom switch cicuit is open
digitalWrite(ledCoopDoor, HIGH); // turn coop door led on
Serial.println(“circuit and door are closed - no danger”); // display “circuit and door are closed - no danger”
}
*** ah right. a "duh" on my part.
I'm disconcerted by the amount of global data you have. It is not at all obvious what they're each used for, hard to be sure that you're using the right one in each place and that each one has a sensible lifecycle. This would IMO be easier to follow if you used local statics for data that were only used within a given function. LoopDurationDebounceTopReedSwitch is a good example of a variable that doesn't need to be global.
***ahhh, right... didn't really need to make those global. bad habit of mine! (a waste of ram, yes?)
The way you're doing time comparisons is not rollover-safe. Instead of this:
if(currentTime >= (loopDurationDebounceTopReedSwitch + 10)
It would be better to do this, which is logically equivalent but does handle rollover correctly:
if(currentTime - loopDurationDebounceTopReedSwitch >= 10)
***oh man! i've been really concerned about rollover (i was taking it step by step just to get everything actually working before dealing with that) and you just saved me a ton of time by posting. thank you!
Your variable loopDurationDebounceTopReedSwitch should use enumerated values, not multi-byte character constants. For example:
typedef enum { DARK, DUSK, LIGHT, BRIGHT } LightLevel;
photocellReadingRange = DUSK;
***i *almost* understand your example... (haven't yet studied typedef nor enum )
If you made readPhotoCel() a function that returned the current range, the global variable photocellReadingRange wouldn't be needed at all.
*** makes complete sense. i was having trouble with photocell numerical values coming back and jerking the door motor back and forth around dusk and dawn, so it was my beginner's attempt at creating ranges. (which you've just again made far simpler for me)
I would suggest applying smoothing to the raw value before returning it.
Since a variable can only have one value at a time, one of these clauses is redundant (similar in several other places):
if (photocellReadingRange == ‘Dark’ && photocellReadingRange != ‘Dusk’)
*** yeah, this again was me trying to tell the coop door motor to only trigger the motor when it was dark or light enough (b/c the door was bouncing around (and then jumping past the reed switches) pretty sure part of my problem was that i didn't need to read the photocell nearly as often. after what i've witnessed sitting there during twilight, i really could read the thing every 15 minutes, which would bypass the problem of the photocell reading dark/dusk/dark/dusk. (which begs the question: what is the preferred method of posting 15 minutes? i.e. "900000" or is that lame and inefficient?
These functions to open and close the coop door would IMO make more sense as a finite state machine. You would have a door state variable (OPEN, CLOSING, CLOSED, OPENING) and a switch statement to detect events relevant to the current state:
void handleDoor()
{
static int doorState = OPENING; // initial value
switch (doorState)
{
case OPEN:
if(getLightLevel() < DUSK_THRESHOLD)
{
closeDoor();
doorState = CLOSING;
}
break;
case CLOSING:
if(isDoorClosed())
{
stopDoor();
doorState = CLOSED;
}
break;
case CLOSED:
if(getLightLevel() > DAWN_THRESHOLD)
{
openDoor();
doorState = OPENING;
}
break;
}
***dang.. very cool! and you probably just wrote that in 60 seconds or less, too. =) much appreciated!
You get the idea. This way needs no global data and is completely decoupled from anything else that is going on.
*** yes sir... got it. pulling my head out of the need for globals.
thanks again, peterh!
