Need help programming multiple menus, cursor wont move!

Ok, I am an aspiring electronics engineer. I was challenged recently to develope from scratch a “Climate Control System” for a friend who is working to start their own local nursery. This job will pay good for me so I have to get it done. Basically, for certain things that are grown out of season and must be grown in a controlled environment indoors, I have to develope a system that can handle up to around 3600 watts spread out over a total of 8 different “zones”. Each zone by itself must be “programmable”, as in each zone must be set on certain times for an ON/OFF cycle (like one zone 60 min on 120 min off, another 5 min on 10 min off or whatever). Basically each zone is settable to a custom ON/OFF time.

Anyways, Im using an atmega 328 16MHz microcontroller to control the whole thing, and I am prototyping using an arduino board. Ok well the way I have it set up is it starts off with a menu with 2 options, run or set. Set leads to more menus in which you can set the timing for the 8 different zones. Run will run the settings and display different information like temperature for a few different areas etc. I havent gotten that far yet. The whole thing is controlled via 3 buttons, a left, a right, and an ok button. Its self explanatory, the directions move a cursor through the menus and ok selects whatever in the menu. Pushing both right and left buttons resets the program and returns to the main menu (set run).

Basically, I have the first main menu programmed and started to do the second menu (zone selection). The first menu by itself works like a charm, everything works as it should. But its when I try to add the second menu things start messing up. Whenever I add the second menu, the right button will no longer move the cursor around. When I reset it and return to the main menu, the cursor wont move to the right but I know the right button still works cause it will still reset when I push right and left together. It just wont move the cursor like it should. The buttons are wired to an analog pin via code that was posted by digimike (http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1267115381). Here is my program so far:

#include <LiquidCrystal.h>

LiquidCrystal lcd(2, 4, 3, 5, 6, 7);
int j = 1; 
int label = 0; 
int counter = 0;
long time = 0;
int debounce_count = 25;
int current_state = 0;
int ButtonVal;
int x = 1;
int y = 1;
int menu = 1;  //for reporting on which menu to display
int zone = 1; //zone number
int Button[4][3] = {{1, 965, 966}, //left
                        {2, 895, 896}, //ok
                        {3, 613, 614}, //right
                        {4, 969, 970}}; //left and right

void setup(){
  analogReference(DEFAULT);
  lcd.begin(16, 2);
  lcd.clear();
  lcd.setCursor(1, 0);
  lcd.print("Climate Control");
  lcd.setCursor(2,1);
  lcd.print("System V1.0");
  delay(2000);
  lcd.clear();

}

void loop(){
 
  Menucheck(); //to check to see which menu the system is currently on and display it
 
  if (millis() != time){
    ButtonVal = analogRead(A0);
    if (ButtonVal == current_state && counter > 0){
      counter--;
    }
    if (ButtonVal != current_state){
      counter++;
    }
    if (counter >= debounce_count){
      counter = 0;
      current_state = ButtonVal;
      if (ButtonVal > 0){
        ButtonCheck();
      }
    }
    time = millis();
  }
}


void ButtonCheck(){
  for (int i = 0; i <= 4; i++){
    if (ButtonVal >= Button[i][j] && ButtonVal <= Button[i][j+1]){
      label = Button[i][0];
      Action();
    }
  }
}

void Action(){
  
  //menu #1 this sections works fine by itself
  
  if (menu == 1){  //check menu variable to see which course of actions to take
  //move right
  if (label == 3){
    x = 7;
    lcd.setCursor(1,1); //clears the pointer from last position
    lcd.print(" ");
  }
  
  //move left
  if (label == 1){
    x = 1;
    lcd.setCursor(7, 1);
    lcd.print(" ");
  }
  
  //select
  if (label == 2 && x == 1){
    lcd.clear();
    menu = 2;
    Menucheck();  //if ok is pushed on "set", then menu will equal 2 which is the next menu, menucheck will then display the next menu
  }
  
  if (label == 2 && x == 7){
    lcd.clear();
    lcd.setCursor(0,0);
    lcd.print("Run");
    delay(1500);          //ignore this section, have not programmed the run portion yet this just
    lcd.clear();           //displays the word "run" whenever run is selected on the main menu as a test
    loop();
  }
  
  //reset
  if (label == 4){
    setup();         //resets the program back to the main menu
  }
    
  }
  //menu #2               //adding this code alone or with the menu 2 code below creates the problem
  if (menu == 2){         //again checks to see what menu is current, and displays its elements
    //increase zone
    if (label == 3){
      zone = zone + 1;  //this is part of the zone selection (menu 2, under "set"), pushing right increases zone number
    }
    //decrease zone
    if (label == 1){
      zone = zone - 1;  //pushing left decreases zone number, i had code to detect if its at the limits of the 1-8 range
    }                           //so if it is 1 and zone is decreased it would stay at one and vice versa for when its at 8
  }                             //but i removed it after having the problem for testing purposes
   lcd.setCursor(x, y);
     lcd.print(">");       //displays the menu pointer, had this encased in "else" statement but removed for troubleshooting, also
                                 //discovered that the "else" statement isnt really needed cause it still works on the first menu either way
}


void Menucheck(){      //code to check which menu the system is currently on and display the appropriate info
  switch(menu){
    case 1:
     lcd.setCursor(0,0);
     lcd.print("Welcome Daryl");
     lcd.setCursor(2,1);
     lcd.print("Set");                  //this section by itself works fine
     lcd.setCursor(8,1);
     lcd.print("Run");
     lcd.setCursor(x, y);
     lcd.print(">");
     break;
     
     case 2:
     lcd.setCursor(0,0);
     lcd.print("Select Zone");
     lcd.setCursor(1, 1);
     lcd.print("Zone num:");   //once again adding this section either by itself or in conjunction with the above menu 2 action
     lcd.setCursor(11,1);       //code creates problems...i.e. the right button will no longer move the cursor around on the first menu
     lcd.print(zone);
     break;
  }
}

So yeah thats it. Like I said, the above program works fine if I dont add the “case 2:” section at the end, or the “menu 2” section under the Action function. Either one will mess it up. Like I said without one or both of those two parts the program works and on the main menu the cursor moves around like it should, pressing ok on run will display the word run and return to the main menu (like its temporarily programmed to do) etc. When I add any code pertaining to the second menu the right button will no longer move the cursor on the main menu like it should. It is still functional though cause when right and left buttons are pushed it will reset just like I programmed it to do, plus when I push ok on “set” it will go to the next menu just like it should. But on the next menu under set “zone select” it will not change zone numbers like it should it just stays on “zone 1”.

Please help! As you can see my knowledge of programming is extremely limited. The only software programming experience I really have is old ass BASIC on 386 machines and what not (and TI basic and apple II e hahaha). I have never programmed in C before until I purchased the arduino board. So Im basically just figuring this shit out as I go along. I have programmed other stuff with the arduino board just messing around like stepper motors and control of that with display, inputs, PWM, etc etc but never wrote a whole program with a purpose before. So any help is greatly appreciated.

Actually on second thought I dont think the left or the right works after adding the second menu. There is no way to tell on the first menu, but the way its programmed now (without the code for the second menu that says "ok your on zone one, you pushed left, stay at zone one, and the same for zone 8 and the right button", which i removed for troubleshooting) it starts on zone one on the second menu and it should display "0" if I push left. But that doesnt work either so that leads me to beleive both left and right stop working. But like I said I know they are still functional cause even though it doesnt work on the first menu if i push right and left together it will reset.

As far as all the other circuitry goes it is all written down, drawn out, and simulated. It will work perfectly. Basically, the atmega will be wired up appropriately (Vcc, internal reset, 16MHz xtal with caps etc). The actuall physical circuit will be made using a homemade "press and peel" type setup, where the physical layout is printed using toner, transferred to the raw board with heat, etched, drilled, soldered mounted and enameled etc. Which I have done before with much success. The whole shebang will be mounted in a fan cooled metal enclosure that used to be an old battery backup system for a desktop with some simple modifications (its gutted, special holes/mounts cut/drilled whatever). The whole thing will be designed so that the extra pins not used on the microcontroller will be ported for future expandibility (complete with ICSP port).

The circuit itself goes basically like this:
One of the analog inputs on the MCU is for controlling the buttons. 3 will be wired to thermistor sensor circuits for temp monitoring, and the remaining 2 will be left for future expandibility. The "Tx and Rx" pins will be left open for future expandibility (interfacing with another MCU, ICSP etc). 6 digital I/O pins will be used to drive the lcd display (simple blue 16 by 2 character lcd display with pots for adjusting brightness and contrast). 2 digital I/O pins will be left open. The remaining four digital I/O pins will be used to control the 8 zones. 3 will output a binary number 000-111 (giving 8 possible states) to external "demultiplexing" logic (using "and", "or", and inverter chips, or "nor/nand" whichever is more effecient). The logic circuitry has an input of 3 pins (the binary num), and an output of 8 pins (one for each zone 1-8, or 000-111). The remaining digital I/O pin from the MCU will be used as the ON/OFF selector.
Basically, to set a zone on or off, the MCU will assert the appropriate binary num on the 3 pins to select a specific zone. The on/off selector will be wired to every zone via a bus. When the MCU selects a zone the demultiplexing logic will send 5V to a transistor which acts to route the signals from the on/off bus to whatever zone (like the enable pin on a memory chip). So in other words the on/off bus sends signals to all zones at once, but the zone selector logic will enable only one specific zone through which the on/off signals will pass in much the same way a computer system addresses and enables different chips in the system. This is then routed to flip-flops which will hold the state on or off while the MCU selects another zone or waits. The outputs of the flip-flops are wired into a series of more transitors to act as buffers (to supply more current to the relay coils), and those are wired into relays which will activate with 5V (but with contacts that can handle up to 120V AC at around 1200W a peice). The out[uts of the relays are wired to female recepticals so you can just plug a regular 3 prong AC plug into each zone. The whole thing will run off its own 120V 30 amp (3600W) line. The line will be wired parallel into the relays, and into a step down transformer that will give me about 15V AC. This transformer will supply the logic circuitry and MCU. It will be wired into a bridge rectifier, through a few caps and choke coil for rough filtration, then to some 5V regulators and more caps for filtration.

A little long winded I know, but that is the gist of my design. Just wanted to give a little background to give an idea of what im working with.

Oh yeah I forgot to mention that an LED will be wired off of every zone to indicate if the zone is on or off.

  if (millis() != time){

The millis() function returns an unsigned long, so the value that it returns, or any comparisons to what it returns, should also involve unsigned longs. Get in the habit now of using unsigned long for all time related variables.

Using const int variables or #define to give meaningful names to pin numbers is a good idea.

int Button[4][3] = {{1, 965, 966}, //left
                        {2, 895, 896}, //ok
                        {3, 613, 614}, //right
                        {4, 969, 970}}; //left and right

The ranges of values for the voltage that corresponds to a single button is pretty small - possibly two small. The range that corresponds to both buttons is the same size, probably too small, and too close to that of left only. Are those ranges reliably defining which button(s) are pressed?

    x = 7;

One letter global variable names are a disaster waiting to happen.

Adding some Serial.print() statements to your code would be a good way to debug the problem. Print out which button(s) was/were pushed. Determine if you have a hardware problem, or a software problem, or both.

Seems like you're mixing everything together. I don't see the point of tweaking this code and seeing it get into trouble in a later stage when you add another menu. I suggest you separate your code into a block that only does one menu, another block that does another menu. They should not have any connection so later you can copy and paste one of these to get a third menu. If you really need something that works with multiple menus and inputs you can check out my blog but since you've done well enough to get one menu working, I don't want just ruin your work by linking to mine yet. :wink:

The millis() function returns an unsigned long, so the value that it returns, or any comparisons to what it returns, should also involve unsigned longs. Get in the habit now of using unsigned long for all time related variables.

Using const int variables or #define to give meaningful names to pin numbers is a good idea.

So in other words your saying when I declare the time variable I should use “unsinged long time = 0;” instead?
I figured I would just use the pin number directly as I thought it would take up extra program space on the MCU. I read in the reference that the #define command doesnt take up space on the MCU, but it didnt say anything about that with the const int command. If you know which pins are connected to what then I see no reason to label them using a variable, unless using pin numbers directly may cause problems somehow.

The ranges of values for the voltage that corresponds to a single button is pretty small - possibly two small. The range that corresponds to both buttons is the same size, probably too small, and too close to that of left only. Are those ranges reliably defining which button(s) are pressed?

So I should probably dig some resistors up with a wider range of values to separate the ranges more huh? And increase the button range in the array a little more? I used a 3.3k to ground, 200 ohm on left, 470 ohm on ok, and a 2.2k on right. The thing is, I wrote a program to display the analog values of each button on the lcd to get the ranges, and everytime I load it up it still returns the same values wich fluctuate only by one point. Also I have another program I wrote to test moving a cursor around that also displays when ok is pressed and when combos are pressed and it works fine. Hell my program worked fine until I started adding the code for menu 2 in both Action() and Menucheck(). Adding just one or the other screws it up, take both out it runs perfectly.

One letter global variable names are a disaster waiting to happen.

Why? Whats the difference if you use one letter or six letters? As long as you keep track of them right? Since x and y control lcd cursor position, maybe I should use “row” and “col” instead.

Adding some Serial.print() statements to your code would be a good way to debug the problem. Print out which button(s) was/were pushed. Determine if you have a hardware problem, or a software problem, or both.

Well thats a damn good idea why didnt I think of that. Duh! Im leaning more toward a range problem rather than a button recognition problem. Like I said my software reset still works when I press left+right combo so obviously its still recognizing its being pushed right?

@liudr:

Yeah I think you may be right. I had already considered doing that Im just not eactly sure how I want to structure it yet. Im thinking I may keep the button code for debouncing and scanning etc in the void loop, and then use the menu variable coupled with if statements or switch/case also in the void loop to determine which menu to display etc. I'll put every menu and its corresponding button actions in its own function and just have the switch/case call whatever function.

What do you think about that?

Every menu in its own place but buttons are lower level and shoud have a common interface, like, senseall() to sense allbuttons for the menu to make sense of what happened and responds. Every menu item calls a function of the menu's purpose.

If you know which pins are connected to what then I see no reason to label them using a variable, unless using pin numbers directly may cause problems somehow.

Using pin numbers directly won’t cause problems, if you keep track of which pin is doing what.

But, something like this:

#define heaterOnePin 4
#define heaterTwoPin 6
#define switchPin 8

won’t ever have you trying to read from a heater pin.

Why? Whats the difference if you use one letter or six letters?

Because loop variable are usually one letter variables.
(for int x=0; x<4; x++)
{
}
and you’ve just introduced a local variable that hides the global variable. Yes, row and col are much better names.

Like I said my software reset still works when I press left+right combo so obviously its still recognizing its being pushed right?

I just noticed this.

  //reset
  if (label == 4){
    setup();         //resets the program back to the main menu
  }

Calling setup again is a really bad idea. It does not reset any of the hardware. If you just want to re-initialize some variables, create your own initialization routine that you call from setup and here in this function.

Yeah it didnt work. In fact it made it worse. I tried everything. I rearranged the program in all kinds of ways and no go. I even started over and redesigned the structure, changed resistor values so the ranges for each button were farther apart, I tried breaking it up into functions, all that shit. It just kept doing the same thing. And after I redesigned it, I just programmed the first menu and actions to display "run" or "set" when you select either one and it didnt even work with just the one menu this time.

But I know the buttons are being recognized. Like I said I changed the resistors and gave them new wider ranges that are farher apart. I added a serial comman to report the value of the buttons and the values were right on target. Plus The ok button works but I can only select "set" cause I cant move the cursor, the cursor flickers when I try to move it, and my button combonation to reset the program also worked. So its got to be some kind of programming error.

Arrrrgggggg Im so damn frustrated! I been messing with this thing all damn day. My back hurts and my eyes are bugging out!

Because loop variable are usually one letter variables.
(for int x=0; x<4; x++)
{
}
and you’ve just introduced a local variable that hides the global variable. Yes, row and col are much better names.

I mean yeah that would happen if you tried to use the same letter but thats why you keep track of stuff and take notes. If I were gonna write a loop variable I would use a different letter. If I used say i and j for loops then how would that mask the global variables x and y? I’ll use row and col anyway just in case but I dont see a problem with just using one letter for global variables.

Calling setup again is a really bad idea. It does not reset any of the hardware. If you just want to re-initialize some variables, create your own initialization routine that you call from setup and here in this function.

Well i know its not a hardware reset! I dont want to reset the hardware I will have an internal button in the box for a hard reset. With the soft reset I want it to not just initialize variables (which I didnt include resetting of the variables in the setup of what I posted above but I have included that in later versions), I also want it to just restart the program, show the title screen, and then return back to the main menu. The reason for this is because the way I’ll have it setup is once you set all the zones and then choose run it will just run over and over in a loop monitoring and controlling the zones, as well as displaying temperature information and other stuff on the lcd. To break the loop I want it where you hit the key combo and it restarts back to the main screen. All environment variables will be re-initialized but the zone variables I want to remain the same so if you hit run again it will still have the same zone timings unless you change them, do a hardware reset, or disconnect power. Which will suck if the power goes out cause then you would have to reset everything. Maybe I should include circuitry with a battery so settings will keep just in case of power loss.

Maybe I should include circuitry with a battery so settings will keep just in case of power loss.

Maybe you should store the settings in EEPROM, and re-read them in setup.

Actually thats not a bad idea! I totally forgot these atmegas have internal EEPROM storage. I may have to incorporate that.

I'm wondering what your hardware looks like, especially the three buttons and resistors.