Something not right

When I press BUTTONR on "change colour" it does not switch to "settingsChangeColour();" and run that code. I'm using an arduino mega and BUTTONR is on pin 2 and BUTTON 1 is on pin 3.

Can anyone shed some light on this. If I can get this working I can finish my menu.

// Include LCD Library Code
#include <LiquidCrystal.h>
// Include Rotary Encoder Library
#include <QuadEncoder.h>
#include <Button.h>

// control
#define ROTENCA 43
#define ROTENCB 45
#define BUTTONR 2 // Intr 0 = Pin 2 encoder push button
#define BUTTON 1 // Intr 1 = Pin 3 push button

// initialize the LCD with the numbers of the interface pins
LiquidCrystal lcd(7,8,9,10,11,12);

// initialize the encoder
QuadEncoder encoder(ROTENCA,ROTENCB); // initialize the encoder
// has the encoder moved on this loop?
boolean moved = false;

Button buttonR (BUTTONR);

void setup() {
  // setup interrupts
  attachInterrupt(BUTTON, displayMenu, RISING); //RISING IS ON THE ENCODER
  // set the LCD's number of cols and lines
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(1,0);
  lcd.print("*** WELCOME! ***"); 

  //lcd.print("T");  
}

void displayMenu() {
  int selected = 3;

  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print("Turn the knob to");
  lcd.setCursor(0,1);
  lcd.print("select an option");
  // display options
  while (!buttonR.wasPressed()) {
    selected += readEncoder();// selected = encoder
    if (selected<0) selected=3;
    if (selected>3) selected=0;
    if (moved) {
      lcd.clear();
      lcd.setCursor(0,0);
      switch(selected) {
      case 0:
        lcd.print("Change Colour    ");
        break;
      case 1:
        lcd.print("empyt 1");
        break;
      case 2:
        lcd.print("empty 2");
        break;
      case 3:
        lcd.print("Exit Settings   ");
        break;
      }
    }
  }// moving this to the bottom makes menuseltion part work 

    // when button is pressed...
  lcd.clear();
  switch(selected) {
  case 0:
    settingsChangeColour();
    break;
  case 1:
    // to be added
    break;
  case 2:
    // to be added
    break;
  }
}

void settingsChangeColour() {
  int selected = 2;
  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print("Select Colour");
  lcd.setCursor(0,1);
  lcd.print("Currently it's ");
  //lcd.write(temperatureUnit);
  while (!buttonR.wasPressed()) {
    selected += readEncoder();
    if (selected<0) selected=2;
    if (selected>2) selected=0;
    if (moved) {
      lcd.setCursor(0,1);
      switch(selected) {
      case 0:
        lcd.print("Red");
        break;
      case 1:
        lcd.print("Yellow");
        break;
      case 2:
        lcd.print("Cancel");
        break;
      }
    }

    // when button is pressed...
    switch(selected) {
    case 0:
      lcd.print ("yes, it's red");
      break;
    case 1:
      lcd.print ("yes, it's yellow");
      break;
    }
  }
}     

// returns 1 for right, -1 for left, or 0 for no movement
int readEncoder() {
  moved = true;
  switch(encoder.hb()) {
  case '>': 
    return 1;
  case '<': 
    return -1;
  }
  moved = false;
  return 0;
}

How are you hooking up your hardware? Need pictures and descriptions. What if you swap button definitions to use the BUTTONR pin for a working portion of your code?

Thanks for replying. I have added a photo of the hardware setup picture below. I also swapped the BUTTONR and BUTTON around but I get the same result.

additional link

Imgur

Can't see the attachment, unfortunately - most likely the forum software having another bad day. Perhaps you could post it on a public file sharing site and post a link instead?

If possible, a drawing of the circuit might be clearer than a photograph.

Additional link added 8)

http://www.flickr.com/photos/96527043@N02/

There's too much going on there to see what the problem is.

I don't see any pull-up or pull-down resistors in the external circuit and the sketch doesn't seem to enable the internal ones, so you probably have a floating input when the switch is not being pressed. This could result in spurious button presses being detected.

I haven't used the Button library you're using. Does it provide some way for you to know that the button has been released after being pressed, so that you know you're seeing two distinct button presses? If not, it would fly straight through to read the encoder without allowing any time for you to move it.

I'm not familiar with that encoder library either. There seems to be code to indicate whether the encoder has been moved (presumably since last time it was read) but I can't see any way for movement of the encoder to actually update 'moved', so I don't get how that works. Does it actually give you the encoder movement events you're expecting when you expect them?

It would be useful to add diagnostic code using Serial print to tell you what events your sketch is detecting.

I don't see any pull-up or pull-down resistors in the external circuit

They are their, if you click the image it will expand and you can see them.

Does it provide some way for you to know that the button has been released after being pressed

yes. that is done in the button library. ".waspressed" ".isPressed".

Does it actually give you the encoder movement events you're expecting when you expect them?

yes it does. The encoder navigates the menu ok but when the lcd displays change colour and I press BUTTONR it should call "void settingsChangeColour()" but nothing happens. I can still rotate the encoder through the menu "empty 1", "empty 2" etc.

Put a print statement at the point where you expect settingsChangeColour() to be called, so you can tell whether it's actually being called.

If it isn't, put a print statement at the point where the decision was made not to execute the code path that calls settingsChangeColour(), so you can see why the decision was made.

The buttonr looks different from the rest of the buttons. Can you use same buttons for all of them?

Almost the ENTIRE sketch is inside the interrupt service routine. :astonished: Interrupts are for extreemly time critical tasks and have to be kept short else it disturbs all other interrupts (like millis() driving servos etc). :fearful: They are not a way to avoid writing a proper loop().... :roll_eyes:

Msquare:
Almost the ENTIRE sketch is inside the interrupt service routine. :astonished: Interrupts are for extreemly time critical tasks and have to be kept short else it disturbs all other interrupts (like millis() driving servos etc). :fearful: They are not a way to avoid writing a proper loop().... :roll_eyes:

I never was keen on interrupts, the code I was using was lifted from someone Else's sketch, as I could not get my Original code to work. in hind sight I think Help is better focused on my Original problem which is below. I think this is easier to solve and any insight into this would be appreciated. I have awarded karma to all of you for you help so far.

From power up the LCD shows Time and Date

void loop()
{
  if (menuRoot == true)
  {
    lcd.setCursor(4, 0);
    lcd.print(rtc.formatTime());
    lcd.setCursor(3, 1);
    lcd.print(rtc.formatDate());

I then navigate the menus using the encoder

void readEncoder() //read the encoder and set lastButtonPushed to values used by navigateMenu()
{
  n = digitalRead(encoder0PinA);// n is current state of encoder0PinA pin
  if ((encoder0PinALast == HIGH) && (n == LOW)) // check if encoder0PinA pin has changed state
  {
    if (digitalRead(encoder0PinB) == LOW) 
    {
      lastButtonPushed = 1; //if it has changed and its now low decrement  encoder0Pos;
    } 
    else 
    {
      lastButtonPushed = 2; // if it has changed and its now high, increment encoder0Pos
    }
  } 
  encoder0PinALast = n; // set the variable holding the previous state to the value n read above
}

Then using case 1; or case 2 of the void navigateMenus, I move Left/Right through the menu till I get to for example "Min" from void menuChanged.

void menuChanged(MenuChangeEvent changed){

  MenuItem newMenuItem=changed.to; //get the destination menu

  lcd.setCursor(0,0); //set the start position for lcd printing to the second row

  if(newMenuItem.getName()==menu.getRoot()){
    lcd.clear();
    menuRoot = true;    
  }
  //MENU SET CLOCK
  else if(newMenuItem.getName()=="menuSetClock"){
    menuRoot = false;
    lcd.clear();
    lcd.print("Set Clock");
  }
  else if(newMenuItem.getName()=="menuItemHr"){
    lcd.clear();
    lcd.setCursor(0,1);
    lcd.print("Hr");
  }
  else if(newMenuItem.getName()=="menuItemMin"){
    lcd.clear();
    lcd.setCursor(0,1);
    lcd.print("Min");
  }
}

Now that I have "Min" displayed on the LCD. I select it by pressing the encoder push button on pin 45.

void menuUsed(MenuUseEvent used){ //                  **if you have a function call it here**
  if ( used.item == menuItemHr ){
    lcd.clear();//etc.
    lcd.setCursor(0, 0);
    lcd.print(rtc.formatTime());
    changeHour();
  }
  else if ( used.item == menuItemMin ){
    lcd.clear();//etc.
    lcd.setCursor(0, 0);
    lcd.print(rtc.formatTime());
    changeMin();
  }

  if ( used.item == menuExit ){
    menu.toRoot();//etc.
  }
}

This now shows the LCD displaying the clock. because of the function changeMin() called from void menuUsed above

void changeMin(){

  //min=rtc.getMinute();
  if(digitalRead(encoderPushButton) == HIGH)
  {
    min = ++min % 60; //  this is better than // min += 1;as it rolls overfrom 59 to 00
    //if (incrementMinuteButtonPressDectected) min = ++min % 60 // increment and rollover past 59
    //if (decrementMinuteButtonPressDectected) min = (--min + 60) % 60 // decrement and rollunder past 0 
  }
  rtc.setTime(hour, min,sec);
}

At this point If I use the encoder case 1 or case 2, it does not change the Mins on the clock. It simply navigates the menu left or right.

but if I press the encoder push button, the minutes change... I would like the enocder to change Min up/down instead of the encoder push button.