loosing encoder count

[Rickertsen2]

I am having a terrible problem where my encoders gradually loose track of position as if they are dropping ticks or something.

Is it possible that maybie the secret IFI stuff is using High priority interrupts that take a long time and are causing the LP ISR not to execute? I notice that occasionally characters printed to the terminal are dropped as well. Then again the terminal printing thing could be a buffer overrun caused by printing faster than the UART can transmit.

The relevant code is below:

#pragma code
#pragma interruptlow InterruptHandlerLow save=PROD,section(".tmpdata")
//#pragma interruptlow InterruptHandlerLow,section(".tmpdata")
void InterruptHandlerLow ()     
{
	HandleTurretInterrupts();
...

All you really need to look at here is the HandleTurretInterrupts() function.

/*
 * FILE: turret.h
 *
 * Responsible for turret limits, keeping
 * track of it's absolute position and performing the 'homing' sequence.
 */
//==================================================
#include "turret.h"
#include "boolean.h"
#include "hardware.h"
#include <stdio.h>
#include <p18f8722.h>

//--- private variables ---//
boolean pan_homed = false;
boolean tilt_homed = false;

//the current position of the pan and tilt axis
volatile signed int pan_position = 0;
volatile signed int tilt_position = 0;

//used for 'thread' synchronization probably unnecessary after i made the change to diable interrupts during access like kevin did in his code. Currently not used
volatile boolean panTicksBusy	= false;
volatile boolean tiltTicksBusy	= false;


//--- Private Prototypes --//
void Pan_Routine(void); //
void Tilt_Routine(void);

//-------Functions --------//

//used by all other pars of the code to set the pan axis speed.
void setPanSpeed(signed char speed){
	rc_turret_pan = oi_joy_turret_pan;
	
	//make sure we don't override the homin sequence
	#ifdef TURRET_AUTO_HOMING
		if(!pan_homed) return;
	#endif
	//limit range of motion
	#ifdef USE_TURRET_PAN_LIMIT
		panTicksBusy	= true;
		rc_turret_pan = ((rc_turret_pan<127)&&(pan_position<=PAN_MIN))? 127 : rc_turret_pan;
		rc_turret_pan = ((rc_turret_pan>127)&&(pan_position>=PAN_MAX))? 127 : rc_turret_pan;
		panTicksBusy	= false;
	#endif
}

//used by all other parts of the code to set the speed of the tilt axis.
void setTiltSpeed(signed char speed){
	rc_turret_tilt = oi_joy_turret_tilt;
	
	//make sure we don't override the homin sequence
	#ifdef TURRET_AUTO_HOMING
		if(!tilt_homed) return;
	#endif
	//limit range of motion
	#ifdef USE_TURRET_TILT_LIMIT
		tiltTicksBusy	= true;
		rc_turret_tilt = ((rc_turret_tilt<127)&&(tilt_position<=TILT_MIN))? 127 : rc_turret_tilt;
		rc_turret_tilt = ((rc_turret_tilt>127)&&(tilt_position>=TILT_MAX))? 127 : rc_turret_tilt;		
		tiltTicksBusy	= false;
	#endif
}

//Finds the 0 position of the turret based on the limit switches.
//The axis rotate in the direction of the limit switches until the limit switches are reached. When this happens, movement stops
//and the position cound is set to 0. 
void TurretHome(void){
	#ifdef TURRET_AUTO_HOMING
	//---pan---
	if(pan_homed){
		//do nothing
	}else{
		//find our home position
		if(rc_sw_pan_home){
			pan_homed 		= true;
			 panTicksBusy	= true;
			 pan_position	= 0;
			 panTicksBusy	= false;
			rc_turret_pan	= 127;
		}else{
			rc_turret_pan	= PAN_HOME_SPEED;
		}
	}
	//---tilt---
	if(tilt_homed){
		if(rc_sw_tilt_home){
			tilt_position = 0;	
		}
		//do nothing/**/
	}else{
		//find our home position
		if(rc_sw_tilt_home){
			tilt_homed 		= true;
			 tiltTicksBusy	= true;
			 tilt_position	= 0;
			 tiltTicksBusy	= false;
			rc_turret_tilt	= 127;
		}else{
			rc_turret_tilt	= TILT_HOME_SPEED;
		}
	}
	#endif
}
void TurretRoutine(void){
	TurretHome();
	#ifdef PRINT_TURRET_STATUS
		//printf("TURR pl:%u, tl:%u, ph:%u, th:%u, p:%d,		t:%d\n\r",rc_sw_pan_home,rc_sw_tilt_home,pan_homed,tilt_homed,pan_position,tilt_position);
		//printf("TURR pl:%u, tl:%u, pan:%d, tilt:%d\n\r",rc_sw_pan_home,rc_sw_tilt_home,pan_position,tilt_position);
		//printf("tilt:%d		pan:%d\n\r",tilt_position,pan_position);
		//printf("Pan:%d   	Tilt:%d\n\r",rc_turret_pan,rc_turret_tilt);
	#endif
}

//--- called by the ISR in user_routines_fast.c ---
void HandleTurretInterrupts(void){
	if(INTCON3bits.INT2IF) //this is a tilt encoder tick (rising edge of tilt A line)
	{
		INTCON3bits.INT2IF = 0; //clear the flag
		/*
		//we have an concurrent access error?
		if(tiltTicksBusy)
		{
			 printf("p!!!!!!!");	
			 return;
		}
		*/
		tiltTicksBusy = true;
		if(rc_dig_in03) {						// tilt B
			 tilt_position++;
		} else {
			tilt_position--;
		}
		tiltTicksBusy = false;
	}
	if(INTCON3bits.INT3IF) //this is a pan encoder tick (rising edge of pan A line)
	{
		INTCON3bits.INT3IF = 0; //clear the flag
		/*
		//we have an acces violation
		if(panTicksBusy){
			printf("p!!!!!!!");
			return;
			
		}
		*/
		panTicksBusy = true;
		if(rc_dig_in04) {						// pan B
			pan_position--;
		} else {
			pan_position++;
		}
		panTicksBusy = false;
	}
}
//-- return whether or not the axis' are homed and ready to use --
boolean isPanHomed(){
	return pan_homed;
}
boolean isTiltHomed(){
	return tilt_homed;
}

//--- configures the turrent encoders ---
void InitTurret(void){
	//--- initialize positions to 0 ---
	//panTicksBusy	= true;
	//tiltTicksBusy	= true;
	 pan_position	= 0;
	 tilt_position	= 0;
	//panTicksBusy	= false;
	//tiltTicksBusy	= false;
	/*==== ENCODER PINS ====
	 *Pan	A: INT2/RB2/digital input 01
	 *Tilt	A: INT3/RB3/digital input 02
	 *Pan	B: RB4/digital input 3
	 *Tilt	B: RB5/digital input 4
     */
	//--- setup pins as inputs ---
	rc_dig_iomode_01 = INPUT;
	rc_dig_iomode_02 = INPUT;
	rc_dig_iomode_03 = INPUT;
	rc_dig_iomode_04 = INPUT;
	
	//--- setup tilt encoder interrupt (INT2/RB2/digital input 01/pan A) ---
	INTCON3bits.INT2IF  = 0; //make sure flag is clear
	INTCON2bits.INTEDG2	= 1; //interrupt on low->high
	INTCON3bits.INT2IP	= 0; //make it low priority
	INTCON3bits.INT2IE	= 1; //enable it

	//-- setup pan encoder interrupt (INT3/RB3/digital input 02/tilt A) ---
	INTCON3bits.INT3IF =  0; //make sure flag is clear
	INTCON2bits.INTEDG3	= 1; //interrupt on low->high
	INTCON2bits.INT3IP	= 0; //make it low priority
	INTCON3bits.INT3IE	= 1; //enable it
	
	printf("encoders initialized\n");
	//OpenRB2INT(PORTB_CHANGE_INT_ON & FALLING_EDGE_INT & PORTB_PULLUPS_OFF);
	
}

//-- Functions to return the position of the axis --
signed int getPanPos(void){
	signed int temp;
	//panTicksBusy = true;
	INTCON3bits.INT3IE = 0; //temporarily diable
	 temp = pan_position;
	INTCON3bits.INT3IE = 1; //enable
	//panTicksBusy = false;
	return temp;
}

signed int getTiltPos(void){
	signed int temp;
	//tiltTicksBusy = true;
	INTCON3bits.INT2IE = 0;
	 temp = tilt_position; 
	INTCON3bits.INT2IE = 1;
	//tiltTicksBusy = false;
	return temp ;
}

[eugenebrooks]

Go to the IFI site and get the recently posted library that addresses a relatively serious problem with interrupts. This might fix your problem.

Eugene

[Mark McLeod]

In addition to Eugene's suggestion here are some random observations.

I assume the printf's are only there due to the prior dropped count problem, since they can delay you enough themselves to cause dropped counts.

You know you are purposely dropping a count whenever you get to "p!!!!!!!"

I also assume you are enabling low priority interrupts in general (GIEL)somewhere.

I might add "section("MATH_DATA")" to your pragma to be on the safe side, but it doesn't look like you need it the way the code appears now.

Your pan & tilt lines are reversed in code verses your later comments. Certainly crossing the B lines will drive you crazy, e.g.,

	/*==== ENCODER PINS ====
	 *Pan	A: INT2/RB2/digital input 01
	 *Tilt	A: INT3/RB3/digital input 02
	 *Pan	B: RB4/digital input 3
	 *Tilt	B: RB5/digital input 4
*/

Whenever you reverse direction or hover at one position your logic is susceptible to extraneous counts as the A line can flicker on/off.

[Rickertsen2]

Other things such as serial IO are using interrupts as well. It occurs to me that the GIEL flag is disabled while they are working. If an encoder interrupt occured during this time, would its flag be set and would it be correctly handled once the current interrupt handler finished? It looks like it would but i am a little unsure. I'm grasping at straws.

I will try the library update once i get to competition, but i don't think thats the problem.

Originally Posted by Mark McLeod

In addition to Eugene's suggestion here are some random observations.

I assume the printf's are only there due to the prior dropped count problem, since they can delay you enough themselves to cause dropped counts.

You know you are purposely dropping a count whenever you get to "p!!!!!!!"

yes

Originally Posted by Mark McLeod

Your pan & tilt lines are reversed in code verses your later comments.

hrmm they are... the code is right though.

Originally Posted by Mark McLeod

Whenever you reverse direction or hover at one position your logic is susceptible to extraneous counts as the A line can flicker on/off.

Thats a good point. Is there any way to disriminate this from legitimate ticks.

[Kevin Watson]

Originally Posted by Rickertsen2

Thats a good point. Is there any way to disriminate this from legitimate ticks.

Yes, have a look at how I've handled encoder channels 3-6 in my latest encoder code.

-Kevin

[Mark McLeod]

Originally Posted by Rickertsen2

Is there any way to discriminate this from legitimate ticks.

It's not an issue if you're counting many revolutions in one direction just to get a distance or counting the RPMs of a shooter wheel for velocity control, but for an application like pan where it mostly hovers in one position and is not turning your encoders at a steady rate very far in any one direction, it'll bite you.

The answer is simple enough as you think about what's happening. Here's an explanation for those anonymous readers who haven't caught on to the dynamics of what's going on.
The A-line goes high and you count it, because you know the direction for certain. It rolls back and the A-line goes low, but the code is written to ignore that transition. Not a problem if the encoder continued to move back and hit the previous rise of the A-line. It's only an issue when the encoder is poised right at the cusp of the transition from high to low. For instance, think about what happens if just general robot vibration causes the encoder to oscillate just a tiny bit back and forth over that transition point. If the line goes high, then back drives low (so to speak) and goes forward again it looks the same interrupt-wise as constant motion forward.

For this type of application no transition of the A line, high or low, can be safely ignored. Each transition of A must be enabled for both directions, going high at one spot and going low at that very same spot. Everywhere the code can add a count it also must be written to subtract a count, allowing for any backward motion of the A line as well as any forward motion at any single transistion point.

I imagine Kevin's encoder code to handle this issue must test and count both high and low transitions.

[Rickertsen2]

Originally Posted by Mark McLeod

It's not an issue if you're counting many revolutions in one direction just to get a distance or counting the RPMs of a shooter wheel for velocity control, but for an application like pan where it mostly hovers in one position and is not turning your encoders at a steady rate very far in any one direction, it'll bite you.

The answer is simple enough as you think about what's happening. Here's an explanation for those anonymous readers who haven't caught on to the dynamics of what's going on.
The A-line goes high and you count it, because you know the direction for certain. It rolls back and the A-line goes low, but the code is written to ignore that transition. Not a problem if the encoder continued to move back and hit the last rise of the A-line. It's only an issue when the encoder is poised right at the cusp of the transition from high to low. For instance, think about what happens if just general robot vibration causes the encoder to oscillate just a tiny bit back and forth over that transition point. If the line goes high, then back drives low (so to speak) and goes forward again it looks the same interrupt-wise as constant motion forward.

For this type of application no transition of the A line, high or low, can be safely ignored. Each transition of A must be enabled for both directions, going high at one spot and going low at that very same spot. Everywhere the code can add a count it also must be written to subtract a count, allowing for any backward motion of the A line as well as any forward motion at any single transistion point.

I imagine Kevin's encoder code to handle this issue must test and count both high and low transitions.

Thanks. I will rewrite my code to use the port B interrupt on change and count rising AND falling edges if i get to the regional and see that this is the cause of the issue. I didn't do this initially because i overlooked this problem and thought that both edges would just eat more processing time.

[Mike Bortfeldt]

James,

It sounds like Mark has probably identified your problem, however there are two other things that you might want to change. In your HandleTurretInterrupts routine, you need to check for the interrupt enable bit as well as the interrupt flag (INT2IF & INT2IE for example) in your IF statement. Disabling the interrupt enable, doesn't stop the interrupt flag from getting set, it only disables the processor from generating an interrupt in response to the flag. For example, say you have the pan interrupt disabled (INT3IE = 0) and you get a rising edge from both the pan and tilt encoders. Both the INT2IF and INT3IF flags get set, but only the INT2IF flag generates an interrupt. In your code, since you only look at the interrupt flag and not the enable bit, you will handle both interrupt flags even though you want to ignore the pan (INT3IF) one. If you check the interrupt enable flag, you can ignore the INT3IF flag and later, once your main code reenable the pan interrupt (INT3IE = 1), the pending interrupt flag (INT3IF) will immediately geneate a new interrupt and your code can handle it then. Second, if at all possible, I wouldn't use a print statement within an interrupt. You would be better off by incrementing a variable and then printing it from your main loop if it changes (or displaying it on the dashboard). You will get the same information but not delay interrupt processing.

Mike