 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Skirmitt
Joined: 19 May 2009
Posts: 60
|
| Reading an optical rotary encoder slowly |
 Posted: Thu Oct 13, 2011 3:54 am |
 |
|
I'm developing a MIDI board that has some optical rotary encoders. These give me 128cpr en have a pretty neat signal on the scope. The problem I encounter is that when I turn the wheel slowly the code generates both a forward and a backward midi message. If I turn fast the results are as they should be. Is this just a limitation of encoders or can I do some improvement in the code ?
| Code: |
#include <16F886.h>
#fuses INTRC_IO, NOWDT, BROWNOUT, PUT, NOLVP
#use delay(clock=8000000)
#use rs232(baud=31250, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
#define STATUS PIN_C2
//-------------------------
// This function reads Port B without changing the TRIS.
int8 input_state_b(void)
{
#byte PortB = 6 // PortB address for 16F PICs
return(PortB);
}
void midiout(int cc_data, int c_num, int c_val);
int8 current, last;
#int_rb
void int_rb_isr(void)
{
if ((input (PIN_B1) == 0) && (input(PIN_C0) == 0) )
{
midiout(144,40,127);
}
if ((input (PIN_B1) == 0) && (input(PIN_C0) == 1) )
{
midiout(144,40,1);
}
}
//------------------------------
void init()
{
current = input_state_b();
last = current;
}
void main()
{
init();
port_b_pullups(0b11111111); // Enable pullups on PortB
delay_us(10); // Allow time for pull-ups to initialize
enable_interrupts(INT_EXT_L2H);
enable_interrupts(INT_RB1);
enable_interrupts(INT_RB2);
input_state_b(); // Clear mismatch condition
clear_interrupt(INT_RB);
clear_interrupt(INT_EXT);
enable_interrupts(GLOBAL);
while(1)
{
output_high(STATUS);
delay_ms(1000);
output_low(STATUS);
delay_ms(1000);
}
}
// This function sends a Midi CC.
void midiout(int cc_data, int c_num, int c_val){
putc(cc_data);
putc(c_num);
putc(c_val);
}
|
|
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19334
|
|
| Posted: Thu Oct 13, 2011 5:05 am |
|
|
Seriously you _must_ move the midi output, outside the interrupt.
It takes you just on 1mSec to send the message. 128PPR, implies 512 _edges_ per revolution. Your software will stop working properly if more than one edge is received every mSec. There will actually be two failure modes. At higher speeds, edges will be missed. At slightly lower speeds, a second edge will be received before leaving the interrupt, and a second message sent. This is what you are seeing.....
You also need to read all the port bits at just _one_ point in your ISR, otherwise you will get unexpected results, if a bit changes between the various tests. In this regard, there is also something 'dubious' about the connections. Put _both_ of the quadrature bits on port B, and read these both with one operation.
In your interrupt, just increment/decrement a 'position'. Nothing more.
Then in the main, have an 'old_position' count as well. Compare this with the position from the interrupt. If it _differs_, send a movement signal corresponding to which way it has changed, and update the 'old_position' according to this movement. This way if a second move comes while sending, you will see the positions differ again after the transmission, and send a second move.
Remember though that you will have an upper limit on supported rotational speed at about 240RPM, given how long it takes to send the message....
Best Wishes |
|
|
Skirmitt
Joined: 19 May 2009
Posts: 60
|
|
| Posted: Thu Oct 13, 2011 5:53 am |
|
|
Ok I'll try to implement your tips.
With 128 cpr and the 2 bit gray code feature, would it be possible to have more than 128 steps per cycle ? |
|
|
Skirmitt
Joined: 19 May 2009
Posts: 60
|
|
| Posted: Thu Oct 13, 2011 6:55 am |
|
|
I changed the code and HW. I'm now supposed to see (fictional values) 1-2-3-4-1-2-3-4 but I don't. I see 1-2-1-3-4-1-1-2-1-3-4. Do you understand ? Would there be something wrong with the encoder ?
| Code: |
#include <16F886.h>
#fuses INTRC_IO, NOWDT, BROWNOUT, PUT, NOLVP
#use delay(clock=8000000)
#use rs232(baud=31250, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
#define STATUS PIN_C2
int sendrev=0;
int sendfw=0;
int enc;
int encold;
//-------------------------
// This function reads Port B without changing the TRIS.
int8 input_state_b(void)
{
#byte PortB = 6 // PortB address for 16F PICs
return(PortB);
}
void midiout(int cc_data, int c_num, int c_val);
int8 current, last;
#int_rb
void int_rb_isr(void)
{
enc = input_state_b();
}
//------------------------------
void init()
{
current = input_state_b();
last = current;
}
void main()
{
init();
port_b_pullups(0b11111111); // Enable pullups on PortB
delay_us(10); // Allow time for pull-ups to initialize
enable_interrupts(INT_RB1);
enable_interrupts(INT_RB2);
input_state_b(); // Clear mismatch condition
clear_interrupt(INT_RB);
clear_interrupt(INT_EXT);
enable_interrupts(GLOBAL);
while(1)
{
if(encold!=enc) midiout(144,40,enc);
encold = enc;
}
}
// This function sends a Midi CC.
void midiout(int cc_data, int c_num, int c_val){
putc(cc_data);
putc(c_num);
putc(c_val);
}
|
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19334
|
|
| Posted: Thu Oct 13, 2011 7:53 am |
|
|
| Code: |
#include <16F886.h>
#fuses INTRC_IO, NOWDT, BROWNOUT, PUT, NOLVP
#use delay(clock=8000000)
#use rs232(baud=31250, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
#define STATUS PIN_C2
int16 position=0,old_position=0;
#byte PORTB=getenv("SFR:PORTB")
void midiout(int cc_data, int c_num, int c_val);
#int_rb
void int_rb_isr(void) {
static int8 old;
static int8 new;
static int8 value;
new=PORTB; //You don't need a separate function for this....
//Assuming the encoder is on B1, and B2
//Now I have to decode the quadrature changes. There are four
//possibilities:
//I can have a rising or falling edge, on each of the two inputs. I have to
//look at the state of the other bit, and increment/decrement according to
//this.
value=new^old;
//'value', now has the bit set, which has changed
if (value & 0x2) {
//Here the low bit has changed
if (new & 0x2) {
//Here a rising edge on A
if (new & 0x4) --position;
else ++position;
}
else {
//Here a falling edge on A
if (new & 0x4) ++position;
else --position;
}
}
else {
//Here the high bit (B) must have changed
if (new & 0x4) {
//Here a rising edge on B
if (new & 0x2) ++position;
else --position;
}
else {
//Here a falling edge on B
if (new & 0x2) --position;
else ++position;
}
}
old=new;
}
void down(void) {
old_position--; //update position ASAP
midiout(144,40,127);
}
void up(void) {
old_position++;
midiout(144,40,1);
}
void main(void) {
int8 dummy;
port_b_pullups(0b11111111); // Enable pullups on PortB
delay_us(10); // Allow time for pull-ups to initialize
dummy=PORTB; //Clear the mismatch _before_ enabling interrupts
clear_interrupt(INT_RB);
enable_interrupts(INT_RB1);
enable_interrupts(INT_RB2);
clear_interrupt(INT_EXT);
enable_interrupts(GLOBAL);
do {
if(position != old_position) {
//Here moved
if (position<old_position) {
//Here going _down_, unless position is 0
if (position==0) up();
else down();
}
else {
//Here going _up_ unless old_position is 0
if (old_position==0) down();
else up();
}
} while (true);
}
// This function sends a Midi CC.
void midiout(int cc_data, int c_num, int c_val){
putc(cc_data);
putc(c_num);
putc(c_val);
}
|
There are 512 detectable change 'edges' for one rotation, of a 128PPR quadrature decoder (this is the 'quad' bit in the name - two signals in 90 degree phase shift to one another). You don't have to decode them all, this example does.
A quadrature decoder, doesn't 'count'. You should see 0,1,3,2,0,1,3,2, for the output bits on a clockwise rotation, and 2,3,1,0,2,3,1,0 for anti-clockwise. There ought to be a '0' somewhere in the cycle. However you have the code setup for the decoder to be on B1, and B2, not B0, and B1, so you should be counting 0,2,6,4, or 1,3,7,5, according to what B0 is actually set to....
The code I have written assumes that the encoder is on pins B1, and B2, you'd have to change the bit tests in the interrupt, and the enable instructions if it was on B0/B1.
Best Wishes |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
|
| Posted: Thu Oct 13, 2011 10:24 am |
|
|
Your original code only interrupted on change of one of your signals. This halves the number of interrupts, which is a good thing if your running (fairly) fast. It also simplfies the decoding. In fact your original code only did anything on one of the four transitions, when B0 had gone from 1 to zero. This *may* be where your original problems came from. It only worked out the direction after a full cycle 1/128th of a rotation, rather than every 1/512th of a rotation that a full decoder does. If, when going slowly, you didn't rotate monotonically, i.e. even for a moment went backwards, then it could get confused or miscount. Maybe that's what was happening. A full four cycle decoder, provided you can cope with the full rate of interrupts, never get confused. Even if its a (less reliable) mechanical type with contact bounce (which looks like the shaft going forwards and back a tiny bit many times) you should be able to keep track of all the transitions. The downside it that you get twice as many interrupts and four times as many updates, which may be too many in some applications.
Even with your previous scheme you should only read the state in one operation, in other words on one port as you've been advised. Even then you may not have captured the state which triggered the interrupt. This is because there is a significant time - interrupt latency - between the IO line/s changing state and the interrupt routine actually running, and then you reading the state. In that time the state may have changed. It is possible to detect that sort of error, as the states should always change in sequence. It they don't, say they jump a state (i.e. you've missed a transition) or stay in the same state (missed three changes or, more likely, its gone "up" and then back "down" as happen in contact bounce) then you can detect it and even deal with it. A software state machine in the isr, tracking the encoder state, will allow you to do all this. It won't be particularly expensive in code size or speed. Another trick to consider is to read the state twice and only act on it if the two are the same. This would also detect bounce type issues.
A state machine would look something like this:
| Code: |
boolean Up, Down;
// Change the definition of the states to match your bit arrangement
enum Encoded_States { A = 0, B = 1, C = 2, D = 3., Unknown } Encoder_State;
// Isr reads the encoded bits into new as in Ttelmah's code.
void main()
{
...
Encoder State = Unknown;
new = Unknown;
...
if (new != Encoder_State)
{
Up = FALSE; // Assume no change in position.
Down = FALSE;
switch(Encoder_State)
{
case A:
// Last known encoder state was A. Only states B or D are allowed.
// Note that the dodgy transitions move an even number of states: 0 or 2. In both cases we cannot know for sure which direction it was, so in this example we ignore them for simplicity.
if (new == B)
Up = TRUE;
else if (new == D)
Down = TRUE;
break;
case B:
if (new == C)
Up = TRUE;
else if (new == A)
Down = TRUE;
break;
case C:
if (new == D)
Up = TRUE;
else if (new == B)
Down = TRUE;
break;
case D:
if (new == A)
Up = TRUE;
else if (new == C)
Down = TRUE;
break;
Unknown:
// We've moved, but as we had no previous state we assume we've moved "up".
Up = TRUE;
break;
default:
// Catch all. Assume no change;
break;
}
// Act on the valid change of state
if (Up || Down)
Encoded_State = new;
...
}
|
RF Developer |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19334
|
|
| Posted: Thu Oct 13, 2011 2:42 pm |
|
|
Except your state numbers are wrong. You have made the same error as the original poster - the states _don't_ 'count'. The table is:
| Code: |
0 0 first state
0 1 change of first direction signal
1 1 change of second signal
1 0 fourth state - change _back_ of first signal
0 0 Then back to first state
|
With you stepping through this sequence up/down, according to the direction the shaft is going.
Best Wishes |
|
|
Skirmitt
Joined: 19 May 2009
Posts: 60
|
|
| Posted: Fri Oct 14, 2011 1:04 am |
|
|
Yesterday I changed the code a bit more with some more info I found on this forum.
In the interrupt I only read PORT B
| Code: |
#int_rb
void int_rb_isr(void)
{
newencoder = (input_state_b() & 0b00000110); // keep 2 bits of port b
}
|
A function that reads the encoder state
| Code: |
void encoder(void)
{
if(newencoder != lastencoder) //Check for a change
{
if (bit_test(newencoder,2) == bit_test(lastencoder,1))
midiout(144,40,127); else midiout(144,40,1);
}
lastencoder = newencoder; // save state
}
|
I call the encoder function in the main loop. This gives me good results with relative simple code. I guess this is what I had to do with your first advice Ttelmah. I'll test it more today but I hope I'm finished with this part for now. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
