Quad Encoder Problem

[RossWaddell]

This doesn't seem to be my week ... I had problems earlier with converting code from a PIC12F683 to a PIC12F1840 which thankfully was resolved yesterday (thanks Henrik!) but when I come back to something I thought was working fine, it's not.

I found examples of determining the direction of turn of a mechanical quadrature encoder. Darrel helped way back with some of it and all was well until late last night when I thought I'd better retest this after working on other parts of the project. Lo and behold, the MotorRPM variable isn't going up or down steadily but rather goes up by one or two but could just as easily go down (when turning the knob in the same direction). I did spend the first part of the year converting from using PBP's HPWM on CCP1/2 (which are ECCP modules on the 16F1825) to CCP3/4 (regular CCP modules) but at the time I'm fairly certain I tested the quadrature part and it worked OK.

Here's my code (edited for clarity):


' ************************************************** *************
' Pin Connections
' ************************************************** *************

' RA0 -> B pin of rotary encoder
' RA1 -> A pin of rotary encoder
' RA2/CCP3 -> Stbd motor PWM output (motor 1)
' RA3/MCLR -> Button switch pin of rotary encoder
' RC0 -> Stbd motor direction signal (motor 1)
' RC1/CCP4 -> Port motor PWM output (motor 2)
' RC2 -> Port motor direction signal (motor 2)
' RC4/Tx -> Serial LCD output
' RC5/Rx -> Serial input

DEFINE OSC 16 ; Set oscillator 16Mhz

' ************************************************** *************
' Device Fuses
' ************************************************** *************

#CONFIG
__config _CONFIG1, _FOSC_INTOSC & _WDTE_ON & _PWRTE_ON & _MCLRE_OFF & _CP_OFF & _CPD_OFF
__config _CONFIG2, _PLLEN_OFF & _STVREN_ON & _BORV_LO & _LVP_OFF
#ENDCONFIG

' ************************************************** *************
' Initialization
' ************************************************** *************

OSCCON = %01111000 ' 16MHz internal osc

pause 100 ' As advised by Darrel Taylor for EEPROM issue

ANSELA = 0 ; Digital only for all PortA pins
ANSELC = 0 ; Diginal only for all PortC pins

TRISA = %00001011 ' Make PORTA pins 0-1 input for rotary encoder,
' pin 3 for rotary button
TRISC = 0 ' Make all PORTC pins output

' The INTEDG bit of the OPTION_REG register determines on which edge the
' interrupt will occur. When the INTEDG bit is set, the rising edge will
' cause the interrupt. When the INTEDG bit is clear, the falling edge will
' cause the interrupt.
OPTION_REG.6 = 1 ' 1=Rising edge (default) or button "PRESS";
' 0=Falling edge or button "RELEASE"

Old_Bits VAR BYTE
New_Bits VAR BYTE
RotEncDir VAR BIT ' 1=CW, 0=CCW
' Rot Enc pin A connected to PortA.1;
' Rot Enc pin B connected to PortA.0
Old_RPM VAR BYTE
i VAR BYTE

' ************************************************** *************
' Set up registers for PWM on CCP3 & CCP4
' ************************************************** *************

CCP3CON = %00001100 ; Use CCP3 in PWM mode
CCP4CON = %00001100 ; Use CCP4 in PWM mode

T2CON = %00000101 ; Timer2 on with 1:4 prescaler
PR2 = 62 ; For 16Mhz OSC the desired output freq of 15,873Hz is
; achieved with this PR2 value (8-bit resolution
; with 1:4 prescaler)

; PWM freq must be ~ 16-20kHz to reduce noise

PreSpinVal CON 17 ; value to subtract from MinDuty for motor spin up
MinDuty CON 75 ; 75 when max duty = 250 (8-bit resolution)
;SpinUpPause CON 17 ; Pause during motor spin up
SpinUpPause VAR BYTE ; Pause during motor spin up
SpinUpPause = 17
#IFDEF USE_LCD_FOR_DEBUG
SpinUpPause = 12 ; Subtract 5ms pause when outputting to LCD
#ENDIF

MaxDuty VAR WORD ; According to Darrel:
; MaxDuty = (PR2 + 1) * 4

MaxDuty = (PR2 + 1) * 4 ; 252 but with prescaler resolution it's actually 250

DutyVar3 VAR WORD ; temp variable to store duty variable for CCP3
DutyVar4 VAR WORD ; temp variable to store duty variable for CCP4

TimeCnt VAR Word

' ************************************************** *************
' Includes
' ************************************************** *************

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts
' --> copy both files to PBP main folder
' (i.e. c:\pbp3)

' ************************************************** *************
' EEPROM Variables
' ************************************************** *************

MotorRPM_Default CON 177
EE_MotorRPM DATA MotorRPM_Default
MotorRPM VAR BYTE
READ EE_MotorRPM, MotorRPM

PortEngDir_Default CON 0
EE_PortEngDir DATA PortEngDir_Default
PortEngDir VAR BYTE
READ EE_PortEngDir, PortEngDir

' ************************************************** *************
' ASM Interrupt Definitions
' ************************************************** *************

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler IOC_INT, _RotEncAdjust, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM

' ************************************************** *************
' Set default values
' ************************************************** *************

Old_RPM = MotorRPM
Old_Bits = PORTA & (%00000011)

' Enable interrupts on RPM control now that motors are fully spun up
INTCON = %10001000 ' Global int enabled, IOCI enabled,
' INTF flag bit 0 clr, IOCI flag bit 0 clr
IOCAP.0 = 1 ' Enable positive (rising edge) change
IOCAP.1 = 1 ' Enable positive (rising edge) change
IOCAN.0 = 1 ' Enable negative (falling edge) change
IOCAN.1 = 1 ' Enable negative (falling edge) change
IOCAF.0 = 0 ' Clear interupt-on-change flag
IOCAF.1 = 0 ' Clear interupt-on-change flag

@ INT_ENABLE IOC_INT ; Interrupt-on-Change interrupt

Main:
' Check if motor RPM has changed
IF MotorRPM <> Old_RPM Then
Old_RPM = MotorRPM
GOSUB ChngMotorHPWM
EndIF

TimeCnt = 0
While ButtonPress = 0
TimeCnt = TimeCnt + 1
Pause 10
If TimeCnt > 500 Then BtnAction
Wend

BtnAction:
If TimeCnt > 0 and TimeCnt < 200 Then
PortEngDir = PortEngDir + 1
If PortEngDir > 1 Then PortEngDir = 0

WRITE EE_PortEngDir, PortEngDir
#IFDEF USE_LCD_FOR_DEBUG
HSEROUT [LCD_INST, LCD_CLR]
pause 5
HSEROUT ["PortEngDir=", DEC PortEngDir, " ", 13, 10] ' Send text followed by carriage return and linefeed
#ENDIF

GOSUB ReversePortEng
ElseIf TimeCnt >= 200 Then
WRITE EE_MotorRPM, MotorRPM_Default ; restore Default value

MotorRPM = MotorRPM_Default
EndIf

GOTO Main


' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)
IF (New_Bits & %00000011) = (Old_Bits & %00000011) Then DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

WRITE EE_MotorRPM, MotorRPM
pause 100

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN


Here's a video which shows the problem (I hope): http://youtu.be/YM4oqX2lcOg

Here's my schematic:

6893

Again, AFAIK nothing has changed in the two months since I last worked on this code and all was well.

Does anyone see where I'm going wrong? Here's what I've tried:


Swapped out the quad encoder with a spare one (same model)
Swapped out the PIC and programmed another one
Replaced the 10k resistors that tie the encoder's A/B outputs to +5V
Double-checked the connections



Something to do with Timer2 & PWM on CCP3/4? Bad IOC interrupt config? What's strange is that the quad encoder has a pushbutton knob and that's working just fine (although it's not using an interrupt).

[HenrikOlsson]

Hi,
First order of business, remove the Pause 100 from the interrupt service routine.

Also, is the intention really to write the MotorRPM variable to EEPROM on every interrupt? I see two possible issues with that, the first is that the write operation takes a while and the second is that you may wear out the EEPROM. Finally, not related to the actual problem but still

IF (New_Bits & %00000011) = (Old_Bits & %00000011) Then DoneRotEnc
Since you're assigning the value of New_Bits by ANDing PortA with 3 and then, at the end setting Old_Bits = New_Bits there's no need to AND New_Bits or Old_Bits with 3 here as well. Neither of those variables will ever have bits 2-7 set anyway, you've already taken care of that. (As long as you don't use them anywhere else in your program of course).

/Henrik.

[RossWaddell]

It doesn't work with or without the 'Pause 100' in the interrupt service routine - I actually only added that based on some sample interrupt code from DT (I thought it made sense if there's some latency to saving to EEPROM you might want to wait a tiny bit).

Other than saving the new MotorRPM to EEPROM everytime it changes, I don't know what else I can do. Users won't be modifying the motor speed very often (once they find the speed they like, they'll just leave it). I suppose I could set up a timer to save the value to EEPROM if it's changed, but I'm open to suggestions :)

For the above code line, are you saying I could just use this instead?


IF New_Bits = Old_Bits THEN DoneRotEnc

[HenrikOlsson]

Hi,
According to the manual the WRITE command "is self-timed and may take up to 10 milliseconds to execute on a PIC MCU".
I suspect that's the issue, most encoders produce a full quadrature cycle, ie 4 edges (4 interrupts) per click/detent. IF the WRITE operation takes 10ms (and/or you have that Pause 100 in there) it's likely that it will miss pulses/edges and get confused.

Try commenting out the Write operation and see if that fixes it.

Yes, that's what's I'm saying regarding New_Bits = Old_Bits.
At the beginning of the ISR you do New_Bits = PortA & %00000011 so the top 6 bits of New_Bits are cleared. At the end of the ISR you assign the value in New_Bits to Old_Bits so the top 6 bits in Old_Bits will also be zero. Therefor the second AND operation is unnecessary, but it doesn't have anything to do with the actual problem....

/Henrik.

[RossWaddell]

I'll try this out tonight, Henrik. Thanks so much for your help here and on my other thread from this week.

If the WRITE operation in the interrupt service routine is the cause, then I'll move that to the check in the Main loop.

[HenrikOlsson]

Hi,
Just realised I misunderstood you a bit. I thought you were trying to use the 12F1840 with this code, then I realised you're actually using the 16F1825 that's in the schematic...duh...

Anyway, I've breadboarded a 12F1840 (don't have any 18F1825) and now have an encoder counting properly, sending the count over the USART. I simulated the EEPROM write by adding a PAUSEUS xxx in the handler and found that the limit for correct response - in my case - is around 500us. Anything above that it starts missing edges, coutning the wrong way etc.

So, if the EEPROM write takes even remotely as long as 10ms then that is most likely the issue.

/Henrik.

[RossWaddell]

That gives me strong confidence that it is the WRITE line; I'll comment it out but ultimately move it to the main loop.

[Ioannis]

It is too much to have very often (on every click of the rotary encoder) to write the value in EEPROM.

Do it when there is really a need for this, e.g. when the value is different from old value AND a certain time has passed. This time should be reset while the encoder is rotating.

Ioannis

[RossWaddell]

Hi,
Just realised I misunderstood you a bit. I thought you were trying to use the 12F1840 with this code, then I realised you're actually using the 16F1825 that's in the schematic...duh...

Anyway, I've breadboarded a 12F1840 (don't have any 18F1825) and now have an encoder counting properly, sending the count over the USART. I simulated the EEPROM write by adding a PAUSEUS xxx in the handler and found that the limit for correct response - in my case - is around 500us. Anything above that it starts missing edges, coutning the wrong way etc.

So, if the EEPROM write takes even remotely as long as 10ms then that is most likely the issue.

/Henrik.
Ok, it works now - mostly. What's weird is the 'MotorRPM' as displayed on the LCD jumps by 3-4 (i.e. current setting is '120' and then one click of the encoder makes it '124'). The only thing I did was comment out the WRITE line (and the PAUSE 100 too, of course). Sometimes, it misses incrementing the click completely (although only rarely).

Does this have something to do with the comparison logic?



' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)
IF (New_Bits & %00000011) = (Old_Bits & %00000011) Then DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

; WRITE EE_MotorRPM, MotorRPM
; pause 100

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN

[RossWaddell]

It is too much to have very often (on every click of the rotary encoder) to write the value in EEPROM.

Do it when there is really a need for this, e.g. when the value is different from old value AND a certain time has passed. This time should be reset while the encoder is rotating.

Ioannis
That makes sense, Ioannis. I didn't want to lose a user setting if power was interrupted before the save but maybe I could set up a timer (using DT's interrupt library) to check if the value is changed every 30 seconds or so.

[RossWaddell]

Another weird thing - If I comment out this line:



' Enable interrupts on RPM control now that motors are fully spun up
INTCON = %10001000 ' Global int enabled, IOCI enabled,
' INTF flag bit 0 clr, IOCI flag bit 0 clr
IOCAP.0 = 1 ' Enable positive (rising edge) change
IOCAP.1 = 1 ' Enable positive (rising edge) change
IOCAN.0 = 1 ' Enable negative (falling edge) change
IOCAN.1 = 1 ' Enable negative (falling edge) change
IOCAF.0 = 0 ' Clear interupt-on-change flag
IOCAF.1 = 0 ' Clear interupt-on-change flag

@ INT_ENABLE IOC_INT ; Interrupt-on-Change interrupt


and these ones:



' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)
IF (New_Bits & %00000011) = (Old_Bits & %00000011) Then DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

; WRITE EE_MotorRPM, MotorRPM
; pause 100

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN


... the interrupts don't seem to fire. And then the button push doesn't work, either (and that's not interrupt driven):



Main:
' Check if motor RPM has changed
IF MotorRPM <> Old_RPM Then
Old_RPM = MotorRPM
GOSUB ChngMotorHPWM
EndIF

TimeCnt = 0
While ButtonPress = 0
TimeCnt = TimeCnt + 1
Pause 10
If TimeCnt > 500 Then BtnAction
Wend

BtnAction:
If TimeCnt > 0 and TimeCnt < 200 Then
PortEngDir = PortEngDir + 1
If PortEngDir > 1 Then PortEngDir = 0

WRITE EE_PortEngDir, PortEngDir
#IFDEF USE_LCD_FOR_DEBUG
HSEROUT [LCD_INST, LCD_CLR]
pause 5
HSEROUT ["PortEngDir=", DEC PortEngDir, " ", 13, 10] ' Send text followed by carriage return and linefeed
#ENDIF

GOSUB ReversePortEng
ElseIf TimeCnt >= 200 Then
WRITE EE_MotorRPM, MotorRPM_Default ; restore Default value

MotorRPM = MotorRPM_Default
EndIf

GOTO Main

[HenrikOlsson]

Hi,

What's weird is the 'MotorRPM' as displayed on the LCD jumps by 3-4 (i.e. current setting is '120' and then one click of the encoder makes it '124').
Not weird, as I wrote earlier - turning the encoder from one click/detent to the next results in one quadrature cycle which is four "edges", ie four interrupts so, four counts:

6894

That's why you can't have any delay in the ISR because turning the encoder one "click" results in four interrupts, if you have a delay (Pause or other) in there it misses one or more of the four edges in the cycle and gets confused.

You could setup a timer for the EEPROM save, or you could just set a variable to some value in the ISR, the Main routine then decrements this value and when it hits zero you save the value to EEPROM. That will allow the user to turn the knob and the value be saved some time after the change is complete, psuedo code

TimeToSave VAR WORD
ValueDirty VAR BIT

Main:
'Code...
'Code...

If ValueDirty THEN
TimeToSave = TimeToSave - 1
If TimeToSave = 0 ' Time to save ?
Write.....
ValueDirty = 0 ' Clear the flag
ENDIF
ENDIF

'Code
'Code
Goto Main

ISR:
TimeToSave = 5000
ValueDirty = 1
'More code....
@ INT_RETURN
Here the interrupt routine sets a flag indicating that the value has changed, it also sets a "counter" variable which the main routined counts down. If the Main routine executed 5000 times without the value being changed it saves the value to EEPROM. Obviously this relies on a somewhat constant execution time thru the Main routine but you get the idea.

/Henrik.

EDIT: Cross posting.... As for the other "weird" thing. It seems that DT-Ints does NOT handle clearing hte interrupt request flag for the IOC interrupt on these devices so you SHOULD to do that manually before exiting the interrupt (normally NOT needed when the ClearFlag switch is set to Yes). If the interrupt request flag is NOT cleard the interrupt refires over and over and over again which is why the button doesn't seem to work - that code never runs because the CPU spends all the time servicing the interrupt. At least that's what I think is happening, I know it didn't work here if I didn't clear the flags manually.

[RossWaddell]

Hi,

Not weird, as I wrote earlier - turning the encoder from one click/detent to the next results in one quadrature cycle which is four "edges", ie four interrupts so, four counts:

6894

I coulda sworn I had it incrementing by 1 in earlier versions (back when I was using HPWM). Nonetheless, I believe I need to detect the 4 edges in order to determine the direction of turn, so maybe I'll add a counter (1-4) in the ISR and only increment MotorRPM when it hits 4 (and then reset).


You could setup a timer for the EEPROM save, or you could just set a variable to some value in the ISR, the Main routine then decrements this value and when it hits zero you save the value to EEPROM. That will allow the user to turn the knob and the value be saved some time after the change is complete, psuedo code

TimeToSave VAR WORD
ValueDirty VAR BIT

Main:
'Code...
'Code...

If ValueDirty THEN
TimeToSave = TimeToSave - 1
If TimeToSave = 0 ' Time to save ?
Write.....
ValueDirty = 0 ' Clear the flag
ENDIF
ENDIF

'Code
'Code
Goto Main

ISR:
TimeToSave = 5000
ValueDirty = 1
'More code....
@ INT_RETURN
I'll try this out. At first glance, though, the ISR is always setting TimeToSave to 5000 so I think it needs a check like:


If ValueDirty = 0 Then
TimeToSave = 5000
EndIf



EDIT: Cross posting.... As for the other "weird" thing. It seems that DT-Ints does NOT handle clearing hte interrupt request flag for the IOC interrupt on these devices so you SHOULD to do that manually before exiting the interrupt (normally NOT needed when the ClearFlag switch is set to Yes). If the interrupt request flag is NOT cleard the interrupt refires over and over and over again which is why the button doesn't seem to work - that code never runs because the CPU spends all the time servicing the interrupt. At least that's what I think is happening, I know it didn't work here if I didn't clear the flags manually.
I'll have to add a comment to my IOC ISR's that I do need to clear the flags.

Thanks again, Henrik!

[RossWaddell]

This works. Probably a bit inefficient having the TMR1 interrupt firing @ 8Hz, but if only writes to EEPROM if the value has changed:




' ************************************************** *************
' Pin Connections
' ************************************************** *************

' RA0/pin 13 -> B pin of rotary encoder
' RA1/pin 12 -> A pin of rotary encoder
' RA2/CCP3/pin 11 -> Stbd motor PWM output (motor 1)
' RA3/MCLR/pin 4 -> Button switch pin of rotary encoder
' RC0/pin 10 -> Stbd motor direction signal (motor 1)
' RC1/CCP4/pin 9 -> Port motor PWM output (motor 2)
' RC2/pin 8 -> Port motor direction signal (motor 2)
' RC4/Tx/pin 6 -> Serial LCD output
' RC5/Rx/pin 5 -> Serial input

DEFINE OSC 16 ; Set oscillator 16Mhz
DEFINE HSER_TXSTA 20h ; Set transmit status and control register
DEFINE HSER_BAUD 2400 ; Set baud rate

#CONFIG
__config _CONFIG1, _FOSC_INTOSC & _WDTE_ON & _PWRTE_ON & _MCLRE_OFF & _CP_OFF & _CPD_OFF
__config _CONFIG2, _PLLEN_OFF & _STVREN_ON & _BORV_LO & _LVP_OFF
#ENDCONFIG

' ************************************************** *************
' Initialization
' ************************************************** *************

OSCCON = %01111000 ' 16MHz internal osc

pause 100 ' As advised by Darrel Taylor for EEPROM issue

APFCON0.2 = 0 ; Tx on RC4 for LCD display
APFCON0.7 = 0 ; Rx on RC5

BAUDCON.4 = 1 ; Transmit inverted data to the Tx pin

ANSELA = 0 ; Digital only for all PortA pins
ANSELC = 0 ; Diginal only for all PortC pins

TRISA = %00001011 ' Make PORTA pins 0-1 input for rotary encoder,
' pin 3 for rotary button
TRISC = 0 ' Make all PORTC pins output

' The INTEDG bit of the OPTION_REG register determines on which edge the
' interrupt will occur. When the INTEDG bit is set, the rising edge will
' cause the interrupt. When the INTEDG bit is clear, the falling edge will
' cause the interrupt.
OPTION_REG.6 = 1 ' 1=Rising edge (default) or button "PRESS";
' 0=Falling edge or button "RELEASE"

Old_Bits VAR BYTE
New_Bits VAR BYTE
RotEncDir VAR BIT ' 1=CW, 0=CCW
' Rot Enc pin A connected to PortA.1;
' Rot Enc pin B connected to PortA.0
Old_RPM VAR BYTE
i VAR BYTE

ButtonPress VAR PORTA.3 ' Alias PORTA.3 as "ButtonPress"

TimeCnt VAR Word

ISRCounter VAR BYTE
ValueDirty VAR BIT

' ************************************************** *************
' Includes
' ************************************************** *************

INCLUDE "DT_INTS-14.bas" ' Base Interrupt System
INCLUDE "ReEnterPBP.bas" ' Include if using PBP interrupts
' --> copy both files to PBP main folder
' (i.e. c:\pbp3)

' ************************************************** *************
' ASM Interrupt Definitions
' ************************************************** *************

ASM
INT_LIST macro ; IntSource, Label, Type, ResetFlag?
INT_Handler TMR1_INT, ReloadTMR1, ASM, no ; MUST be first
INT_Handler TMR1_INT, _T1handler, PBP, yes
INT_Handler IOC_INT, _RotEncAdjust, PBP, yes
endm
INT_CREATE ; Creates the interrupt processor
ENDASM

;--- Change these to match the desired interrupt frequency -------------------
;--- See http://DarrelTaylor.com/DT_INTS-14/TimerTemplate.html for more Info.
@Freq = 8 ; Frequency of Interrupts in Hz
@Prescaler = 8 ; Timers Prescaler setting
T1CON = $30 ; $30 = Prescaler 1:8, TMR1 OFF
; $00=1:1, $10=1:2, $20=1:4, $30=1:8 -- Must match @Prescaler value

' ************************************************** *************
' Set default values
' ************************************************** *************

Old_RPM = MotorRPM
Old_Bits = PORTA & (%00000011)
ISRCounter = 0


' Enable interrupts on RPM control now that motors are fully spun up
INTCON = %10001000 ' Global int enabled, IOCI enabled,
' INTF flag bit 0 clr, IOCI flag bit 0 clr
IOCAP.0 = 1 ' Enable positive (rising edge) change
IOCAP.1 = 1 ' Enable positive (rising edge) change
IOCAN.0 = 1 ' Enable negative (falling edge) change
IOCAN.1 = 1 ' Enable negative (falling edge) change
IOCAF.0 = 0 ' Clear interupt-on-change flag
IOCAF.1 = 0 ' Clear interupt-on-change flag

@ INT_ENABLE IOC_INT ; Interrupt-on-Change interrupt
@ INT_ENABLE TMR1_INT ; enable Timer 1 interrupts
GOSUB StartTimer ; Start Timer 1 ('MotorRPM' will be saved on every
; interrupt if dirty

Main:
' Check if motor RPM has changed
IF MotorRPM <> Old_RPM Then
Old_RPM = MotorRPM
GOSUB ChngMotorHPWM
EndIF

TimeCnt = 0
While ButtonPress = 0
TimeCnt = TimeCnt + 1
Pause 10
If TimeCnt > 500 Then BtnAction
Wend

BtnAction:
If TimeCnt > 0 and TimeCnt < 200 Then
PortEngDir = PortEngDir + 1
If PortEngDir > 1 Then PortEngDir = 0

WRITE EE_PortEngDir, PortEngDir
#IFDEF USE_LCD_FOR_DEBUG
HSEROUT [LCD_INST, LCD_CLR]
pause 5
HSEROUT ["PortEngDir=", DEC PortEngDir, " ", 13, 10] ' Send text followed by carriage return and linefeed
#ENDIF

GOSUB ReversePortEng
ElseIf TimeCnt >= 200 Then
WRITE EE_MotorRPM, MotorRPM_Default ; restore Default value

MotorRPM = MotorRPM_Default
EndIf

GOTO Main


' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)
; IF (New_Bits & %00000011) = (Old_Bits & %00000011) Then DoneRotEnc
IF New_Bits = Old_Bits Then DoneRotEnc

' Increment ISR counter (since quad encoder throws 4 interrupts
' for every single detent). Only want to update MotorRPM if this counter
' reaches 4 (i.e. a single detent)
ISRCounter = ISRCounter + 1
IF ISRCounter < 4 THEN DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

; GOSUB StartTimer ; Start the TMR1 Timer to save MotorRPM
ValueDirty = 1

ISRCounter = 0
; WRITE EE_MotorRPM, MotorRPM
; pause 100

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN

;____This routine is Called on each TMR1 Interrupt____________________________
T1handler:
IF ValueDirty = 1 THEN
WRITE EE_MotorRPM, MotorRPM
; GOSUB StopTimer ; Stop the Timer

ValueDirty = 0
EndIf

@ INT_RETURN

;---[TMR1 reload - interrupt handler]-----------------------------------------
ASM ; Calculate Timer Reload Constant
ReloadInst = 8 ; # of Intructions used to reload timer
if ((Prescaler == 1)||(Prescaler == 2)||(Prescaler == 4)||(Prescaler == 8))
MaxCount = 65536 + (ReloadInst / Prescaler)
TimerReload = MaxCount - (OSC*1000000/4/Prescaler/Freq)
if ((TimerReload < 0) || (TimerReload > (65535-ReloadInst)))
error Invalid Timer Values - check "OSC", "Freq" and "Prescaler"
endif
else
error Invalid Prescaler
endif
ENDASM

@Timer1 = TMR1L ; map timer registers to a word variable
Timer1 VAR WORD EXT
TimerReload CON EXT ; Get the External Constant
TMR1ON VAR T1CON.0 ; Alias the Timers ON/OFF bit

;---Reload Timer1------
ASM
ReloadTMR1
MOVE?CT 0, T1CON, TMR1ON ; 1 stop timer
MOVLW LOW(TimerReload) ; 1 Add TimerReload to the
ADDWF TMR1L,F ; 1 value in Timer1
BTFSC STATUS,C ; 1/2
INCF TMR1H,F ; 1
MOVLW HIGH(TimerReload) ; 1
ADDWF TMR1H,F ; 1
MOVE?CT 1, T1CON, TMR1ON ; 1 start timer
INT_RETURN
ENDASM

;---Start/Stop controls -----
StartTimer:
Timer1 = TimerReload ; Load Timer
TMR1ON = 1 ; start timer
RETURN

StopTimer:
TMR1ON = 0 ; stop timer
RETURN

(When I turn off the PIC and turn it on again, it sometimes seems to speed up the motor to (MotorRPM - 1), i.e. 1 less than what was displayed on the LCD before turning off power.

[HenrikOlsson]

I'll try this out. At first glance, though, the ISR is always setting TimeToSave to 5000 so I think it needs a check like:
Well, yes, that was the whole idea. As long as the user keeps turning the knob (generating interrupts) the counter will get reset to 5000 (or whatever). But as soon as the user has NOT changed the RPM for the duration of 5000 iterations thru the Main loop the value is saved. When the knob isn't being turned no interrupts are being generated so the Main routine eventually gets the counter down to 0 and saves the value.

/Henrik.

[RossWaddell]

Oop. Yeah, of course that makes sense. Thanks.

[RossWaddell]

I incorporated your idea, Henrik, and all is well. I could have sworn this was working earlier this year when I last touched it but perhaps if using the same chip and saving too often to EEPROM means it starts to take longer to do the WRITE and hence only then really missed out on the IOC's. I thought it also incremented MotorRPM by 1 but I can't prove that now. In the end, it works with using a ISR counter and only on the 4th one does it increment MotorRPM.

I upped the TimeToSave value to 50000 - seems like a good compromise.

[RossWaddell]

Should I be setting Old_Bits to New_Bits in the 'DoneRotEnc' label for the first 3 of 4 interrupts related to an IOC?



' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)

IF New_Bits = Old_Bits Then DoneRotEnc

' Increment ISR counter (since quad encoder throws 4 interrupts
' for every single detent), only want to update MotorRPM when counter
' reaches 4 (i.e. a single detent 'click')
ISRCounter = ISRCounter + 1
IF ISRCounter < 4 THEN DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

ValueDirty = 1 ; Set dirty flag
ISRCounter = 0 ; Reset ISR entry counter
TimeToSave = 50000 ; Set value to begin countdown to EEPROM save

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN

[HenrikOlsson]

Hi,
I'm afraid I don't quite understand. A quadrature signal consists of four states (00-10-11-01), from any one state there are TWO valid states which it can "transition" to, one in either direction. If you don't keep track of Old_Bits it doesn't know in which state it was and will get confused, count in the wrong direction etc - just as it did when you had the delay in there. Are you trying to keep the MotorRPM from incrementing by 4 for each click?

Why don't you simply have an EncoderCount variable which is the one you save to EEPROM, and then you do MotorRPM = EncoderCount / 4. Also, instead of triggering on both the rising and falling edges you could trig on only the rising (or falling) which would give you two counts per click instead of four.

/Henrik.

[RossWaddell]

But if the interrupts are meant to fire on either a rising or falling edge, then going from '00' to '10' has only one edge (A rises from 0 to 1; B stays the same). The 00 to 10 state change represents one detent, right?

I got this ISR code from another post (http://www.picbasic.co.uk/forum/showthread.php?t=4610&p=25396#post25396) and it seemed to work back then (incrementing MotorRPM by 1)

[HenrikOlsson]

Hi,

But if the interrupts are meant to fire on either a rising or falling edge, then going from '00' to '10' has only one edge (A rises from 0 to 1; B stays the same).
Yes of course, one edge.


The 00 to 10 state change represents one detent, right?
It depends on the encoder used but most mechanical type encoders I've come across has a full quadrature cycle per detent. Here's cutout from the datasheet for a EN12-series (http://www.bitechnologies.com/pdfs/en12.pdf) encoder from TT Electronics:

6897

As you can see, going from one detent to the next results in a full quadrature cycle or four edges. So if the IOC is setup to trig the on the rising and falling edge of both the A and the B input then you'll have four interrupts per detent so the MotorRPM variable will increase by 4.

When it "seemed to work back then" was that with this very same encoder or have you changed it? Do you have a modelnumber or datasheet for the encoder you're using?

/Henrik.

[RossWaddell]

Quite possibly it was using an earlier rotary encoder. I switched to a CUI ACZ11 (datasheet (http://www.cui.com/Product/Resource/DigiKeyPDF/ACZ11.pdf)) one because I needed the button. Perhaps the Grayhill one I was using before (datasheet (http://lgrws01.grayhill.com/web1/images/ProductImages/Series%2025L%20Encoder.pdf)) showed the single-increment-per-detent. The CUI datasheet doesn't explicitly state it goes through a full quadrature cycle but one of the diagrams seems to infer that.

Again, thanks for your patience, Henrik. Your explanation makes sense to me now (I hadn't realized that different mechanical encoders behave differently. Should have checked that, of course).

EDIT: I just replaced the CUI encoder with the Grayhill one on my breadboard and guess what - it increments by 1. Mystery solved. Sadly, there doesn't seem to be a cheap Grayhill encoder with a push button so I'll just have to add comments in case I ever switch encoders again to do this test (or read the data sheet more carefully).

[HenrikOlsson]

Hi,
The CUI datasheet shows that for models 12C and 20C it's full a cycle (4 counts) per detent while for the 15P30 it's half a cycle (2 counts) per detent. It's a bit misleading that they state for example, 12ppr, 12 detents when it's actually 12cpr, 12 detents.

The Grayhill datasheet explicitly says 1/4 cycles (1 count) per detent.

So, yes, "problem" explained/verified.

/Henrik.

[Ioannis]

Are you sure this code does work on both directions?

Should I be setting Old_Bits to New_Bits in the 'DoneRotEnc' label for the first 3 of 4 interrupts related to an IOC?



' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
New_Bits = PORTA & (%00000011)

IF New_Bits = Old_Bits Then DoneRotEnc

' Increment ISR counter (since quad encoder throws 4 interrupts
' for every single detent), only want to update MotorRPM when counter
' reaches 4 (i.e. a single detent 'click')
ISRCounter = ISRCounter + 1
IF ISRCounter < 4 THEN DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

ValueDirty = 1 ; Set dirty flag
ISRCounter = 0 ; Reset ISR entry counter
TimeToSave = 50000 ; Set value to begin countdown to EEPROM save

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN



By the time that ISRCounter is 4, both outputs of the encoder will be at 0. So the RotEncDir will be 0 too and only the ELSE expression will be executed.

Maybe I am missing something here?

Ioannis

[RossWaddell]

I have to admit, Ioannis, the code was grabbed from an example here and implemented on my side using a Grayhill encoder which only triggered the interrupt once per detent. Works perfectly with that device, With the CUI ACZ11, I added the check on the ISRCounter but it **does** work - both CW & CCW rotations are captured correctly.

[RossWaddell]

I tried CCW rotation again last night and it does seem to get confused every once in a while - either decrements by 2 or sometimes even increments. Doesn't happen often enough to be too much of a problem, but I would like to make the code as efficient as possible.

Not sure what else I can do in the ISR, though. If I only need one edge to check the rotation direction then I could do as Henrik suggested and turn off 2 of the edge interrupts; that way, it would only hit the ISR twice. I could do the RotEncDir determination on the first interrupt then set a flag to ignore the 2nd interrupt, then reset those after the 2nd.

[Ioannis]

I stil cannot see how this piece of code can be true:



RotEncDir = New_Bits.1 ^ Old_Bits.0
IF RotEncDir = 1 Then


If you follow what Henrik has posted on that pulse train diagram at #21, you will see that after the above XOR, the IF RotEncDir = 1 statement will always fail and the ELSE will be executed.

Ioannis

[RossWaddell]

The New_Bits will be the same for A & B, but the Old_Bits should still have PORTA.0 (B pin of encoder) as 1 for the last falling edge interrupt, won't it?

[Ioannis]

I don't think so after the 4 step counter. At that moment both will be zero.

Ioannis

[RossWaddell]

But experimentally it works, both for CW & CCW. I'd love to optimize this code for full quad cycle encoders like the CUI ACZ11 one I'm using, but I'm at a loss as to what to change (other than trying Henrik's suggestion of reducing the number of edges to 2 from 4).

[HenrikOlsson]

Hi,
Change the code so that you interrupt on one edge of one channel - only. (The rising edge of channel A for example).
In the ISR, poll "the other" channel, if it's low you're moving in one direction, if it's high you're moving in the other.
Verify that it works at "any" speed you may turn the encoder.

/Henrik.

[RossWaddell]

I tried this but nothing happens - the display changes erratically with encoder movement and only really goes up or down by 1:



#DEFINE USE_CUI_ROTARY_ENCODER ; comment out if using Grayhill rotary encoder
; which triggers an interrupt only once per detent

' Enable interrupts on RPM control now that motors are fully spun up
INTCON = %10001000 ' Global int enabled, IOCI enabled,
' INTF flag bit 0 clr, IOCI flag bit 0 clr

#IFDEF USE_CUI_ROTARY_ENCODER
IOCAP.1 = 1 ' Enable positive (rising edge) change, PORTA.1
' The CUI encoder goes thru a full quad cycle with 1 detent turn,
' so only need to capture 1 edge (A channel, rising)

' CH A
' ====
' ------ ------
' | | | |
' | | | |
' --- ------ ------

' CH B
' ====
' ------ ------
' | | | |
' | | | |
' ------ ------ ---

' Detent markers
' ==============
' | | |
' | | |

' If interrupt is triggered, it means A had a rising edge:
' If B = 0 Then CW
' If B = 1 Then CCW
#ELSE
' For Grayhill encoder, 1 detent triggers only 1 edge
IOCAP.0 = 1 ' Enable positive (rising edge) change, PORTA.0
IOCAP.1 = 1 ' Enable positive (rising edge) change, PORTA.1
IOCAN.0 = 1 ' Enable negative (falling edge) change, POARTA.0
IOCAN.1 = 1 ' Enable negative (falling edge) change, PORTA.1
IOCAF.0 = 0 ' Clear interupt-on-change flag
IOCAF.1 = 0 ' Clear interupt-on-change flag
#ENDIF

@ INT_ENABLE IOC_INT ; Interrupt-on-Change interrupt

Main:
' Check if motor RPM has changed
IF MotorRPM <> Old_RPM Then
Old_RPM = MotorRPM
GOSUB ChngMotorHPWM
EndIF

If ValueDirty = 1 THEN
TimeToSave = TimeToSave - 1
If TimeToSave = 0 THEN ' Time to save ?
WRITE EE_MotorRPM, MotorRPM

ValueDirty = 0 ' Clear the flag

#IFDEF USE_LCD_FOR_DEBUG
HSEROUT [LCD_INST, LCD_CLR]
pause 5
HSEROUT ["Saved RPM=", DEC MotorRPM, " ", 13, 10] ' Send text followed by carriage return and linefeed
#ENDIF
ENDIF
ENDIF

GOTO Main

' ************************************************** *************
' [IOC - interrupt handler]
' ************************************************** *************
RotEncAdjust:
#IFDEF USE_CUI_ROTARY_ENCODER
IF PORTA.0 = 0 THEN
RotEncDir = 1 ; CW rotation if B channel is 0
ELSE
RotEncDir = 0 ; CCW rotation
ENDIF
#ELSE
New_Bits = PORTA & (%00000011)

IF New_Bits = Old_Bits Then DoneRotEnc

RotEncDir = New_Bits.1 ^ Old_Bits.0
#ENDIF

IF RotEncDir = 1 Then
; CW rotation - increase speed but only to a max of 'MaxDuty'
IF MotorRPM < MaxDuty then MotorRPM = MotorRPM + 1
Else
' CCW rotation - decrease speed to a min of 0
IF MotorRPM > 0 Then MotorRPM = MotorRPM - 1
EndIF

ValueDirty = 1 ; Set dirty flag
TimeToSave = 50000 ; Set value to begin countdown to EEPROM save

DoneRotEnc:
Old_Bits = New_Bits

IOCAF.0 = 0 ' Clear interrupt flags
IOCAF.1 = 0

@ INT_RETURN

(I left out the rest of the code)

[HenrikOlsson]

In theory it should work... Perhaps the interrupt latency is just too large to be able to catch the correct state of the 'other' channel once you get to the ISR. One way to test that is to try it with the Grayhill encoder, then it should count every 4th click. If it does then it's a timing problem, if it doesn't it's something else.

Nice demonstration of conditional compilation!

/Henrik.

[Ioannis]

Maybe an DSO can help here. Do you have access to such an instrument?

By the way as Henrik noted, I liked the conditional compilation too. Great example!

Ioannis

[RossWaddell]

If you mean an oscilloscope, no I don't have one (I'd like one, mind, but they're pretty darned expensive).

And thanks for compliment, Henrik & Ioannis! Although I got the idea from a Darrel Taylor post, I believe - this forum is such an excellent source of ideas.

[HenrikOlsson]

From a price/performance standpoint the Rigol DS1052E is quite hard to beat at around $350 and a very popular entry level scope. A very handy tool to have when working with electronics.

[RossWaddell]

From a price/performance standpoint the Rigol DS1052E is quite hard to beat at around $350 and a very popular entry level scope. A very handy tool to have when working with electronics.
Closer to $470 here in Canada, I'm afraid.