Rotary encoder with PIC16F887 troubles

[newtekuser]

Hello all and Happy New Year!

I'm starting the first day of the year with some new experiments trying to make a rotary encoder blink some LEDs depending on rotation using the PIC16F887. The trouble I have is I cannot get them to light up properly. If I remove the 100nF caps attached to pins A and B of the encoder, the LEDs blink erratically, even if the shaft is not rotated. If I leave them on, the LEDs don't toggle when I rotate the shaft. (I also tried with lower cap values, the LEDs blink but not correctly)
I know the encoder works properly - if I remove the PIC and just connect the encoder terminals to the LEDs and 300ohm resistors, then they turn off based on the rotation of the shaft (LED to A turns off when rotated CCW, LED to B turns off when rotated CW).

I think my problem is in the code logic.

As seen from the code, each LED is connected to RD1 and RD2 of the PIC while terminal A of the encoder is connected to RD0 and terminal B to port RD3.

Anything that stands out to you?

Code: [Select]

// CONFIG1
#pragma config FOSC = INTRC_NOCLKOUT// Oscillator Selection bits (INTOSCIO oscillator: I/O function on RA6/OSC2/CLKOUT pin, I/O function on RA7/OSC1/CLKIN)
#pragma config WDTE = OFF       // Watchdog Timer Enable bit (WDT disabled and can be enabled by SWDTEN bit of the WDTCON register)
#pragma config PWRTE = OFF      // Power-up Timer Enable bit (PWRT disabled)
#pragma config MCLRE = OFF      // RE3/MCLR pin function select bit (RE3/MCLR pin function is digital input, MCLR internally tied to VDD)
#pragma config CP = OFF         // Code Protection bit (Program memory code protection is disabled)
#pragma config CPD = OFF        // Data Code Protection bit (Data memory code protection is disabled)
#pragma config BOREN = OFF      // Brown Out Reset Selection bits (BOR disabled)
#pragma config IESO = ON        // Internal External Switchover bit (Internal/External Switchover mode is enabled)
#pragma config FCMEN = ON       // Fail-Safe Clock Monitor Enabled bit (Fail-Safe Clock Monitor is enabled)
#pragma config LVP = OFF        // Low Voltage Programming Enable bit (RB3 pin has digital I/O, HV on MCLR must be used for programming)

// CONFIG2
#pragma config BOR4V = BOR40V   // Brown-out Reset Selection bit (Brown-out Reset set to 4.0V)
#pragma config WRT = OFF        // Flash Program Memory Self Write Enable bits (Write protection off)

#include <xc.h>
#define _XTAL_FREQ 8000000
#include <stdio.h>            // for sprintf

#define Encoder_CLK PORTDbits.RD0
#define Encoder_DT PORTDbits.RD3

int position;

void main(void)
{
    OSCCON =  0X70;    // set internal oscillator to 8MHz
    TRISD0 = 1; // Encoder CLK
    TRISD1 = 0;
    TRISD2 = 0;
    TRISD3 = 1; // Encoder DATA
   
    position = Encoder_CLK;

    while (1)
    {       

       if (Encoder_CLK != position){
            if (Encoder_DT != position){    // CW rotation
                RD2 = 1;
                RD1 = 0;
            }
            else{   // CCW rotation
                RD1 = 1;
                RD2 = 0;
            }           
        }
        position = Encoder_CLK;         

    }
}
// End of code.

[Ian.M]

Two things are against you:

• The Read-Modify Write effect, which can corrupt your outputs.  See RMW and solutions for it on the Microchip forums.
• Mechanical encoders typically have *LOTS* of contact bounce.

Due to the contact bounce, using both edges of one channel to clock latching the direction from the other channel generally gives poor results, with the direction output toggling on the spurious bounce edges.  While it is possible to implement effective debouncing in hardware, it isn't as simple as just swamping it with a capacitor across the contacts, which if too large can damage the encoder.  See Debouncing Contacts and Switches by Jack Ganssle. 

Its generally preferable to use a state machine for the quadrature decoding, which even with severe contact bounce will only jitter by one edge step, so simply delay updating the direction outputs till its 'seen' two steps in the same direction. I posted C code and a hardware version of the state machine here: https://www.eevblog.com/forum/projects/quadrature-rotary-encoder/

[TomS_]

Do the outputs from the encoder need any kind of pull up/down resistors? What is the signal state when the encoder is stationary? Is it a complete circuit or floating? If the LEDs go crazy without the caps it sounds like you could have floating inputs on the PIC which are interpreted as rapid changes in encoder state.

[newtekuser]

Two things are against you:

• The Read-Modify Write effect, which can corrupt your outputs.  See RMW and solutions for it on the Microchip forums.
• Mechanical encoders typically have *LOTS* of contact bounce.

Due to the contact bounce, using both edges of one channel to clock latching the direction from the other channel generally gives poor results, with the direction output toggling on the spurious bounce edges.  While it is possible to implement effective debouncing in hardware, it isn't as simple as just swamping it with a capacitor across the contacts, which if too large can damage the encoder.  See Debouncing Contacts and Switches by Jack Ganssle. 

Its generally preferable to use a state machine for the quadrature decoding, which even with severe contact bounce will only jitter by one edge step, so simply delay updating the direction outputs till its 'seen' two steps in the same direction. I posted C code and a hardware version of the state machine here: https://www.eevblog.com/forum/projects/quadrature-rotary-encoder/

I did read up on the bouncing issue with rotary encoders and it makes sense after reading it. However, this article shows it working w/o anything special done to it: https://circuitdigest.com/microcontroller-projects/pic16f877a-rotary-encoder-interfacing
The only difference I see is use of slightly older PIC mcu and external crystal. How did that person get it to work? Maybe there's some other magic involved that wasn't documented?

[newtekuser]

Do the outputs from the encoder need any kind of pull up/down resistors? What is the signal state when the encoder is stationary? Is it a complete circuit or floating? If the LEDs go crazy without the caps it sounds like you could have floating inputs on the PIC which are interpreted as rapid changes in encoder state.

Unfortunately the rotary encoder was purchased off Amazon and no datasheet was provided. However, one of the reviewers posted a diagram showing two 100nF caps attached to each channel which is how I'm using it. Do these normally also need pull up/down resistors?
https://www.amazon.com/gp/product/B07D3DF8TK/ref=ppx_yo_dt_b_search_asin_title?ie=UTF8&psc=1

I did connect all unused pins on the PIC to GND and while the erratic behavior w/o caps is largely gone (LEDs still toggle randomly but less frequent), when caps are attached, rotating the shaft does nothing.

LE: I did see a review just now that mentions the use of 10K pull-up resistor on each channel. I may have to give that a try.

[DrGeoff]

When using rotary encoders I've always found using edge-triggered interrupts from the encoder to be the better method.

[cv007]

You can use portb pins which have the ability to enable internal pullups. The port b pins also allow interrupt on change. These pins may need ansel cleared as the ansel bits are set by default for the ones that can do analog.

[MikeK]

However, one of the reviewers posted a diagram showing two 100nF caps attached to each channel which is how I'm using it. Do these normally also need pull up/down resistors?

Digital inputs require pullup resistors.  As mentioned, this may be done via the micro's internal weak pullups if that PIC has them.  Floating the inputs mean they will have an undefined state, neither high (5V) nor low (0V)...so of course the behavior will be wacky.  10K pullups are typical, don't require any software, and are faster than internal pullups (which are actually a current source).

[Ian.M]

Its a clone of an Alps EC11 series encoder: https://tech.alpsalpine.com/assets/products/catalog/ec11.en.pdf
Expect reduced life, lower max. rotation speed and increased contact bounce and noise compared to the original.

It definately needs pullups.  Alps suggests 5K resistors as pullups in their test circuit and also specifies the *MINIMUM* current is 500uA @5V DC, so the PIC's internal pullups are too high resistance and 10K pullups would be marginal.  This would be to provide adequate wetting current to break through any film of oxides and contaminants that may build up over time.   Any resistor value near 5K would be suitable, e.g 4K7 or 5K6.

As far as the circuit digest project goes, the encoder can clearly be seen to be mounted on a breakout board, which includes 10K pullup resistors: https://www.rcscomponents.kiev.ua/datasheets/ky-040-datasheet.pdf
The problems with the simplistic edge triggered code typically only become apparent as the encoder contacts start to wear, though if you are unlucky and get a particularly low quality one, it may need small value capacitors (e.g. 10nF) from A and B to ground to work at all.

[MikeK]

And worth pointing out, in case the OP doesn't know...larger value pullups are weaker (slower) and smaller value pullups are stronger (faster).

[themadhippy]

After spending far to long trying to get,  all be it an arduino ,to work reliable with rotary encoders either within software or resistor capacitor networks i gave up and went old skool and added  a schmitt trigger or 6 ,problem solved

[newtekuser]

Thank you all for the suggestions!

I did wire 10K pull-up resistors to CLK and DATA pins as well as 10nF caps on each and got it almost to work to my liking. At first when the PIC is powered up, rotating the shaft slowly in each direction toggles the LEDs correctly, but then it only works if I rotate the shaft faster (i.e.: 2 detents instead of just one), otherwise it skips steps. For the purpose of my experiment this works well enough.

I'll have to look into the schmitt trigger option at some point.

[Ian.M]

Try the state machine code - it shouldn't take much to convert it to XC8.
You need to:

• Put the state machine in your main loop (or in a  function and call it from your main loop).
  
• Supply pin names or #defined pin names in place of A and B in the if() statements.
  
• Declare vector, count and an additional variable old_count as uint8_t globals.
  
• After the state machine has run, add code to evaluate (int8_t)(count-old_count), and based on the result, set your LEDs.  i.e. if its >=2 the encoder is moving in one direction, and if<=-2, the other.  If (and only if) either movement is detected, update old_count from count to 'rearm' the detection for the next movement.

In real life, you *probably* wouldn't choose to poll the encoder in the main loop as it may not poll fast enough due to other stuff in the loop so may miss edges.  Alps says the EC11M is 15 pulses per rev, so on  A and B combined it will deliver 60 edges per rev.  To guarantee it doesn't miss an edge, the code needs to poll faster than the edge rate. A polling rate of 1KHz (off a timer tick interrupt) would be reliable up to around 500 RPM which is significantly faster (over 8 revs/sec) than anyone is likely to spin a manual knob.  Between 500 and 1000 RPM it *may* keep up depending on the amount of contact bounce.  Above 1000 RPM it will fail miserably, but you've probably toasted the encoder due to friction at such a high speed!

As you can see from the above, interfacing high line count optical shaft encoders is a nastier problem, as they may have a line count orders of magnitude higher than these simple mechanical knob encoders, and the shaft speeds may be thousands or even tens of thousands of RPM, so the required polling rate to run the state machine can be several orders of magnitude higher.  For that, you'd either need a much faster MCU or hardware assistance - some PICs have the state machine implemented in hardware, called a QEI module, capable of polling at a significant fraction of the system clock speed and accumulating the resulting up/down count till you get around to dealing with it.

[cv007]

This is a loose translation and simplification of what I have done with other mcu's, minus the interrupt usage. I had a 16F1619 in a curiosity board and an encoder laying nearby, so I created something to test. The idea is that you detect any change on one pin and record the state of the other at that point. When both recorded states are 0 an increment/decrement took place.

Normally interrupts are doing the work in the background- one pins edge interrupts (both) are enabled, the isr takes care of the rest of the details as below and enables the other pin edge interrupts. You use the 'bouncy' pin to get you to the interrupt where the stable pin can be read, and that stable pin now has its edge interrupt enabled.

Seems to work pretty good for me anyway (human interaction), plus its very simple code.

Code: [Select]

//16F1619
#include <xc.h>
#include <stdbool.h>
#include <stdint.h>

//4 leds on curiosity low pin count dev board
//led D4=A5, D5=A1, D6=A2, D7=C5
//led chaser moved by encoder, encoder also determines direction
static void ledSet(uint8_t v){
    TRISA5 = 0, TRISA1 = 0, TRISA2 = 0, TRISC5 = 0;
    //v is bit position 0-3
    v = (uint8_t)(1<<(v&3)); //compiler complains, so cast
    LATA5 = v & 1 ? 1 : 0; //compiler does not these bits as bool and there is no implicit conversion, so need 1/0
    LATA1 = v & 2 ? 1 : 0;
    LATA2 = v & 4 ? 1 : 0;
    LATC5 = v & 8 ? 1 : 0;
}

int main(){
    OSCCONbits.IRCF = 0b1101; //default is 500khz MF, set to 4MHz HF

    //RC1=encoderA, RC2=encoderB
    ANSC1 = 0, ANSC2 = 0; //analog input off, digital on, already set to input out of reset
    nWPUEN = 0; //global enable weak pullups
    WPUC1 = 1, WPUC2 = 1; //enable pullups of AB pins

    ledSet( 0 ); //init leds
    bool a = 1, b = 1;
    uint8_t count = 0; //value for leds (0-3, other bits ignored)

    while(1){
        while( a == RC1 ){} //wait for A to change
        b = RC2; //get B state
        if( a==0 && b==0 ){ ledSet( ++count ); }
        while( b == RC2 ){} //wait for B to change
        a = RC1; //get A state
        if( a==0 && b==0 ){ ledSet( --count ); }
    }
}

[Ian.M]

That's another algorithm and a fairly good one, as in its interrupt driven form, provided you disable the interrupt on the pin that's just triggered, and re-enable when the other pin next triggers (and visa-versa) it cant cause an interrupt storm due to angular shaft vibration and/or contact bounce.

[ppTRN]

The solution i found to be the most effective while dealing with encoders was to choose a phase and use it as a source of interrupt. So when an interrupt on change is detected, the mcu check for the other phase, and based on his value it determine if it is moving CW or CCW. I found MANDATORY to filter both phases with capacitors in range 5 - 20 nF. Bigger capacity values won't work, and whidouth caps it is a mess.

Here is the code i used on arduino UNO, that check for an encoder movement or pushbutton pressing using sw debaunce:

Code: [Select]

void encoderPress(){                          //PRESSING BUTTON ISR               
  if(!TimerBuisy){                            //if TIM2 is not buisy
    TimerBuisy = 1;                           //set it buisy
    btnDebounce = 1;                          //flag to signal the usage for the button and not for the encoder
    TCCR2B |= 1<<2 | 1<<1 | 1<<0;     //turn on TIM2 for 16ms ovfl time
  }
}

void encoderMovement(){
  if(!(PINB & 1))
    rotation++;
  else rotation--;
}

ISR(TIMER2_OVF_vect){
  TCCR2B = 0;               //timer stopped
  if(btnDebounce){                      //was it to debounce the btn or the encoder?
      btnDebounce = 0;                  //clear flag
      TimerBuisy = 0;                   //timer no more buisy
      if(!(PIND & 1<<3))                //btn still pressed?
        press = 1;                      //if so, than it's positive, BTN PRESSED!
  }
}


[newtekuser]

I ended up using a TI CD74AC14E Schmitt trigger to fix this. However, it only worked perfectly after adding an additional 100ms delay before toggling the LEDs otherwise it would still skip once or twice.

Screenshots showing the signal before and after the Schmitt trigger.

R1, R2 = 10K
R3, R4 = 1K
C1, C2 = 10nF

[paulca]

Not sure if it helps here, I only scan read.

The encoder has 2 contacts.  They can bounce all over the place.  If you draw a truth table for the transitions, only 2 are valid, but as they are transitions you need the previous state.

So store the previous pin states and in each interrupt shift them left 2 and add the current states.
EG:

void InteruptHandlerForBothPins() {
    state = state <<2;
    state &= (pinA<<1);
    state &= (pinB);

So you have 4 bits showing the previous state and the current state.

Then with the encoder datasheet, work out which 2 transitions are valid and they equate to an integer each.

So, it's then just something like:

if( ( state & 0xF ) == 7 ) {
   direction = ENCODER_LEFT;

} else if (( state & 0xF ) == 14 ) {
  direction = ENCODER_RIGHT;
} else {
  direction = ENCODER_NULL;
}


I tested this and it doesn't matter how fast or harsh you spin the encoder it doesn't skip, jump back or miss behave.

I found this by copying someone elses supposedly "model implementation" in arduino land.  However as I have seen many times before the approach they took was similar to the above, but for some bizarre reason they went ahead and implmented the WHOLE state machine, including all the bounce and invalid states.  I looked at it pulling an increasing ugly face and exclaimed, "Why would you do that?" and removed almost all of his code and put just the two VALID transitions.  Code was half the size and 10 times neater.

It pays when working on fiddly bits of low level code to keep stepping back and looking at the big picture.  It's not uncommon for a software engineer when asked to open a file to go off and open read and decode the file and then be told.... "em.... we just needed the file size, we didn't need to you to read and decode the image, delete that code."

[Ian.M]

I linked to the 4 bit state machine version back in reply #2, but thanks for independently suggesting it, providing a simplified form, and testifying to its robustness.

Why would you implement the bounce/invalid states?  Well one good reason would be to detect excessive contact wear - if the ratio of invalid to valid events increases too much, alert the user that service is required, *before* the encoder becomes unusable.

Also the switch() ... case  implementation of the algorithm is preferable when you want to increment/decrement on two or four edges per cycle for a higher resolution, as the if() ... else if() version gets fugly.

[Peabody]

I found this by copying someone elses supposedly "model implementation" in arduino land.  However as I have seen many times before the approach they took was similar to the above, but for some bizarre reason they went ahead and implmented the WHOLE state machine, including all the bounce and invalid states.  I looked at it pulling an increasing ugly face and exclaimed, "Why would you do that?" and removed almost all of his code and put just the two VALID transitions.  Code was half the size and 10 times neater.

The standard state machine versions I've seen use the 4-bit previous/current value as an index into a 16-byte lookup table, with each value in the table being 1, -1, or 0.  Zero being no change or an invalid change.  I don't quite understand whether your simplified version is different from that.  Could you post an Arduino sketch demonstrating your version if you have one?

It seems that in any case you will still get an interrupt on each transition of either line, so all the noise transitions will generate interrupts that have to be serviced even if there is no change to the count.

Also, if you have an invalid transition, do you shift left the current state into the previous state position next time, even though you know it's an invalid state, or do you completely ignore the invalid transition and retain the most recent state that you know is valid?  Does it make any difference which way you do it?

Also, when you read the current state of the lines following an interrupt, you could get a false value on the interrupting pin because of noise.

It seems to me that the solution to all this ought to be enabling interrupts on only one line at a time, so that when a line interrupts, in the ISR you disable further interrupts on that line, and enable interrupts on the other line, which should now be stable.  And since you know which line is interrupting, and which transition it must have made to generate the interrupt, you don't actually have to read its current value.  I wrote such a version, and while it generated far fewer interrupts, it produced more errors than the standard lookup table method.  I think that's because sometimes the other line, which should be stable, isn't.

[paulca]

The standard state machine versions I've seen use the 4-bit previous/current value as an index into a 16-byte lookup table, with each value in the table being 1, -1, or 0.  Zero being no change or an invalid change.  I don't quite understand whether your simplified version is different from that.  Could you post an Arduino sketch demonstrating your version if you have one?

Exactly and what use are all those invalids in the table, why bother with a look up, only 2 of them are valid.  Just use an IF ELSE and no lookup table.  It's not needed.

[DavidAlfa]

Yeah, always use pullup resistors.
Can't remember if that PIC could enable internal pullups for PortD, I think they were only available for PortB.

Try this.
Count direction or output pins might be reversed, easy fix. Tested, works fine.

Code: [Select]

int encoder_pos;                                 // Absolute position (You might need to increase the storage type to signed long)
unsigned char encoder_update;                    // 0=Nothing, 1=Left_rotation, 2=Right_rotation (Might need reversing). User must clear it after reading.

void manage_encoder(void){
    static unsigned char stable;
    static signed char add;

    unsigned char now = PORTD & 0xF9;           // Single read to avoid changes, mask RD1&2
    unsigned char now_a = (now & 1) &&1;        // RD0
    unsigned char now_b = (now & 8) &&1;        // RD3

    if (now_a && now_b && stable) {             // Both inputs high after stable condition reached
        stable=0;
        encoder_pos += add;                     // Update absolute position
        encoder_update = (add>0 ? 1 : 2);       // Update flag

        now |= add>0 ? 4 : 2;                   // Update LEDS with the last valid value (10/01 depending on the rotation direction)
        PORTD = now;                            // Update RD1 & 2 outputs
    }
    else if(!now_a && !now_b && !stable) {      // Both inputs low and unstable state
        stable = 1;                             // Reached stable state
    }
    else if(!stable) {                          // Only one pin low, sample pins
        add = now_a ? 1 : -1;                   // Update direction
    }
}


[Peabody]

The standard state machine versions I've seen use the 4-bit previous/current value as an index into a 16-byte lookup table, with each value in the table being 1, -1, or 0.  Zero being no change or an invalid change.  I don't quite understand whether your simplified version is different from that.  Could you post an Arduino sketch demonstrating your version if you have one?

Exactly and what use are all those invalids in the table, why bother with a look up, only 2 of them are valid.  Just use an IF ELSE and no lookup table.  It's not needed.

I still don't understand. If you're tracking the transitions of both pins, then for the typical encoder you will have four legal transitions between detents, which can occur in either direction.  So there are eight valid entries, four invalid entries, and four no-change entries (which if using interrupts are also invalid).  Are you only looking for the *last* valid entry going into the detent position?
 

[Doctorandus_P]

Never ever put ceramic caps directly over those encoder switches.
(Don't feel ashamed, it's a common mistake)

The problem is that such capacitors can deliver very high peak currents. 10A or more is quite possible, and that means you have 10A though your switches right when they are bouncing, and this will wear out your switches prematurely.

Edit:
Further on I see a schematic with added series resistors. That's much better.

[paulca]

The standard state machine versions I've seen use the 4-bit previous/current value as an index into a 16-byte lookup table, with each value in the table being 1, -1, or 0.  Zero being no change or an invalid change.  I don't quite understand whether your simplified version is different from that.  Could you post an Arduino sketch demonstrating your version if you have one?

Exactly and what use are all those invalids in the table, why bother with a look up, only 2 of them are valid.  Just use an IF ELSE and no lookup table.  It's not needed.

I still don't understand. If you're tracking the transitions of both pins, then for the typical encoder you will have four legal transitions between detents, which can occur in either direction.  So there are eight valid entries, four invalid entries, and four no-change entries (which if using interrupts are also invalid).  Are you only looking for the *last* valid entry going into the detent position?

Only two transitions are valid.  One says it moved right, one says it moved left.  All the others are pointless to know.

Code

Works perfectly.

Put another way, yes the encoder can produce more transitions, but they are invalid, bounces, null states and ultimately all you want to know is... did it move anti-clock wise or clockwise.  The datasheets often show the state transition diagrams.  People too often just implement the whole statemachine.  Yet I, again, quesion, what is the point in knowning it bounced?  What is the point in knowing it went to it's detent and back again?  None.  So just ignore them and pick the 2 transitions you want.

Balancing the encoder in the both on, both off is not a state.