Go Down

Topic: Poor coding structure--need help (Read 648 times) previous topic - next topic

flyboy

Before arduino, I've never used C.  My prior experience, which is mainly fortran, pascal, basic, and assembly, is from classes in school and not really for real life interfacing, etc.  My delimma is this: I believe I need a state machine structure.  I think I'm on the right track, but I need an experienced coding jedi to steer me in the right direction.  Attached is my program.  I'm trying to enter values in the first two menus and then show the total value in the last menu.  I think I'm close, but my program gets stuck somewhere.  Any help would be greatly appreciated.
Quote

#include <LiquidCrystal.h>
//This is a short program to explore a method of using delay timing to
//cause a value to incriment slowly until the button has been held down
//for a period of time, after which it will incriment much faster
//The initial delay is reset if the button is released
//LiquidCrystal(rs, rw, enable, d0, d1, d2, d3)
//My Display pinout(rs-4, rw-5, enable-6, d0-11, d1-12, d2-13, d3-14)
LiquidCrystal lcd(12, 11, 10, 5, 4, 3, 2);
int backBtn = 14;          //These are the input buttons
int incBtn = 15;
int decBtn = 17;
int nextBtn = 18;
int selectBtn = 16;
int milliSeconds = 0;     //starting value for milliSeconds
int microSeconds = 0;
int del = 10;             //This is my delay while the button is pressed
int back;                 //The values used for a digital read
int inc;
int dec;
int next;
int select;
int debounce = 20;        //button debounce time
int state = 0;
void setup()
 {
  pinMode(backBtn,INPUT);         //set pin as input
  digitalWrite(backBtn, HIGH);    //turn on the internal pull-up
  pinMode(incBtn,INPUT);
  digitalWrite(incBtn, HIGH);     //turn on the internal pull-up
  pinMode(selectBtn,INPUT);
  digitalWrite(selectBtn, HIGH);  //turn on the internal pull-up
  pinMode(decBtn,INPUT);
  digitalWrite(decBtn, HIGH);     //turn on the internal pull-up
  pinMode(nextBtn,INPUT);
  digitalWrite(nextBtn, HIGH);    //turn on the internal pull-up
  constrain(del, 0, 10);          //limit the range of values for del
  constrain(milliSeconds, 0, 999); //limit the range of values for milliSeconds
  constrain(microSeconds, 0, 999);  
  constrain(state, 0, 2);
}

void loop()
 {
//    lcd.clear();
//    lcd.print("Hi-Speed Camera");
//    lcd.setCursor(0,1);
//    lcd.print("  delay timer");
//    delay(3000);
//    lcd.clear();
   currentState();         //go to the first menu -- choose the milliseconds value
       if(state == 0)
     {
       choosemilliSeconds();
     }
   if(state == 1)
     {
       choosemicroSeconds();
     }
   if(state == 2)
     {
       showTotalTime();
     }
 }                               //close void loop

void btnPressed()
 {
   if (del > 0)                //if del value is greater than 0, we want
     {                         // to delay a bit to increment more slowly
       del=(del-1);
       delay(250);
     }
 }                            //close void btnPressed
 
void printRightArrow()
 {
   lcd.setCursor(11,1);
   lcd.print("next");
   lcd.write(0b01111110);
 }
void printLeftArrow()
 {
   lcd.setCursor(0,1);
   lcd.write(0b01111111);
   lcd.print("back");
 }

void printmilliSeconds()
 {
   lcd.setCursor(0,0);
   if (milliSeconds < 100)
     {
       lcd.print("0");        //add zeroes as place holders on the display
     }
   if (milliSeconds <10)
     {
       lcd.print("0");
     }
     lcd.print(milliSeconds);  //print the current value on the display
     lcd.print(" msec");
 }
void choosemilliSeconds()
 {
   readBtns();
   while(next == HIGH)
   {
   readBtns();
   printmilliSeconds();
   printRightArrow();
   if (dec == LOW)               //If I press the decriment button, begin reducing the milliSeconds value
   {
     if(milliSeconds >0)
       {
         milliSeconds = (milliSeconds-1);
       }
     btnPressed();
   }
 if (inc == LOW)               //If I press the incriment button, begin increasing the milliSeconds value
    {
     if(milliSeconds <999)
       {
         milliSeconds = (milliSeconds+1);
       }
     btnPressed();
    }
   }
 readBtns();
/*  if(next == LOW)
 {
   choosemicroSeconds();
 }*/                             //close While loop
 }                             //close void choosemilliSeconds
void readBtns()
 {
   back = digitalRead(backBtn);      //Read the value of backBtn
   next = digitalRead(nextBtn);      //Read the value of nextBtn
   select = digitalRead(selectBtn);  //Read the value of selectBtn
   inc = digitalRead(incBtn);        //Read the value of incBtn
   dec = digitalRead(decBtn);        //Read the value of decBtn
   delay(debounce);                  //Wait a short time
   if (back == HIGH && next == HIGH && inc == HIGH && dec == HIGH && select == HIGH)
     {
       del = 10;                 //if no buttons are pressed, del is set to 10
     }
 }
void choosemicroSeconds()
 {
   readBtns();
   while(back == HIGH && next == HIGH)
  {
    readBtns();
   printmicroSeconds();
   printLeftArrow();
   printRightArrow();
   if (dec == LOW)               //If I press the decriment button, begin reducing the milliSeconds value
   {
     if(microSeconds >0)
       {
         microSeconds = (microSeconds-1);
       }
     btnPressed();
   }

 if (inc == LOW)               //If I press the incriment button, begin increasing the milliSeconds value
   {
     if(microSeconds <999)
       {
         microSeconds = (microSeconds+1);
       }
     btnPressed();
   }
   }                          //close While loop  
/*     readBtns();
   
 if(next == LOW)
 {
//   lcd.clear();
  showTotalTime();
 }
 if(back == LOW)
[color=#7

AlphaBeta

#1
Apr 19, 2009, 01:59 am Last Edit: Apr 19, 2009, 02:07 am by AlphaBeta Reason: 1
Quote
...Any help would be greatly appreciated.


It would've been simpler if you posted the whole code. If it is long you maybe want to use http://arduino.pastebin.com/

Post here again when that is done, including a URL if you decide to use pastebin. I'll gladly do my best to help.
:)

flyboy

I think I have this thing figured out.  I think my debounce time was too short.  I increased the time and I seem to have all of the menus that I expected to have.  I'll play with it some more and let you know how it goes.  Thanks for your help.

Anachrocomputer

Looks like you've misunderstood the use of 'constrain' as a library function.  You can't call it in your 'setup()' code and expect it to constrain the variable throughout your code (nice though that would be).  You must call 'constrain()' each time you want the variable's value to be constrained, and you must assign the result back into a variable (i.e. 'constrain()' is a function returning a value).

Could you post the whole of your sketch code?

flyboy

Here is what I have at this time.  I'm trying to get these 3 menus to work.  There is some code that has been commented out.  That was some stuff that I was trying, but had even less success with.  Once I get the menus working, I'll move on, so here's what I have so far:


Code: [Select]
#include <LiquidCrystal.h>

//This is a short program to explore a method of using delay timing to
//cause a value to incriment slowly until the button has been held down
//for a period of time, after which it will incriment much faster
//The initial delay is reset if the button is released

//LiquidCrystal(rs, rw, enable, d0, d1, d2, d3)
//My Display pinout(rs-4, rw-5, enable-6, d0-11, d1-12, d2-13, d3-14)

LiquidCrystal lcd(12, 11, 10, 5, 4, 3, 2);

int backBtn = 14;          //These are the input buttons
int incBtn = 15;
int decBtn = 17;
int nextBtn = 18;
int selectBtn = 16;
int milliSeconds = 0;     //starting value for milliSeconds
int microSeconds = 0;
int del = 10;             //This is my delay while the button is pressed
int back;                 //The values used for a digital read
int inc;
int dec;
int next;
int select;
int debounce = 20;        //button debounce time
int state = 0;

void setup()
 {
  pinMode(backBtn,INPUT);         //set pin as input
  digitalWrite(backBtn, HIGH);    //turn on the internal pull-up
  pinMode(incBtn,INPUT);
  digitalWrite(incBtn, HIGH);     //turn on the internal pull-up
  pinMode(selectBtn,INPUT);
  digitalWrite(selectBtn, HIGH);  //turn on the internal pull-up
  pinMode(decBtn,INPUT);
  digitalWrite(decBtn, HIGH);     //turn on the internal pull-up
  pinMode(nextBtn,INPUT);
  digitalWrite(nextBtn, HIGH);    //turn on the internal pull-up
  constrain(del, 0, 10);          //limit the range of values for del
  constrain(milliSeconds, 0, 999); //limit the range of values for milliSeconds
  constrain(microSeconds, 0, 999);  
  constrain(state, 0, 2);
}

void loop()
 {
//    lcd.clear();
//    lcd.print("Hi-Speed Camera");
//    lcd.setCursor(0,1);
//    lcd.print("  delay timer");
//    delay(3000);
//    lcd.clear();
   currentState();         //go to the first menu -- choose the milliseconds value
       if(state == 0)
     {
       choosemilliSeconds();
     }
   if(state == 1)
     {
       choosemicroSeconds();
     }
   if(state == 2)
     {
       showTotalTime();
     }

 }                               //close void loop

void btnPressed()
 {
   if (del > 0)                //if del value is greater than 0, we want
     {                         // to delay a bit to increment more slowly
       del=(del-1);
       delay(250);
     }
 }                            //close void btnPressed
 
void printRightArrow()
 {
   lcd.setCursor(11,1);
   lcd.print("next");
   lcd.write(0b01111110);
 }

void printLeftArrow()
 {
   lcd.setCursor(0,1);
   lcd.write(0b01111111);
   lcd.print("back");
 }

void printmilliSeconds()
 {
   lcd.setCursor(0,0);
   if (milliSeconds < 100)
     {
       lcd.print("0");        //add zeroes as place holders on the display
     }
   if (milliSeconds <10)
     {
       lcd.print("0");
     }
     lcd.print(milliSeconds);  //print the current value on the display
     lcd.print(" msec");
 }
 
void choosemilliSeconds()
 {
   readBtns();
   while(next == HIGH)
   {
   readBtns();
   printmilliSeconds();
   printRightArrow();
   if (dec == LOW)               //If I press the decriment button, begin reducing the milliSeconds value
   {
     if(milliSeconds >0)
       {
         milliSeconds = (milliSeconds-1);
       }
     btnPressed();
   }

 if (inc == LOW)               //If I press the incriment button, begin increasing the milliSeconds value
    {
     if(milliSeconds <999)
       {
         milliSeconds = (milliSeconds+1);
       }
     btnPressed();
    }
   }
 readBtns();
/*  if(next == LOW)
 {
   choosemicroSeconds();
 }*/                             //close While loop
 }                             //close void choosemilliSeconds
 
void readBtns()
 {
   back = digitalRead(backBtn);      //Read the value of backBtn
   next = digitalRead(nextBtn);      //Read the value of nextBtn
   select = digitalRead(selectBtn);  //Read the value of selectBtn
   inc = digitalRead(incBtn);        //Read the value of incBtn
   dec = digitalRead(decBtn);        //Read the value of decBtn
   delay(debounce);                  //Wait a short time
   
   if (back == HIGH && next == HIGH && inc == HIGH && dec == HIGH && select == HIGH)
     {
       del = 10;                 //if no buttons are pressed, del is set to 10
     }

 }
void choosemicroSeconds()
 {
   readBtns();
   while(back == HIGH && next == HIGH)
  {
    readBtns();
   printmicroSeconds();
   printLeftArrow();
   printRightArrow();
   if (dec == LOW)               //If I press the decriment button, begin reducing the milliSeconds value
   {
     if(microSeconds >0)
       {
         microSeconds = (microSeconds-1);
       }
     btnPressed();
   }

 if (inc == LOW)               //If I press the incriment button, begin increasing the milliSeconds value
   {
     if(microSeconds <999)
       {
         microSeconds = (microSeconds+1);
       }
     btnPressed();
   }
   }                          //close While loop  
/*     readBtns();
   
 if(next == LOW)
 {
//   lcd.clear();
  showTotalTime();
 }
 if(back == LOW)
 {
//    lcd.clear();
   choosemilliSeconds();
 }*/
 }                          //close void choosemicroSeconds
 
void printmicroSeconds()
 {
   lcd.setCursor(0,0);
   if (microSeconds < 100)
     {
       lcd.print("0");        //add zeroes as place holders on the display
     }
   if (microSeconds <10)
     {
       lcd.print("0");
     }
     lcd.print(microSeconds);  //print the current value on the display
     lcd.print(" usec");
  }                            //close void printmicroSeconds


void showTotalTime()          //show the total delay time
 {
   readBtns();
   while(back == HIGH && next == HIGH)
  {
    readBtns();
   printLeftArrow();
   printRightArrow();
   lcd.setCursor(0,0);
  lcd.print("0.");
  if (milliSeconds < 100)
     {
       lcd.print("0");        //add zeroes as place holders on the display
     }
   if (milliSeconds <10)
     {
       lcd.print("0");
     }
     lcd.print(milliSeconds);  //print the current value on the display

  lcd.print(",");

  if (microSeconds < 100)
     {
       lcd.print("0");        //add zeroes as place holders on the display
     }
   if (microSeconds <10)
     {
       lcd.print("0");
     }
     lcd.print(microSeconds);  //print the current value on the display
     lcd.print(" sec");
  }                             //close While loop

/*     readBtns();
   
 if(next == LOW)
 {
  lcd.clear();
  choosemilliSeconds();
 }
 if(back == LOW)
 {
   lcd.clear();
   choosemicroSeconds();
 }*/
}                              //close showTotalTime

void currentState()
 {
   readBtns();
   delay(debounce);
   if(back == LOW)
     {
       state = state-1;
     }
   if(next == LOW)
     {
       state = state-1;

//        return;
     }
 }

AlphaBeta

I think maybe you should start over.

Open a new sketch and implement some functionality, get it to work, and implement the next.


I often find it useful to separate the operations needed, such as:

//LOOP
   //READ BUTTON STATES
   //DO LOGIC THAT USES BUTTON STATES
   //DO OTHER LOGIC
   //OUTPUT THE RESULTS

I've actually made two libraries becuase of you code :) Thank you for the inspiration.
The two libs are:

  • Menu Library

    • Makes creating and managing menu systems easier.
    • Event based, no source code needs to be edited.

  • Constrain Library

    • Creates variables that are constrained throughout the scope of that variable. No longer needed to call constrain() everytime you need to use the variable.




PostScriptum: Although it might seem wierd to post my libraries like this, I do not intend for anything but to be helpful for others.

flyboy

That's kind of what I'm getting from one of our software engineers at work.  She has suggested using routines that test states and change variables.  Once the variable is processed, change it back to its original state, for example.  I'll get back to you.

Go Up