Can't make button increment my Array :(

Here is the code. It is not finished so some of it will not seem useful.

const int menuBut = 7;
const int upBut = 9;
const int downBut = 8;
#include <LiquidCrystal.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
char value[16];
int number = 5;
char* topLine[4]={"  Temperature   ", "   Temp.  Set   ", "  Calibration   ", "   Timer  Set   "};
int index = 0;



void setup()
{
  Serial.begin(9600);
  pinMode (menuBut, INPUT);
  pinMode (upBut, INPUT);
  pinMode (downBut, INPUT);
  // set up the LCD's number of columns and rows: 
  lcd.begin(16, 2);
  // Print a message to the LCD.
  lcd.print("Measuring Temp");
  
}

void loop()
{
  if (digitalRead(menuBut) == HIGH)index++;
  if (index >= 4)index = 0;
  
 
  Serial.println(index);
  
  

  sprintf(value, "      %1d         ", number);
  // set the cursor to column 0, line 1
  // (note: line 1 is the second row, since counting begins with 0):
  lcd.setCursor(0, 0);
  // print the number of seconds since reset:
  lcd.print(topLine[index]);
  lcd.setCursor(0, 1);
  // print the number of seconds since reset:
  lcd.print("5");
  
}

I am trying to make a simple push button (pull down configuration) increment my array to the next slot. I did not bother debouncing it because I plan to do this in hardware when the parts come in.
When I serial print the value of the button I can see the 1 and 0 just fine.
When I serial print the index value it just mimics the value of the button instead of incrementing 0…1…2…3…0…1…etc like I want.
I have tried a few variations. I tried adding a delay.
I tried making a button state value and using that. Same thing.
I tired having a current and last button state but then the value of my var named index never changed off 0.

i know I am just making some kind of newb mistake. Please help. How do I write this?

That loop is running very fast, so the pin will be high for a (large) number of times through the loop.

So it's probably just wrapping round to zero constantly and you only get to see a few of the values.

Try adding a delay in the loop for debugging to slow down the behaviour and see what you get ....

RogerRowland: That loop is running very fast, so the pin will be high for a (large) number of times through the loop.

So it's probably just wrapping round to zero constantly and you only get to see a few of the values.

Try adding a dely in the loop for debugging to slow down the behaviour and see what you get ....

Even better is to look at the state change detection example. Take action only when the switch BECOMES pressed, not is pressed.

How ARE the switches wired? Do you have external resistors? Why? There are internal pullup resistors that can be used, which make wiring the switches so much easier.

You need to increment the index variable when the button becomes pressed, not when it is pressed. Look at the state change detection example in the IDE to see how to do it.

Also consider usingpinMode(menuBut, INPUT_PULLUP);to activate the internal pullup resistor and remove the external pull down resistor if you have one, which I suspect you don’t, and wire the button to pull the pin low when pressed. This will ensure that the input is always at a known voltage and not floating between 0V and 5V. Note that when you do this you will need to invert the logic of the test to see whether the button is pressed.

Thanks for all the comments. I am looking at the example detection while I was waiting for a response. Also....I do have pull down resistors. I did not know that this chip had built in pull up resistors that could be activated. That is cool. Guess that means I just wasted money on a Inverting Schmitt Trigger for hardware debouncing? I already tried the whole current button last button thing. It did not work. I might have messed it up but have used this before successfully in other codes I wrote. I also did try to put a 1 second delay into the loop but it did not change anything either. Let me see what I can do with this example code. That seems to be something that I could just mod. Will get back to you if I still cannot make it work.

Thank You

OK so here is the updated code. I used the example and it did work. So I don’t have a hardware issue. I modified it to go with the code I’m writing. Just name of variable changes etc.

Here is the new code.

const int menuButPin = 7;
const int upBut = 9;
const int downBut = 8;
int menuButState = 0;        
int lastMenuButState = 0;
#include <LiquidCrystal.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
char value[16];
int number = 5;
char* topLine[4]={"  Temperature   ", "   Temp.  Set   ", "  Calibration   ", "   Timer  Set   "};
int index = 0;



void setup()
{
  Serial.begin(9600);
  pinMode (menuButPin, INPUT);
  pinMode (upBut, INPUT);
  pinMode (downBut, INPUT);
  lcd.begin(16, 2);
  lcd.print("Measuring Temp");
  }

void loop()
{
 menuButState = digitalRead(menuButPin);
 if (menuButState != lastMenuButState)
 {
   delay (10);
   if (menuButState == HIGH)
   index++;
   Serial.println(index);
 }
 
 lastMenuButState = menuButState;
    

  sprintf(value, "      %1d         ", number);
   lcd.setCursor(0, 0);
   lcd.print(topLine[index]);
  lcd.setCursor(0, 1);
    lcd.print("5");
  
}

I’m pretty much getting the same thing. When I hold the button down I see 1 in the serial monitor. When I let it go…it goes back to 0. I don;t understand how it is going back. Why isn’t the ++ command leaving it incremented?

Thank You

Modified notes and capitals per request. may have missed one :slight_smile:

Even though you ultimately will be driving the line digitally, you should debounce it when testing with a button - button presses bounce, sometimes 2 or 3 times, sometimes dozens, its hopeless to test things with an unreliable method, just causes confusion.

Jarrod: Can you remove the stupid comments from the code. None of them reflect what the code is actually doing, and just add visual clutter to the program.

Naming the variables so that they appear to be related, like currMenuSwitchState and prevMenuSwitchState, makes for more understandable code, too.

Variables are supposed to be named using camelCase, which means that EVERY word in the name is capitalized, except the first one. Your practice of random capitals for some words in the name makes for unreadable code.

Unconditional printing of index might provide a clue.

Cleaned up the code in the post above for you. It wasn't random it was alternating :) Also...I do not know what this "Unconditional printing of index" means. Will ask Google.

Thank You

I do not know what this "Unconditional printing of index" means. Will ask Google.

Right now, you are conditionally printing index. That is, you print it only if a condition is met - the switch has become pressed.

Move that statement outside of the if block.

I started of that way. Does the same thing just keeps the number running down the page really fast. I tried adding a delay. It delayed but same effect.

It looks like your value array is too small - there is no space for the terminating NULL byte. Your use of sprintf is overflowing.

Sorry I don’t know enough to understand what you mean. In the mean time I moved the print command out of the brackets and added a delay of 100ms. It isn’t doing what I thought before. It is scrolling 0 and when I push the button it turns to a 1 just briefly . Weather I let go of the button or not it changes back to 0.

I found this:

when declaring an array of type char, one more element than your initialization is required, to hold the required null character.

But I don’t understand it:(

Sorry I don’t know enough to understand what you mean.

Here:

char value[16];

you define an array that can hold 16 characters.

Here:

  sprintf(value, "      %1d         ", number);

you write 17 characters into it - 6 spaces, 1 digit, 9 spaces AND the terminating NULL.

Brilliant!!

PaulS:)

I took away one space and it worked. I understand I had 1 too many characters but it seems like your telling me somehow there is a Null object which counts as a character and is not visible? Can you point me to something that explains please? This is obviously important :)

Edit: I read up on it. I sort of understand it. Does that mean if I store char strings in an array I need to make sure the array hase one more char then I actually plan to use for each slot? Would I write char(17) arrayName(4) or can I get away with char arrayName(4) ?

Thank You.

Does that mean if I store char strings in an array I need to make sure the array hase one more char then I actually plan to use for each slot?

Yes.

Would I write char(17) arrayName(4) or can I get away with char arrayName(4) ?

No and no.

When the type is char *, the values are NULL terminated strings, and the compiler takes care to allocate enough space for each string. And, you use [4], not (4), to define an array with 4 elements.

When you are defining an array that you are going to put data in later, that is when you need to be careful to size the array correctly.

Removing one space was not the correct solution. You are writing 16 characters, plus a NULL, into the array so that when you print the array on the LCD, the whole line gets blanked. In order to do that, you need to make the array able to hold 17 characters.

So I should have wrote char[17] arrayName[4]?

So I should have wrote char[17] arrayName[4]?

No. For a 2D array of characters, the [4] and [17] go together:

char arrayName[17][4];

But, for what you want to do, letting the compiler count the characters, and allocate space, and keep track of pointer is preferred.

char *arrayName[4];