I have a question about moving from minute-set to year-set.
When I press the button at the end of setting minutes, if I hold it down, why doesn't the while loop for year-set get skipped?
// Here we set the minute Rotor = Minute + 120; // The extra value on rotor allows easier turn-back. while (!ButtonPressed()) { delay(UIDelay); Minute = Rotor % 60; }
// Here we set the year Rotor = Year; while (!ButtonPressed()) { }
It's because of the way my ButtonPressed() function works. If the button is held down, successive calls to ButtonPressed() alternate between true and false. So ButtonPressed() returns true in the minute-set loop and the loop exits; then the year-set loop begins but (even though the button is held down) the ButtonPressed() function returns false on the next call and the year-set loop executes once.
I'd like to say I planned it that way, because it works nicely. But the truth is I screwed up in thinking out the debounce logic of ButtonPressed(). I left it alone because the function debounces the button very nicely but still allows "scrolling" through most of the things that don't need to be changed.
Yeah, that was confusing and obviously I didn't get it figured out.
I was hoping for a sweet way to avoid:
while(!buttonpressed()); // Wait for button press Dosomething(); while(buttonpressed(); // Wait for release
while(!buttonpressed()); // Wait for button press DosomethingElse(); while(buttonpressed(); // Wait for release
while(!buttonpressed()); // Wait for button press DosomethingAnotherThing(); while(buttonpressed(); // Wait for release
But I think I'll still keep looking. Having a continuous press return true then false then true would probably lead to mighty difficult bugs the way I structure the rest of my code, but I can see how it's great in your application.
My original intent was to have ButtonPressed() return true only on transitions between unpressed and pressed. I think that would work for you... try this modification to my code, which is what I should have had in the first place:
boolean ButtonPressed() { NewButtonState = !digitalRead(Button); if ((!OldButtonState) && (NewButtonState)) { // Button was unpressed and is now pressed OldButtonState = true; delay(DEBOUNCE); // debounce delay return(true); } OldButtonState = NewButtonState; return(false); }
... And having tried that fixed version, I think it's an improvement and have modified my clock accordingly. It doesn't "scroll through" now, but it doesn't get multiple hits on a single press either.
You code is still a little hard to understand. (I've been a programmer for 25 years, so i may be a bit rusty ;-) )
1) I never let my guys write a function that has multiple 'return' statements. The need to store the return value in a variable and then reutrn(code); That makes it possible to print the returned value, or set a breakpoint just before the return.
2) I'm not sure it does much more than: --- delay(DEBOUNCE); return (!digitalRead(Button));
My code returns true if the button state used to be false and is now true. In other words, it's sensitive to the false-true TRANSITION rather than the false-true CONDITION. This is what I want the function to do, and it works perfectly. Take your time and figure it out --- it's quite a bit more functional than delay(DEBOUNCE); return(!digitalRead(Button));, which would then require the additional code you initially complained of, with the extra while loops to wait until the button was released.
If you have personal issues with multiple return statements, then do something different. I can think of three ---no, four--- different ways of writing the same transition-sensitive function that don't involve multiple return statements. Your PDF has two more I hadn't thought of... but I'm not one of your guys and this one doesn't bother me.
Oh, Hey, I didn't mean to make it sound like I was insisting on your changing your software at all. Sorry about that.
If it works for you, that's fine.
I'll try to find some time to look at it more thoroughly later in the week and see if I can more clearly explain the areas that make me a little nervous.
There's nothing wrong with it right now, and if you're not interested in hearing about <potential> improvements, just tell me to buzz off.
I'm an adult, and I see your code is really superior, so please don;t take these comments as anything other than one perfectionist trying to help another perfectionist make beautiful code.
You've built a fantastic system here. Superb job man.
No problem. Here, see if this function makes more sense to you. It turns out that the debounce delay I had was rendered unnecessary with the hardware debouncing I'd done, so I dropped it also.
boolean ButtonPressed() { boolean WasIt; // as in, "Was It pressed?" NewButtonState = !digitalRead(Button); if ((!OldButtonState) && (NewButtonState)) { // Button was unpressed and is now pressed WasIt = true; } else { WasIt = false; } OldButtonState = NewButtonState; return(WasIt); }
There's still a bug somewhere in the alarm-related code, as I discovered at 5:30 this morning when Monday's alarm went off on Sunday. Definitely gotta fix that. :-)