[SOLVED] Problem with my program flow

jonhander12345 · December 11, 2020, 3:05am

My problem is that in a function when you enter a conditional and it is met, it must send me to another sector of the program. But instead what it does is continue with the next conditional of the same function

here is my example code

int  contador = 1;    // Cuenta el nivel del menu en el que se esta
int  led1 = 10;       // Variable pin entrada estado alarma
int  clave;           // Almacena ultima clave

void setup () {
  lcd.init();
  lcd.backlight();
  pinMode(llave, OUTPUT);
  pinMode(buzzer, OUTPUT);
  pinMode(led1, INPUT);
}

void loop() {
  if (contador == 1) {
    menu_1();
    accion_1();
  }
  if (contador == 2) {
    menu_2();
    accion_2();
  }
}
///////////////////// Menu_1 /////////////////////
void menu_1() {
  lcd.setCursor(0, 0);
  lcd.print("Mostrar Variable");
  lcd.setCursor(0, 1);
  lcd.print("   >");
  lcd.setCursor(5, 1);
  lcd.print(clave);
  lcd.setCursor(10, 1);
  lcd.print("   H>*");
  led1 = 2;
  avisodes();
  led1 = 2;
}
///////////////////// accion_1 /////////////////////
void accion_1() {
  if (pulsacion == '*') contador = 1;
}
///////////////////////////////////////////////////
void ini2() {
  lcd.setCursor(0, 0);
  lcd.print(" Iniciando  ... ");
  lcd.setCursor(0, 1);
  lcd.print("   2 ? ? ? ?    ");

  if (digitalRead(led1) == 0) {
    contador = 1;
  } else {
    ingresarpro();
    c21111();
    delay(250);
  }
  if (digitalRead(led1) == 0) {
    clave = 21111;
    contador = 1;
  } else {
    ini();
    c21112();
    delay(250);
  }
  if (digitalRead(led1) == 0) {
    clave = 21112;
    contador = 13;
  } else {
    ini();
    c21113();
    delay(250);
  }
  if (digitalRead(led1) == 0) {
    clave = 21113;
    contador = 1;
  } else {
    delay(9000);
    ini3();
  }
}

the problem is that when the conditional is met, for example the third "21112" and it goes to menu_1 instead of showing that saved variable, it shows the last variable of the function "21113"

I am using arduino uno, 4x3 keypad and a 16x2 lcd via I2C

Here is the complete code. Although it is very poorly optimized

Prueba_Subir.ino (52.6 KB)

gfvalvo · December 11, 2020, 3:23am

The code you posted does not compile.

IoT_hobbyist · December 11, 2020, 3:44am

Please post the full code. At least, you code need to include LCD Library

StefanL38 · December 11, 2020, 11:11am

the code you posted contains some functions that are not defined. how should somebody understand the program-flow if a part of the code is missing. simply not possible.

So please post your sketch from the very first line to the very last line.

Best regards Stefan

osval · December 11, 2020, 12:51pm

contador is alway=1 that´s why you cant jump to another part

jonhander12345 · December 12, 2020, 5:04am

StefanL38:
the code you posted contains some functions that are not defined. how should somebody understand the program-flow if a part of the code is missing. simply not possible.

So please post your sketch from the very first line to the very last line.

I tried to summarize it a bit so as not to bother with so much code. The rest were test menus and submenus and my problem I think is in the ini1 (), ini2 () and ini3 () functions.

oswe:
contador is alway=1 that´s why you cant jump to another part

I don't have so many problems with the one who sent me to the other menu. The problem is that instead of saving in the variable the value where the conditional is met for the first time, it saves me the variable of the last conditional of the function. For example, instead of showing me the value where "21112" was left, it shows me the last "21113". I hope what I need is well understood

Prueba_Subir.ino (54.5 KB)

StefanL38 · December 12, 2020, 9:05am

Huh ..... wow .....
well ..... that is a programming-concept too.
I tried to follow one path of possible keypresses.
Without knowing anything about the backround of this project it is hard to follow it.

You know your project very well. So could you please describe one path of keypressed very very very detailed.

With very very very detailed I mean

program starts contador is initialised with value 1
if a key is pressed the value of the key is stored into variable pulsation let's say key '1'
this means the if-condition if (contador == 1){ menu_1();accion_1();} becomes true
the display shows menu1

Menu5>1  Menu2>2
Menu6>3

and executes accion_1
inside accion_1
the followoing code is executed

void accion_1() {
  if (pulsation == '1') contador = 5;
  if (pulsation == '2') contador = 2;
  if (pulsation == '3') contador = 6;
}

this means as pulsation was NOT updated or resetted to 0 pulsation still has value '1' so
contador gets stored value 5

your code jumps immidiately to menu_5

I guess you coded a lot of lines of code before you started testing. Or if you did some testing you did not test all combinations. Is that true? I'm pretty sure the answer is yes.

This is one of the major "bad habits" of newbees. See my footnote.
Now you are in the situation that there already is a lot of code where it is hard to follow what is really happening.
So there are two ways:

start from scratch
or
add a lot of debug-output that makes visibile what your variables have for values which if-conditions are met and which not etc. etc. etc

I attached a version of your code that has serial-debug-output to show the principle how this can be done
you should add the serial-output to that lines of code that you want to examine.
This will make you understand what is really going on

You coded a lot of small functions that do just call other functions. Well this works but means to have a lot extra of lines of code

all your functions like

void c33312() {
  n3();
  n3();
  n3();
  n1();
  n2();
}

can be replaced by a variable that gets assigent the value 33312 and then call one single function that does some calculations to extract each digit and then using a switch-case to execute the right nX()-function
with this your code gets compressed down to just one function DoClaving()
and the calls of this function look like

  if (alm1 == 0) {
    clave = 11112;
    contador = 13;
  } else {
      ini();
      DoClaving(11113)            //instead of c11113();
      delay(250);
    }
  }

best regards Stefan

by the way: names like "Menu5" etc. are the opposite of self-explaining. If this is not part of an escape-room game
where there shall be an explicit difficulty to remember what does "menu5" mean you should use self-explaining names

jonhander-Prueba_Subir-Debug-Print-001.ino (55.4 KB)

jonhander12345 · December 13, 2020, 11:13pm

I have already managed to solve my problem with the flow of my program using switch-case (many switch-cases).

But if I wanted to know how to do that function or where to get information to guide me. From the beginning if I wanted to make a function that with calculation groups the minis functions n1, n2 and n3 with the indicated order but I have no idea how to do it or how to find information about it and thanks in advance :o

StefanL38:
You coded a lot of small functions that do just call other functions. Well this works but means to have a lot extra of lines of code

all your functions like
void c33312() {

n3();
n3();
n3();
n1();
n2();
}




can be replaced by a variable that gets assigent the value 33312 and then call **one single function** that does some calculations to extract each digit and then using a switch-case to execute the right nX()-function 
with this your code gets compressed down to just one function DoClaving()
and the calls of this function look like

if (alm1 == 0) {
clave = 11112;
contador = 13;
} else {
ini();
DoClaving(11113) //instead of c11113();
delay(250);
}
}

StefanL38 · December 14, 2020, 7:52am

So here is a demo-code with my bareminimum.ino-file which has some useful additional functions

it is simmilar to written dividing
extracting each digit from a intergernumber

and can be even more compressed through writing a function Do_n1n2n3(digit) which has the digit as parameter

//#include 
void PrintFileNameDateTime()
{
  Serial.println("Code running comes from file ");
  Serial.println(__FILE__);
  Serial.print("  compiled ");
  Serial.print(__DATE__);
  Serial.print(" ");
  Serial.println(__TIME__);  
}


boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();  
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

unsigned long MyTestTimer = 0;                   // variables MUST be of type unsigned long
const byte OnBoard_LED = 13;


void BlinkHeartBeatLED(int IO_Pin, int BlinkPeriod) {
  static unsigned long MyBlinkTimer;
  pinMode(OnBoard_LED, OUTPUT);
  
  if ( TimePeriodIsOver(MyBlinkTimer,BlinkPeriod) ) {
    digitalWrite(IO_Pin,!digitalRead(IO_Pin) ); 
  }
}



void setup() {
  Serial.begin(115200);
  Serial.println("Setup-Start");
  PrintFileNameDateTime();

}

long clave = 32121;

long rest;
long one;
long ten;
long hundred;
long thousand;
long tenthousand;

void loop() {
  BlinkHeartBeatLED(OnBoard_LED,100);

  
  if ( TimePeriodIsOver(MyTestTimer,1000) ) {
    rest = clave;
    
    tenthousand = rest / 10000;
    rest = rest - (tenthousand * 10000);    
    
    thousand = rest / 1000;
    rest = rest - (thousand * 1000);    

    hundred = rest / 100;
    rest = rest - (hundred * 100);    

    ten = rest / 10;    
    rest = rest - (ten * 10);    

    one = rest;
    
    Serial.print("10000 =");     
    Serial.println(tenthousand);
    
    Serial.print("1000=");     
    Serial.println(thousand);
    
    Serial.print("100=");     
    Serial.println(hundred);

    Serial.print("10=");     
    Serial.println(ten);

    Serial.print("1=");     
    Serial.println(one);

    switch (tenthousand) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      
    }

    switch (thousand) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      
    }
    switch (hundred) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      
    }

    switch (ten) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      
    }

    switch (one) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      
    }
    Serial.println();
    Serial.println();

  }  

}


void n1() {
  Serial.println("n1");
}

void n2() {
  Serial.println("n2");
}

void n3() {
  Serial.println("n3");
}

best regards Stefan

StefanL38 · December 14, 2020, 3:37pm

I knew that there is a much more compact way of extracting digits from an integer

this codeversion is prepared to execute n1, n2, n3, for any combination of 5digit integers between 11111 and 33333

//#include 
void PrintFileNameDateTime()
{
  Serial.println("Code running comes from file ");
  Serial.println(__FILE__);
  Serial.print("  compiled ");
  Serial.print(__DATE__);
  Serial.print(" ");
  Serial.println(__TIME__);  
}


boolean TimePeriodIsOver (unsigned long &expireTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();  
  if ( currentMillis - expireTime >= TimePeriod )
  {
    expireTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

unsigned long MyTestTimer = 0;                   // variables MUST be of type unsigned long
const byte OnBoard_LED = 13;


void BlinkHeartBeatLED(int IO_Pin, int BlinkPeriod) {
  static unsigned long MyBlinkTimer;
  pinMode(OnBoard_LED, OUTPUT);
  
  if ( TimePeriodIsOver(MyBlinkTimer,BlinkPeriod) ) {
    digitalWrite(IO_Pin,!digitalRead(IO_Pin) ); 
  }
}



void setup() {
  Serial.begin(115200);
  Serial.println("Setup-Start");
  PrintFileNameDateTime();
}


unsigned getDigit(unsigned p_Number, unsigned p_PosOfDigit) {

  unsigned Digit;
  // extract the digit from p_Number at p_PosOfDigit 
  switch (p_PosOfDigit) {
    case 1:Digit = (p_Number /     1U) % 10; break;
    case 2:Digit = (p_Number /    10U) % 10; break;
    case 3:Digit = (p_Number /   100U) % 10; break;
    case 4:Digit = (p_Number /  1000U) % 10; break;
    case 5:Digit = (p_Number / 10000U) % 10; break;

    default: Digit = 0; break;
  }
  return Digit; // handover this digit back
}


void Do_n123(unsigned p_Digit) {
  switch (p_Digit) {
      case 1: n1(); break;
      case 2: n2(); break;
      case 3: n3(); break;      

      default: notValidDigit(); break;
  }
}


void DoClaving(unsigned p_ClaveNr) {
  for (unsigned Pos = 5; Pos > 0;Pos-- ) {
    Do_n123( getDigit(p_ClaveNr, Pos) );     
  }
}


void loop() {
  BlinkHeartBeatLED(OnBoard_LED,100);

  
  if ( TimePeriodIsOver(MyTestTimer,1000) ) {
    unsigned myNumber = 32121;
    
    Serial.print("myNumber has value ");
    Serial.println(myNumber);
    Serial.println();
    DoClaving(myNumber);
    Serial.println();
    Serial.println();
  }  

}


void n1() {
  Serial.println("n1");
}


void n2() {
  Serial.println("n2");
}


void n3() {
  Serial.println("n3");
}


void notValidDigit() {
  Serial.println("Error no valid Digit");
}

best regards Stefan

system · May 6, 2021, 12:53am