quick help -- button pressing and timing

Hi all. I am new to arduino, and thought that for my first project, I would tackle the ever-popular camera intervalometer.

I have a decent understanding of the circuit I would like to use, aside from how I will minimize the number of necessary input ports on the arduino. I have an idea as to how things will work, but I would appreciate a little guidance on how I handle button presses.

UPDATE: See 3rd reply for slight update

I have 4 input buttons (Left, Right, Up, Down) and they will cause my lcd to refresh and increase my various values, etc. My initial idea was something like:

void loop(){

unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval) {
      previousMillis = currentMillis;   
      stateR = digitalRead(btnR);
      stateL = digitalRead(btnL);
      stateU = digitalRead(btnU);
      stateD = digitalRead(btnD);

      if (stateR==HIGH){do something;}
      if (stateL==HIGH){do something;}
      if (stateU==HIGH){do something;}
      if (stateD==HIGH){do something;}
  }
}

I am not sure if this is the best way to do this as my code seems to miss button presses and get stuck somewhere. Does anyone have any advice they can offer? Is my method right and my code just buggy?

im not exactly sure what it is you are trying to do but i want to give you some tips nonetheless

     stateR = digitalRead(btnR);
      stateL = digitalRead(btnL);
      stateU = digitalRead(btnU);
      stateD = digitalRead(btnD);

      if (stateR==HIGH){do something;}
      if (stateL==HIGH){do something;}
      if (stateU==HIGH){do something;}
      if (stateD==HIGH){do something;}
// is a bit double so my advice is to skip the middle boolean
// allso when using a if in comination with a boolean you dont need the ==HIGH
// so a sample line will be

if(digitalRead(BTN_x)){doobiedo;}

// we can even clean this up some more because when there is not more then 1 action after a statement there is no need for { }
if(digitalRead(BTN_x))doobiedo;

obviousely this is not at all a awnser to youre question, but i am not entirly sure what the question is
will give you a very important tip
in your code you use ""if (stateR==HIGH)"" this will cause some serious bugs because when you press the button the action after the if statement will be executed a couple of times so ther will be far more the one meassurement done. try researching how to prgram a switch to execute a action on the uprising or falling flank. if you dont understand this i will explain it a little better

Take a look here to find out how you can connect 5 pushbuttons using only a single analog input.
the timing problems are also solved

http://www.dfrobot.com/wiki/index.php?title=Arduino_LCD_KeyPad_Shield_(SKU:_DFR0009)

I apologize for my messy question, I was excited and rushed to get help. Let me try and clarify myself.

I need 4 buttons to control my circuit/device. I have updated my initial code with the recommended button reading that daatse recommended. The new rough code outline is as below:

void loop(){
//check for button press
    if (digitalRead(btnR)){/*change my menu, update screen*/}
    if (digitalRead(btnL)){/*change my menu, update screen*/}
    if (digitalRead(btnU)){/*change my variable, update screen*/}
    if (digitalRead(btnD)){/*change my variable, update screen*/}
    
    if (/*variables are XX*/){/*run loop for 5 iterations*/}
}

Does this make sense? I will look more into the "uprising flank" mentioned.

Thanks luxxtek, I was aware of such setups and will be sure to check that one out.

scales11:
Does this make sense? I will look more into the "uprising flank" mentioned.

You may have an issue finding information with that term, as I've never seen it called that before. The more commonly used term would be signal edge detection. Basically, you want the instant that the signal changes from HIGH to LOW (Falling edge) or LOW to HIGH (Rising Edge). There are plenty of libraries dealing with this for buttons, but it's pretty simple to implement in Arduino.

You simply need to keep track of the last reading of the button, and compare it with the current reading. If they are different, then a signal edge was detected. Here is a quick example:

void setup() {
  // stuff
}

void loop() {
  static int lastReading = LOW;
  int currentReading = digitalRead(btnPin);

  if (lastReading != currentReading ) {
    // Any signal edge
  }

  if ((lastReading==HIGH)&&(currentReading==LOW)) {
    // Falling (negative) edge
  }

  if ((lastReading==LOW)&&(currentReading==HIGH)) {
    // Rising (positive) edge
  }
}

I did understand what he meant, I dealt with it a little when I coded in labview. But you are correct, and I did not know the proper term. I was thinking more along the lines of the button, ie. latched, etc.

Thanks, I may need to add that functionality into my code when it progresses.

dear scales,

---explanation why to use the rising edge/uprising flank---
when you use tact switches (switches where contact is made when pushed and lost when released) or any kind of singlepole toggle switch there is a problem you should watch out for. because you hold the switch at very least for 40-100 ms(random guess) and your program's looptime is much less. Your program will register the button you pushed only once, as if it was pushed many times in a row. this will result in that the action you wanted to link to the button will be executed many times. one of the solutions is to program your arduino so that the button will only be registerd as pushed when the in previous loop it wasn't pushed. thus when the INPUT rises from '0' to '1'. <--thats the rising flank for ya

Also i'm still not quite sure what the question is you were asking?!

and ofcourse a litle tip to close the deal:
i think this is much more logical (and probably faster) it also removes a neatness bug that when 2 opposite buttons are pushed the screen goes back and forth rapidly

void loop(){
//check for button press
    if (digitalRead(btnR)){/*change my menu*/}
    else if (digitalRead(btnL)){/*change my menu*/}
    else if (digitalRead(btnU)){/*change my variable*/}
    else if (digitalRead(btnD)){/*change my variable*/}
    update screen;

    if (/*variables are XX*/){/*run loop for 5 iterations*/}
}

sidenote: Arrch is right about the correct names, my excuses (i'm dutch and i used a litteral translation)

Thanks Daatse,
I was able to get some response from my buttons with my initial code, though I did have the problem of dealing with one button press being registered as many as you described. My question was "Am I dealing with button presses in a proper and efficient way". It has now become clear that I was not.

I think you are making a good point bringing up the latched issue (you call it flanking). I think I need to incorporate that into my code. Shall I look for a library, or function to use? Perhaps I will make one. I do not know if this will change things, but I may want to use a rotary encoder rather than buttons in my final design, will that still present the latching/flanking issue?

Also, is it a problem to refresh the screen that many times? At the moment I am using the 16x2 LCD, but I may move to a few alphanumeric segmented displays.

I think you can pull it of by yourself
the code that Arrch provided will do the trick but when using more then one button u must use array's to keep everything neat

#define NOB 4 // number of buttons

boolean currentReading;
boolean status[NOB];

void loop() {
for(int i = 1; i++;  i<NOB;){

int currentReading = digitalRead(i); // only use this when using pin 1 to 4 when using for example pin9 to 13 use digitalRead((i+9));
if ((status[i]==LOW)&&(currentReading==HIGH)) {
//the behavior that that switch schould provoke
}
status[i] == currentReading;
}
}

something like the above, the above code is only to help you it will probably not work

also that much refreshes are not really a problem but when holding 2 buttons it will rapidly show oneframe and then a other (much like nowadays videoclips)

You can also use an array of function pointers to keep the code down as well. For example:

void btn1Action() { // stuff }
void btn2Action() { // stuff }
void btn3Action() { // stuff }
void btn4Action() { // stuff }

void (*btnActions)[4]() = { btn1Action, btn2Action, btn3Action, btn4Action };

This would allow a single for loop to check for the edge, run the function if it's found, and update the last state variable.

Clever, daatse and Arrch.

I will definitely use more arrays in my code now. I was previously defining my menu by leaving the titles in a string array (ie. {"exposure","interval","iterations"}) and then stepping through the array when a keypress was registered. The ideas you both mentioned will be a nice addition.

Thanks, I think I have enough to go test out some different algorithms. I will report back if i hit any problems.

Well I made some progress. I do have a loop but I am just going to show one button press to keep it simple. I am using the following code, but it still seems "laggy" or non-responsive with some presses. Many work fine but far from all. Does anyone have an opinion or explanation?

Thanks!

Here is my current code:

boolean status=0;

void setup(){
Serial.begin(9600);
}

void loop(){
        if (digitalRead(5)==0 && status == 1){
                  Serial.println(5);
        }
        
    status=digitalRead(5);
}

While it may not make a major difference in terms of functionality, it's good form to use int's and the HIGH, LOW constants when dealing with digitalRead and digitalWrite.

You shouldn't be reading you pin again because the value could change in between, you should be reading it into a variable, and using that as the comparison and assignment value.

Depending on the button, you may need to implement a debounce.

youre coding now is aimed for the falling edge witch means it wil react when the button is released (if this is not correct toggle al the TRUE/FALSE except "boolean status = FALSE;"

also in the println function youre trying to print a variable called 5(wich does not exist) in stead of the number 5 so use "5" in stead of just 5

give this baby a spin:

boolean status = FALSE;

void setup(){
Serial.begin(9600);
}

void loop(){
        if (digitalRead(5) == FALSE && status == TRUE){
                  Serial.println("5");
                  status = FALSE;
        }
        if (digitalRead(5) == TRUE){
                  status = TRUE;
        }

}

Daatse,

Thanks for this. It works, but I am noticing lots of repeating...Any tips?

i am not quite sure what the problem is but it may just be that something called 'contact bounce' is at work here
please look this up

i made a small tweak to the code
try this code:

boolean status;

void setup(){
Serial.begin(9600);
Status == digitalRead(5)
}
void loop(){
        if (digitalRead(5) == FALSE && status == TRUE){
                  Serial.println("5");
                  status = FALSE;
        }
        if (digitalRead(5) == TRUE){
                  status = TRUE;
        }
delay(20);
}

if contact bounce is the issue the code above should do the trick, also experiment with the delay time to find the minimum value for application because else it would slow down the whole loop

Yes I think that delay could fix it, but I was trying not to use delays so that I wouldn't miss key presses. Do you think using some of the "Blink without Delay" code would fix it?

Also, while I am currently using a button, I would like to switch to a rotary encoder. I am not sure what that will do to my code.

in my prev sketch Status == digitalRead(5) --> Status = digitalRead(5)

yes the blink without delay example will do fine just implement it in the right way
sketch:

#define contactbounce 20

boolean status;
unsigned int time;

void setup(){
Serial.begin(9600);
Status = digitalRead(5)
time =millis()
}
void loop(){
if (millis()+contactbounce>= time){
        if (digitalRead(5) == FALSE && status == TRUE){
                  Serial.println("5");
                  status = FALSE;
        }
        if (digitalRead(5) == TRUE){
                  status = TRUE;
        }
time = millis()
}
}

i never worked with rotaty encoders myself but ill bet there is some code for them somewhere on the arduino forum

some links:

http://arduino.cc/playground/Main/RotaryEncoders

unsigned int time;

void setup(){
Serial.begin(9600);
Status = digitalRead(5)
time =millis()

Aside from the milling semicolons, this code is a disaster waiting to happen.

millis() returns an unsigned long, not an unsigned int. Storing the value in an unsigned int will result in mangled data.