Calling routines/functions within IF statement

Projects Programming
1

Hey all,
I'm new to Arduino coding and was hoping someone could suggest why my code isn't behaving as I had hoped.

Without going into verbose detail, there is an Uno with an LCD shield that has up/down/left/right buttons.
I can read the buttons fine, I can write to the screen fine, what is stumping me is how to change 'menus'. When I press my change menu button, it changes to the other menu (routine/function), then changes straight back.

Any help would be greatly appreciated!
-Dax.

The basic construction is this:

void loop()
{
setUVTimer();
}

// Functions //
void setUVTimer()
{
  MB1 = analogRead (0);  
  if (MB1 < 60) { //change menu button
   setTemperature();
  }
  else if (MB1 < 200) { //up button
   uvtimer++;
   if (uvtimer >15){uvtimer = 15;}
   delay(250);
  }
  else if (MB1 < 400){ //down button
   uvtimer--;
   if (uvtimer <1){uvtimer = 1;}
   delay(250);
  }
  lcd.print(uvtimer, DEC),lcd.print(" mins ");
}

void setTemperature()
{
  lcd.print("Set temperature");
  MB1 = analogRead (0);  
  if (MB1 < 60) { //change menu button
   setUVTimer();
  }
  else if (MB1 < 200) { //up button
   target_temp++;
   if (target_temp >60){target_temp = 60;}
   delay(180);
  }
  else if (MB1 < 400){ //down button
   target_temp--;
   if (target_temp <15){target_temp = 15;}
   delay(180);
  }
  lcd.print(target_temp, DEC),lcd.print("\xDF""C  ");
}

UPDATE: I realised an alternative solution was to put each of those subroutines inside for(::){} loops, since the subroutines all had exit points (to other subroutines) anyway.

2

Why not print the value of MB1, then you might see where the problem is?
Paul

3

I probably try to use a switch case with break;

if MB1 < 200) is true
it also true
MB1 < 400)
checked that!

maybe try (MB1 >200 && MB1 <400)

4

Ahh yes, @LFRED. I took this button-reading code from an example someone had posted and it worked fine, but it didn't have any nested functions like I have. I guess that when I press the button I'm not able to release it fast enough (<1ms) so the value is read again and so it moves back to the original function.
I'll try the CASE statements today.

Thank you very much for your help!

All the best,
Dax.

5

Hmm, perhaps I'm not getting the syntax of this correct?

"error: expected primary-expression before '<' token"

switch (MB1) {
  case <60:  //right button
    setTemperature();
    break;
  case (>=60 && <200): //up button
    uvtimer++;
    if (uvtimer >15){uvtimer = 15;}
    delay(250);
    break;
  case (>=200 && <400): //down button
    uvtimer--;
    if (uvtimer <1){uvtimer = 1;}
    delay(250);
    break;
  case (>=400 && <600): //left button
    setTemperature();
    break;
  case (>=600 && <800): //select button
    // statements
    break;  
}
6

Okay, I've gone back to the IF statements, but added ranges instead of open-ended values. (x>a && x<b)

Still no dice, it just changes and changes back. I added a 200ms delay before going to the next function, but it still just switches back, though now with a tiny delay.

I just can't understand why the heck it keeps changing back/

7

Well this is kinda interesting. With this code, it won't change to the setTemperature function unless I HOLD the button for 1 second. That doesn't make sense, right?

  if (MB1 < 60) { //right button
   delay(1000);
   setTemperature();

(and of course, as soon as it switches to the other function it switches straight back to the original :confused: )

8

Post your latest code.

9

Thanks @wildbill

#include <LiquidCrystal.h>
//LCD pin to Arduino
#define RELAY_UV         2  //
#define RELAY_TT         3  //Turntable
#define RELAY_FAN        11 //
#define RELAY_HEATER     12 //
const int pin_RS = 8; 
const int pin_EN = 9; 
const int pin_d4 = 4; 
const int pin_d5 = 5; 
const int pin_d6 = 6; 
const int pin_d7 = 7; 
const int pin_BL = 10; 
LiquidCrystal lcd( pin_RS,  pin_EN,  pin_d4,  pin_d5,  pin_d6,  pin_d7);

int uvtimer = 1;
int target_temp = 30;
int MB1; //multi-button 1 <60=right, <200=up, <400=down, <600=left, <800=select


void setup()
{
  pinMode(RELAY_UV, OUTPUT);
  pinMode(RELAY_TT, OUTPUT);
  pinMode(RELAY_FAN, OUTPUT);
  pinMode(RELAY_HEATER, OUTPUT);
  digitalWrite(RELAY_UV, LOW);
  digitalWrite(RELAY_TT, LOW);
  digitalWrite(RELAY_FAN, LOW);
  digitalWrite(RELAY_HEATER, LOW);
 lcd.begin(16, 2);
 lcd.setCursor(11,1);
 lcd.print("READY");
 
 lcd.setCursor(0,1);
 lcd.print(uvtimer, DEC),lcd.print(" mins ");
 }

void loop()
{
setUVTimer();
//setTemperature();
}


// Functions //
void setUVTimer()
{
  lcd.setCursor(0,0);
  lcd.print("Set UV time    ");
  MB1 = analogRead (0);  
  if (MB1 < 60) { //right button
   delay(1000);
   setTemperature();
  }
  else if (MB1 < 200) { //up button
   uvtimer++;
   if (uvtimer >15){uvtimer = 15;}
   delay(250);
  }
  else if (MB1 < 400){ //down button
   uvtimer--;
   if (uvtimer <1){uvtimer = 1;}
   delay(250);
  }
  else if (MB1 < 600){ //left button
   delay(1000);
   setTemperature();
  }
  else if (MB1 < 800){ //select button
   lcd.print ("Select"); 
  }
  lcd.setCursor(0,1);
  lcd.print(uvtimer, DEC),lcd.print(" mins ");
}

void setTemperature()
{
  lcd.setCursor(0,0);
  lcd.print("Set temperature");
  MB1 = analogRead (0);  
  if (MB1 < 60) { //right button
   delay(1000);
   setUVTimer();
  }
  else if (MB1 < 200) { //up button
   target_temp++;
   if (target_temp >60){target_temp = 60;}
   delay(180);
  }
  else if (MB1 < 400){ //down button
   target_temp--;
   if (target_temp <15){target_temp = 15;}
   delay(180);
  }
  else if (MB1 < 600){ //left button
   delay(1000);
   setUVTimer();
  }
  else if (MB1 < 800){
   lcd.print ("Select"); //select button
  }
  lcd.setCursor(0,1);
  lcd.print(target_temp, DEC),lcd.print("\xDF""C  ");
}

I added a 1 second delay before switching to a new function with the hope that if it was an issue with me not releasing the button fast enough it would cure that, but the most curious behaviour happens...

  if (MB1 < 60) { //right button
   delay(1000);
   setTemperature();
  }

It now requires me to HOLD that button for 1 second before it will move to the setTemperature function. (then, of course, it switches right back to the setUVTimer function.) I'm not sure why it would do that, since the code says to analogRead value once, evaluate it first, then do stuff, then repeat the analogRead.
Arduino is such a mature product that I can't imagine I've found a bug and I suspect it's more likely that there's some Arduino quirk that I'm unaware of. A programmer friend of mine suggested it might be holding the buffers, then releasing them after the delay time rather than following it explicitly.

Here's a video of it:

10

I think the delay is the problem. The button is read correctly but nothing happens for a second, so you keep pressing it until the other function is called which reads again and sends you back.

I suspect that if you tap the button and wait with your finger off the button, it will behave better.

11

Thanks @wildbill, my original code did not have the delay(1000); lines before changing functions. Without it, pressing the left/right buttons would cause it to switch to the other function then instantly switch back.
I thought the possible cause could be because I wasn't letting go of the button fast enough, but adding delay(1000); proved that something weird is going on.

I'm pretty confused. Why is it reading the button again? That's not the sequence of events as written in the code. The code says this should happen (example with right button pressed):

Start the setUVTimer(); function
Print stuff to LCD (uvtimer)
analogRead MB1
Evaluate MB1 value (right button)
Wait 1 second (during which button is released)
Go to setTemperature function

Print stuff to LCD (targetTemp)
analogRead MB1
Evaluate MB1 value (right button)
Wait 1 second (during which button is released)
Go to setUVTimer function

12

I still think you're underestimating how long a second is. Click it and go and get a cup of tea, or other beverage.

Paradoxically though, I think one part of the video, you're holding it long enough for the second delay to be up, the LCD will show the other menu title but immediately after, pick up the press, kick off another delay and return to the first menu.

13

Oh, I see it.

Loop calls setUVTimer repeatedly. If you press the right button, it calls setTemperature. If a button is pressed at that point, it will be serviced, but if not, in microseconds, setTemperature exits, so does setUVTimer and you're back in loop and setUVTimer is in charge again.

Add a mode variable that tells you which menu you're in. Flip it when the right button is pressed in either function. In loop, call only the function that the mode var wants.

14

Thanks @wildbill. Do you mean like adding a variable such as "menuPage"? Then the functions would start like:

void setUVTimer()
{
  menuPage = 0;

...

void setTemperature()
{
  menuPage = 1;
  lcd.setCursor(0,0);

I thought I could shortcut this by moving setUVTimer(); from void loop() to void setup(), but then the buttons stopped working altogether. (which is strange, because they should at least work for the first press, right?)

15

Not quite. Set menuPage when you press the button that changes the desired menu, not unconditionally.

Moving stuff to setup means it'll run once and you have less than a microsecond to press a button or you'll miss it.

16

Okay, then I'm officially stumped. I think this might be beyond my current level.
Thank you very much for your help with this @wildbill!

All the best,
Dax.

17

Hi daxliniere,

as a first thing:
I'm not sure what kind of functionality you want to achieve.

I write what my guess is what you want:

program starts
and program shal be in inputmode
set UV time

with pressing the up/down-buttons UV time can be adjusted
with pressing the "right"-button inputmode shall change to

set temperature
with up/down-button the temperature-value can be adjusted

pressing the "right"-Button shall change to inputmode

set UV time

Your previous code calls the a function from inside a function
this is nested calling and will lead to stackoverflow

imagine you have a servant that you give the order
"go buying a newspaper with the bicycle"
before the servant is back at your home you call him on the phone oh please can you buy flowers using the car

He has to put the bicycle into the car driving to buy flowers
then you call him again
please buy a magazine by using a bike.
Now the anaogon becomes ridiculious but this description is analog to what happens inside your code

the servant takes the car with the bike in trunk and put both on a second bike
and with each call a new car / bike is added always carrying around the former cars/bikes.

In a program all that has to be "carried around" is RAM-memory so not really heavy but memory-consuming

To avoid this your program should work like this

define two contants

#define setUVTime_value 1
#define setTemperature_value 2

in void setup set inputmode to set UV Time

inputmode = setUVTime_value

inside function setUVTimer if the "change input-mode-"button is pressed set

inputmode = setTemperature_value;

inside function setTemperature if the "change input-mode-"button is pressed set

inputmode = setUVTime_value;

inside function
inside loop
there are two if-conditions

if (inputmode  == setUVTime_value) {
  setUVTimer();
}

if (inputmode  == setTemperature_value) {
  setTemperature();
}

This means you use the variable "inputmode" to change what the name of the variable says

and each function gets terminated before the other function gets called.

This is like coming back with the bike put the bike into the shelter start driving with car
and bring car back into garage taking bike out of the shelter

best regards Stefan

18

Try this:

#include <LiquidCrystal.h>
//LCD pin to Arduino
#define RELAY_UV         2  //
#define RELAY_TT         3  //Turntable
#define RELAY_FAN        11 //
#define RELAY_HEATER     12 //
const int pin_RS = 8;
const int pin_EN = 9;
const int pin_d4 = 4;
const int pin_d5 = 5;
const int pin_d6 = 6;
const int pin_d7 = 7;
const int pin_BL = 10;
LiquidCrystal lcd( pin_RS,  pin_EN,  pin_d4,  pin_d5,  pin_d6,  pin_d7);

int uvtimer = 1;
int target_temp = 30;
int MB1; //multi-button 1 <60=right, <200=up, <400=down, <600=left, <800=select
bool InTemperatureMode = false;

void setup()
{
  pinMode(RELAY_UV, OUTPUT);
  pinMode(RELAY_TT, OUTPUT);
  pinMode(RELAY_FAN, OUTPUT);
  pinMode(RELAY_HEATER, OUTPUT);
  digitalWrite(RELAY_UV, LOW);
  digitalWrite(RELAY_TT, LOW);
  digitalWrite(RELAY_FAN, LOW);
  digitalWrite(RELAY_HEATER, LOW);
  lcd.begin(16, 2);
  lcd.setCursor(11, 1);
  lcd.print("READY");
  lcd.setCursor(0, 1);
  lcd.print(uvtimer, DEC), lcd.print(" mins ");
}

void loop()
{
  if (InTemperatureMode)
    setTemperature();
  else
    setUVTimer();
}


// Functions //
void setUVTimer()
{
  lcd.setCursor(0, 0);
  lcd.print("Set UV time    ");
  MB1 = analogRead (0);
  if (MB1 < 60)   //right button
  {
    InTemperatureMode = true;
  }
  else if (MB1 < 200)   //up button
  {
    uvtimer++;
    if (uvtimer > 15)
    {
      uvtimer = 15;
    }
    delay(250);
  }
  else if (MB1 < 400)  //down button
  {
    uvtimer--;
    if (uvtimer < 1)
    {
      uvtimer = 1;
    }
    delay(250);
  }
  else if (MB1 < 600)  //left button
  {
    delay(1000);
    setTemperature();
  }
  else if (MB1 < 800)  //select button
  {
    lcd.print ("Select");
  }
  lcd.setCursor(0, 1);
  lcd.print(uvtimer, DEC), lcd.print(" mins ");
}

void setTemperature()
{
  lcd.setCursor(0, 0);
  lcd.print("Set temperature");
  MB1 = analogRead (0);
  if (MB1 < 60)   //right button
  {
    InTemperatureMode = false;
  }
  else if (MB1 < 200)   //up button
  {
    target_temp++;
    if (target_temp > 60)
    {
      target_temp = 60;
    }
    delay(180);
  }
  else if (MB1 < 400)  //down button
  {
    target_temp--;
    if (target_temp < 15)
    {
      target_temp = 15;
    }
    delay(180);
  }
  else if (MB1 < 600)  //left button
  {
    delay(1000);
    setUVTimer();
  }
  else if (MB1 < 800)
  {
    lcd.print ("Select"); //select button
  }
  lcd.setCursor(0, 1);
  lcd.print(target_temp, DEC), lcd.print("\xDF""C  ");
}
1 Like
19

Hey @wildbill! That mostly works! Thank you so much!!
It was jumping back and forth between menu pages, but I suspected that was because the buttons were being held for longer than 1ms. I added a delay and now it's perfect!

  {
    delay(100);
    InTemperatureMode = false;
  }

Brilliant work, Bill, I really appreciate it. All the best!!
-Dax.

20

Here's the code in action!