void loop()
{
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = true;
}
while (menu == true)
{
W1Value = (readEncoder() * 5) % 51;
lcd.setCursor (0, 0);
lcd.print("W1:");
lcd.print(W1Value);
lcd.print("ms ");
lcd.print("*");
lcd.print(" ");
} menu = false;
}
void loop()
{
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = true;
}
while (menu == true)
{
W1Value = (readEncoder() * 5) % 51;
lcd.setCursor (0, 0);
lcd.print("W1:");
lcd.print(W1Value);
lcd.print("ms ");
lcd.print("*");
lcd.print(" ");
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = false;
}
}
And yes, I wrote that as (true == menu) on purpose.
if ( menu == true ) // bad
if ( true == menu ) // bad
if ( menu != false ) // ok
if ( menu ) // best
The "break" instruction is really just bad, lazy coding practice.
The "break" instruction is really just bad, lazy coding practice.That's an unjustifiably broad statement. I think you meant that using break to jump around willy-nilly through nested loops is bad, and sure, it is--spaghetti is spaghetti--but that is more about the structure of the program than the tool used to assemble it. There are plenty of situations where break provides the cleanest and easiest to read solution to a problem. Just like goto, it's a tool of the language, and it's up to the programmer to use it where it makes the most sense. You can just as easily create inscrutable spaghetti using tons of ifs to check different flags inside of loops where judicious use of break/continue would have done a much more elegant job. It all depends on the problem you're trying to solve.
. The "break" instruction is really just bad, lazy coding practice. In code with multiple (and especially nested) compound statements it can be a nightmare keeping track of just which compound statement your "break" will exit. Write your code to be clear and easy to read.
All of which is not really helping the guy out. The guy has a logic problem, not a syntax problem. What he started with would break the loop just fine, if it were ever executed.
This is one of the most typical litmus tests that show who are competent and who just blindly repeat "good practices" they were taught in school or by other incompetent instructors.
Break, on the other hand, goes to "the end of the current loop". And that can change, sometimes innocently.
ooops lost the other bit while copying. here it is again.Code: [Select]void loop()
{
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = true;
}
while (menu == true)
{
W1Value = (readEncoder() * 5) % 51;
lcd.setCursor (0, 0);
lcd.print("W1:");
lcd.print(W1Value);
lcd.print("ms ");
lcd.print("*");
lcd.print(" ");
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = false;
}
}
There are two button press here one entering the While loop and the second exiting it. The first works and its the second part does not seem to be working.
There are two button press here one entering the While loop and the second exiting it. The first works and its the second part does not seem to be working.
The main problem is that what you have written is "spaghetti code". It is overly complicated with too many levels of nested braces, such that it takes great effort to follow the logic and understand what is going on (or not, as the case may be).
Your problem will disappear if you can learn how to reorganize and simplify the code. You may start perhaps by breaking out some parts into separate functions. If you inline the functions there will not be a performance penalty.
(You might also consider implementing a state machine with a switch statement.)
what kind of oil do you use to lubricate and condition a while loop?
ooops lost the other bit while copying. here it is again.Code: [Select]void loop()
{
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = true;
}
while (menu == true)
{
W1Value = (readEncoder() * 5) % 51;
lcd.setCursor (0, 0);
lcd.print("W1:");
lcd.print(W1Value);
lcd.print("ms ");
lcd.print("*");
lcd.print(" ");
ENC_SWstate = digitalRead(ENC_SW);
ENC_SWpressDuration = 0;
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == LOW && prevENC_SWstate == HIGH)
{
TimeStamp = millis();
}
}
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
delay(10);
ENC_SWstate = digitalRead(ENC_SW);
if (ENC_SWstate == HIGH && prevENC_SWstate == LOW)
{
ENC_SWpressDuration = (millis() - TimeStamp);
}
}
if (ENC_SWpressDuration > 0 && ENC_SWpressDuration >= delayTime)
{
menu = false;
}
}There are two button press here one entering the While loop and the second exiting it. The first works and its the second part does not seem to be working.