rotary encoder and LCD... weird results

Here's what I'm trying to do as part of a larger project. I have an LCD screen and a rotary encoder hooked up to my Arduino. I want to display the position of the encoder as a number and as the encoder turns clockwise the number goes up and when the encoder turns counter clockwise the number goes down. My code, while in my head should work for this, doesn't. I didn't write any of this myself except to add the lcd.print(encoderPos); into the interrupts. The rest of the code comes from the rotary encoder section of the playground and the LCD library examples

 #include <LiquidCrystal.h>
 // encoder setup
 #define encoder0PinA 6
 #define encoder0PinB 7
 int encoderPos = 0;
 // initialize the library with the numbers of the interface pins
 LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

 void setup() 
 {
   pinMode(encoder0PinA, INPUT); 
   pinMode(encoder0PinB, INPUT); 
   // encoder pin on interrupt 0 (pin 2)
   attachInterrupt(0, doEncoderA, CHANGE);
   // encoder pin on interrupt 1 (pin 3)
   attachInterrupt(1, doEncoderB, CHANGE);
   // set up the LCD's number of rows and columns: 
   lcd.begin(16, 2);
   // Print a message to the LCD.
   lcd.print("hello, world!");
 }

 void loop() 
 {
   lcd.clear();
   // Turn off the cursor:
   lcd.noCursor();
   delay(500);
    // Turn on the cursor:
   lcd.cursor();
   delay(500); 
 }

void doEncoderA()
{
  // look for a low-to-high on channel A
  if (digitalRead(encoder0PinA) == HIGH) 
  { 
    // check channel B to see which way encoder is turning
    if (digitalRead(encoder0PinB) == LOW) 
    {  
      encoderPos = encoderPos + 1;      // CW
      lcd.print(encoderPos);
    } 
    else 
    {
      encoderPos = encoderPos - 1;         // CCW
      lcd.print(encoderPos);
    }
  }
  else   // must be a high-to-low edge on channel A                                       
  { 
    // check channel B to see which way encoder is turning  
    if (digitalRead(encoder0PinB) == HIGH) 
    {   
      encoderPos = encoderPos + 1;          // CW
      lcd.print(encoderPos);
    } 
    else 
    {
      encoderPos = encoderPos - 1;          // CCW
      lcd.print(encoderPos);
    }
  }
}

void doEncoderB()
{
  // look for a low-to-high on channel B
  if (digitalRead(encoder0PinB) == HIGH) 
  {   
   // check channel A to see which way encoder is turning
    if (digitalRead(encoder0PinA) == HIGH) 
    {  
      encoderPos = encoderPos + 1;         // CW
      lcd.print(encoderPos);
    } 
    else 
    {
      encoderPos = encoderPos - 1;         // CCW
      lcd.print(encoderPos);
    }
  }
  // Look for a high-to-low on channel B
  else 
  { 
    // check channel B to see which way encoder is turning  
    if (digitalRead(encoder0PinA) == LOW) 
    {   
      encoderPos = encoderPos + 1;          // CW
      lcd.print(encoderPos);
    } 
    else 
    {
      encoderPos = encoderPos - 1;          // CCW
      lcd.print(encoderPos);
    }
  }
}

The lcd.print stuff in the interrupt handler is not a good idea. Interrupt handlers should be quick. Lcd.print is not. Look for the thread this week on using interrupts.

You said

My code, while in my head should work for this, doesn't.

. How, exactly, does it not work?

To utilize attachinterrupts 0 and 1 you must wire the encoder channels to pins 2 and 3, not pins 6 and 7 as you have in your setup section.

PS: And as you are using pins 2 and 3 for the LCD, you have a classic pin conflict. I don't have a standard LCD display (mine is serial) so I don't know if the LCD library can easily be changed for pin usage or not. It is possible to read a encoder without using interrupts, therefore any two avalible free digital pins, however that encoder code you are using would have to be rewritten in a polling manner in the loop section rather then using ISR routines.

Lefty

Ok, I changed the pins for the encoder to 2 and 3 and bumped those pins from the LCD to 6 and 7. I modified the code so that the LCD.Print is in the main loop now. I even changed the encoderPos to a volatile int just in case that's what was messing things up.

As for what was wrong, with the previous set up, it was outputting a string of symbols and meaningless numbers. Now, I turned the encoder one notch clockwise and the count went to -1. Not a big deal, I probably got the pins switched.... and turned it again in the clockwise direction. the count went to 1 and then 24 and then 259.....

Is this a mechanical encoder with switch contacts or an optical encoder? If mechanical there will be need for contact bounce filtering, either externally with components or with software debouncing routines.

Lefty

It's a mechanical encoder. How do you debounce... component or software?

How do you debounce... component or software?

Neither, I gave up on a mechanical encoder and ended up getting a nice deal on some surplus optical encoders. No contact bounce and very smooth operation. They are out of stock, but possibly they will have them again:

http://www.bgmicro.com/index.asp?PageAction=VIEWPROD&ProdID=12916

Lefty

So here's what I did: Off of A and B, a 10k resistor to +5V, a .047 uF cap to GND, and a 10k resistor to the Arduino. In the programming, set interrupt 0 to detect rising and interrupt 1 to detect on falling.

The result: For the most part it increments and decrements by one. Every once in a while it goes by two, but that's acceptible for what I'm doing.

The result: For the most part it increments and decrements by one. Every once in a while it goes by two, but that's acceptible for what I'm doing.

Good for you, glad you got something working. Just so you know in the future, you don't really have to utilize an interrupt pin and attachinterrupt function for both encoder channels A & B. You can an interrupt piin and function for one of them and in the one ISR you read the state of the other pin with a simple digitalRead to determine step direction. As long as you have the pins to spare it's fine to use what you have, but's it's a little overkill.

Lefty