This must be a simpleton problem:
I know there are libraries that might do this, but this seems so simple and I want to understand why the logic doesn't work.
It's a simple 1 btn menu. Using a Nano. The menu scrolls fine every 2000 millis. When I press the btn it drops out of the while loop fine. The menuName[i] in the loop increments fine. menuLoop = i works fine. However, when it gets to the switch case menuLoop always equals 3. Thus it only takes the case 3. Why can't I get menuLoop to equal whatever i is ?
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
int btn;
long start;
long timer;
long btnDelay = 2000; // menu scroll time in millsecs
int menuLoop; // loop counter
int menuLoops = 3; //0 to 3 so thats = 4 loops
char* menuName[] = {"Signal Strength", "Store 4 Signals", "Store 8 Signals", "Store 12 Signals"}; // character array for menu
void setup() {
pinMode(pBtn, INPUT_PULLUP);
}
void loop() {
menu();
}
void menu() {
for ( int i = 0; i <= menuLoops; i++) {
menuLoop = i;
start = millis();
timer = 0;
lcd.clear();
lcd.setCursor(0, 0);
lcd.print(menuName[i]);
lcd.setCursor(0, 1);
lcd.print(menuLoop); //lcd.print("Button to Select");
// loop below waiting for button
do {
btn = digitalRead(pBtn);
timer = millis();
timer = timer - start;
}
while (timer <= btnDelay && btn == 1); //loop if not timed out
// timer elapsed or btn pressed. Test for btn
}
if (btn == 0) {
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("menuLoop = ");
lcd.print(menuLoop);
delay(3000);
switch (menuLoop) {
case 0:
btn = 1;
menuLoop = 0;
sigStrength();
break;
case 1:
btn = 1;
menuLoop = 0;
store(4);
break;
case 2:
menuLoop = 0;
btn = 1;
store(8);
break;
case 3:
btn = 1;
menuLoop = 0;
store(12);
break;
}
}
}
void store(int readings) {
//calibrate();
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("Store ");
lcd.print(readings);
delay(3000);
waitSig(); //wait for signal threshold
//do {
// readVoltage = analogRead(voltPin); // read the input pin
//} while (idleVoltage - totalVoltage < 10);
btn = 1;
}
Then, why does lcd.print(menuName[i]); work? The i increments fine there.
If I the button is pressed in the loop when i is 1 for example. Why doesn't menuLoop =1 after the loop?
?? lcd.print(menuLoop); When this line is executed it shows that menuLoop equals whatever i is.
It says menuLoop = whatever i is. (0-3). But after the while loop, menuLoop always equals 3. Something basic here that I don't grasp. ( How can I pass i (or menuLoop) on after the while loop?
See the switch case . menuLoop is the case selection. If menuLoop equals 0 (eg) then it goes to the sigStrength function. (not shown). If it equals 1- 3 then it should go to the store() function.
So I just want to get out of the while loop with menuLoop equal to whatever the value was when exited. It always = 3 though.
Thanks. Yes, you have described the function I want precisely.
Due to the size of the device and the already built PC board I prefer to use a single button. (Yes, it would be simple with 2 buttons)
Sorry about the section of code not compiling. I didn't think anyone would actually take the time. Thank you for that. Included here is the full (at least the part completed) that does compile for me fine but just won't pass on menuLoop.
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
//CALIBRATE TO ZERO
//Simple Signal Strength Constantly
//signal strength stored into 4, 6, 8, or 12.
// Set the LCD address to 0x27 for a 20 chars and 4 line display
LiquidCrystal_I2C lcd(0x27, 16, 2);
int pBtn = 6; // Menu Select
int ledPin = 4; // Led Pin
int voltPin = A1;
int btn;
long totalVoltage = 0;
long idleVoltage = 0;
long readVoltage = 0;
long start;
long timer;
long btnDelay = 2000; // menu scroll time in millsecs
int menuLoop; // loop counter
int menuLoops = 3; //0 to 3 so thats = 4 loops
char* menuName[] = {"Signal Strength", "Store 4 Signals", "Store 8 Signals", "Store 12 Signals"}; // character array for menu
int sigArray[16]; //an array capable of holding 16 entries numbered 0 to 15
byte arrayIndex = 0;
void setup() {
pinMode(voltPin, INPUT); // sets the digital pin as output
pinMode(pBtn, INPUT_PULLUP);
pinMode(ledPin, OUTPUT);
// initialize the LCD
lcd.begin();
// Turn on the blacklight and clear screen.
lcd.backlight();
lcd.clear();
logo();
}
void loop() {
//calibrate();
menu();
//lcd.clear();
//sigStrength();
}
void logo() {
lcd.clear();
lcd.setCursor(0, 0);
lcd.print("RF Signal Meter");
lcd.setCursor(5, 1);
lcd.print("AI4SR");
delay(3000);
}
void calibrate() {
// establish idle voltage
totalVoltage = 0;
readVoltage = 0;
lcd.clear();
lcd.setCursor(2, 0);
lcd.print("Calibration");
for (int i = 0; i <= 100; i++) {
readVoltage = analogRead(voltPin); // read the input pin
totalVoltage = totalVoltage + readVoltage;
delay(10);// wait for stablization
}
idleVoltage = totalVoltage / 101;
lcd.setCursor(4, 1);
lcd.print("Complete");
delay(1000);
}
void menu() {
for ( int i = 0; i <= menuLoops; i++) {
menuLoop = i; // ************************************* DONT WORK
start = millis();
timer = 0;
lcd.clear();
lcd.setCursor(0, 0);
lcd.print(menuName[i]);
lcd.setCursor(0, 1);
lcd.print(menuLoop); //lcd.print("Button to Select");
// loop below waiting for button
do {
btn = digitalRead(pBtn);
timer = millis();
timer = timer - start;
}
while (timer <= btnDelay && btn == 1); //loop if not timed out
// timer elapsed or btn pressed. Test for btn
}
if (btn == 0) {
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("menuLoop = ");
lcd.print(menuLoop);
delay(3000);
switch (menuLoop) {
case 0:
btn = 1;
menuLoop = 0;
sigStrength();
break;
case 1:
btn = 1;
menuLoop = 0;
store(4);
break;
case 2:
menuLoop = 0;
btn = 1;
store(8);
break;
case 3:
btn = 1;
menuLoop = 0;
store(12);
break;
}
}
}
void sigStrength() {
//calibrate();
totalVoltage = 0;
btn = 1;
delay (400); // wait for stabilization
do {
totalVoltage = 0;
for (int i = 0; i <= 50; i++) {
readVoltage = analogRead(voltPin); // read the input pin
totalVoltage = totalVoltage + readVoltage;
delay(10);
}
totalVoltage = totalVoltage / 51;
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("Relative Signal");
lcd.setCursor(0, 1);
lcd.print(" ");
lcd.setCursor(6, 1);
lcd.print(idleVoltage - totalVoltage);
btn = digitalRead(pBtn);
} while (btn >= 1); // loop no button pressed
btn = 1;
}
void waitSig() {
do {
readVoltage = analogRead(voltPin); // read the input pin
} while (readVoltage < 5);
}
void store(int readings) {
//calibrate();
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("Store ");
lcd.print(readings);
delay(3000);
waitSig(); //wait for signal threshold
//do {
// readVoltage = analogRead(voltPin); // read the input pin
//} while (idleVoltage - totalVoltage < 10);
btn = 1;
}
void error() {
lcd.clear();
lcd.setCursor(2, 0);
lcd.print("Signal Error!");
lcd.setCursor(1, 1);
lcd.print("Re-XMIT 3 Secs.");
for (int i = 0; i <= 10; i++) {
digitalWrite(ledPin, HIGH);
delay(100);
digitalWrite(ledPin, LOW);
delay(100);
}
}
Thanks for the extra. What do you want to happen after you've pressed the button to select the storage? Does it all reset and wait for you to choose a new selection again? and return to cycling?
It's based on the first code you pasted. I'm not saying that this is the BEST way to do it, but it's how I would approach this at the moment! It also uses my own button library (KTS_Button) that you're welcome to use if you like it.
I've given two approaches:
The first is using a single button press to cycle menus (done away with the auto-cycling), then a long button press (as in press and hold) to make the selection. After selection there's a 2sec delay (configurable with the define at the top) then it resets ready for a new selection.
The second behaves more like you describes, with the menus changing every 2secs and storing when you press the button.
In Wokwi, you can highlight the entire of either Method One or Method Two I wrote, and then Command + / to comment or uncomment out the whole code block. Probably Win + / windows.
You'll obviously have to re-implement the other bits, as functions like sigStrength() I just commented out.
Thank you for the code. I may end up just doing that.. However, my question here was mainly academic. I just wanted to know why the code I was using didn't work. It would seem to me that menuLoop should get passed on to the switch case and that bugs me. So there has to be something wrong with that code?
I may fool some more with it , but I thank you for your time and code...
#define arrSize(x) sizeof(x) / sizeof(x[0])
#define MENU_RESET_DURATION 2000
#define BTN_PIN 6
#define LED_PIN 4
#define VOLT_PIN A1
#include "KTS_Button.h"
#include <LiquidCrystal_I2C.h>
KTS_Button button(BTN_PIN);
LiquidCrystal_I2C lcd(0x27, 16, 2);
char* menuItems[] = {"Signal Strength", "Store 4 Signals", "Store 8 Signals", "Store 12 Signals"}; // character array for menu
byte currentMenu = 0;
long totalVoltage = 0;
long idleVoltage = 0;
long readVoltage = 0;
void displayMenu(byte item) {
lcd.clear();
lcd.print(menuItems[item]);
}
void setup() {
Serial.begin(115200);
lcd.init();
lcd.backlight();
logo();
delay(MENU_RESET_DURATION);
displayMenu(currentMenu);
}
void loop() {
ActionType action = button.read();
if (action == SINGLE_PRESS)
displayMenu(++currentMenu %= arrSize(menuItems));
else if (action == LONG_PRESS)
menuSelection();
}
void menuSelection() {
switch (currentMenu) {
case 0:
sigStrength();
break;
case 1:
store(4);
break;
case 2:
store(8);
break;
case 3:
store(12);
break;
}
reset();
}
void reset() {
currentMenu = 0;
displayMenu(currentMenu);
}
void logo() {
lcd.clear();
lcd.setCursor(0, 0);
lcd.print("RF Signal Meter");
lcd.setCursor(5, 1);
lcd.print("AI4SR");
}
void calibrate() {
// establish idle voltage
totalVoltage = 0;
readVoltage = 0;
lcd.clear();
lcd.setCursor(2, 0);
lcd.print("Calibration");
for (int i = 0; i <= 100; i++) {
readVoltage = analogRead(VOLT_PIN); // read the input pin
totalVoltage = totalVoltage + readVoltage;
delay(10);// wait for stablization
}
idleVoltage = totalVoltage / 101;
lcd.setCursor(4, 1);
lcd.print("Complete");
delay(MENU_RESET_DURATION);
}
void sigStrength() {
calibrate();
totalVoltage = 0;
delay(400); // wait for stabilization
do {
totalVoltage = 0;
for (int i = 0; i <= 50; i++) {
if (button.read())
return;
readVoltage = analogRead(VOLT_PIN); // read the input pin
totalVoltage = totalVoltage + readVoltage;
delay(10);
}
totalVoltage = totalVoltage / 51;
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("Relative Signal");
lcd.setCursor(0, 1);
lcd.print(" ");
lcd.setCursor(6, 1);
lcd.print(idleVoltage - totalVoltage);
} while (!button.read()); // loop no button pressed
}
void waitSig() {
do {
readVoltage = analogRead(VOLT_PIN); // read the input pin
} while (readVoltage < 5);
}
void store(int readings) {
calibrate();
lcd.clear();
lcd.setCursor(0, 0);
lcd.print("## Stored ");
lcd.print(readings);
lcd.print(" ##");
waitSig(); //wait for signal threshold
do {
readVoltage = analogRead(VOLT_PIN); // read the input pin
} while (idleVoltage - totalVoltage < 10);
}
void error() {
lcd.clear();
lcd.setCursor(2, 0);
lcd.print("Signal Error!");
lcd.setCursor(1, 1);
lcd.print("Re-XMIT 3 Secs.");
for (int i = 0; i <= 20; i++) {
digitalWrite(LED_PIN, !digitalRead(LED_PIN));
delay(100);
}
}
KTS_Button.h (3.6 KB)
I don't 'think' I broke too much, but it's difficult to test without your hardware.
I didn't try to change much/any of your other existing functions outside of the menu code I suggested based on your description. I think there might be better ways of doing some other bits that don't require so many 'for' and 'while' loops but I'll leave that up to you. For now, if this is useful at all, feel free to take anything you like from it.
Loaded your suggestion and it works almost perfect. (I like the short and long press idea better than my implementation. ) I only need to change a couple minor things in the other functions to work with your code. I guess I will never find out why I can't pass menuLoop on. Thanks again.
I think that was already explained above for you, at the end of the for loop menuLoop is 3 and there's nothing that changes that before the switch block.
I think all you need to do it move the '}' at the end of the for loop, to after the if (btn == 0) { block ends.
So change your whole menu() function (entirely your code with moved bracket) to this:
void menu() {
for ( int i = 0; i <= menuLoops; i++) {
menuLoop = i;
start = millis();
timer = 0;
lcd.clear();
lcd.setCursor(0, 0);
lcd.print(menuName[i]);
lcd.setCursor(0, 1);
lcd.print(menuLoop); //lcd.print("Button to Select");
// loop below waiting for button
do {
btn = digitalRead(pBtn);
timer = millis();
timer = timer - start;
}
while (timer <= btnDelay && btn == 1); //loop if not timed out
// timer elapsed or btn pressed. Test for btn
// <- THIS WAS WHERE THE MOVED BRACKET MOVED FROM
if (btn == 0) {
lcd.clear();
lcd.setCursor(1, 0);
lcd.print("menuLoop = ");
lcd.print(menuLoop);
delay(3000);
switch (menuLoop) {
case 0:
btn = 1;
menuLoop = 0;
// sigStrength();
break;
case 1:
btn = 1;
menuLoop = 0;
store(4);
break;
case 2:
menuLoop = 0;
btn = 1;
store(8);
break;
case 3:
btn = 1;
menuLoop = 0;
store(12);
break;
}
}
} // <- THIS WAS WHERE THE MOVED BRACKET MOVED TO
}
So by moving the bracket above, it now places the if (btn == 0) { block ALSO inside the for loop (where i and menuLoop are changing with each pass)
Yep. You did and it works as I wanted it to. As I said, it was a simpleton problem. I feel better. But I still think I will use your library (KTS_Button). Thanks much for your help and time. I have posted a couple other questions here and as often as not I am told "you don't understand what you are doing", " read the directions" , or sometimes offered a fix that may or may not fix the problem, but didn't help me to understand it. You did both. (Explained why and offered an alternative.)
I certainly don't feel like I 'know what i'm doing'. There are plenty of members here who reply frequently that know 100x what I do!
But I like to use the forum and tackling questions/small projects (usually in Wokwi as it simplifies the hardware aspect), as a battleground for practice and improvement. Always plenty of 'real world' problems to try out.
Hopefully in the process my attempts can solve an issue here or there, or at least be helpful in a small way.