Glitches Eliminated using Interrupt Context Saving, BUT ...

[mluckham]

Glitches Eliminated using Interrupt Context Saving, BUT ...


Can someone give me some advice?

I have written some pulse-measurement code for last years' wheel encoders, and along the way I discovered some things.

1) the CCP (Capture/Compare) feature that would have been the ideal solution cannot be used because there are no CCP input pins available on the full-size controller.

2) therefore I had to use pulse edge detection - interrupt on the leading edge of the pulse, capture the time, reprogram to interrupt on trailing edge, interrupt on trailing edge, capture the time again - subtracting the two times to get pulse width. To get the times I use TIMER3 programmed at 1/10 microsecond resolution.

3) all along (even before I had started working on the pulse measurement code) I had been seeing glitches on printf() output and also servo chattering. I'm using Kevin's interrupt-driven serial port code, but the printf() glitches were happening using the standard IFI serial port code also.

Suspecting some registers were getting clobbered by the interrupt service routines, I played around with the interrupt #pragma in user_routines_fast and managed to fix both problems ... but at a cost of substantial additional overhead - that affects both the minimum length of pulse I can measure, and the accuracy in measuring them.

I have some other interrupt-driven code that implements a wallclock timer using TIMER0, but disabling those interrupts does not fix the glitches - they are coming from elsewhere.

Here is the #pragma along with comments:

#pragma interruptlow InterruptHandlerLow save=PROD, section(".tmpdata"), section("MATH_DATA") /* You may want to save additional symbols. */
			// Dec2006 - failure to save section .tmpdata results in glitchy servos
			//         - failure to save section MATH_DATA results in glitchy printf
			//         - COST OF SAVING SECTIONS IS ABOUT 14 MICROSECONDS PER SECTION!!!
			//           WHICH AFFECTS MEASURING ENCODER PULSE LENGTHS
			//              minimum pulse measurable with save=PROD  is 14 usec
			//              adding section (".tmpdata")              is 27 usec
			//              adding section ("MATH_DATA")             is 40 usec

I'm happy to be rid of the glitches, but if I wasn't getting them in the first place I wouldn't need the extra #pragma sections and could measure smaller pulses.

Can anyone shed some light on this?

[dtengineering]

I don't have any specific answers to your question... however the approach our team is investigating is to use a custom circuit using a dedicated PIC to monitor the output from each gear tooth sensor. This allows you to oversample (provide one output, say, for every 20 gear teeth that go by... still plenty of precision, but 1/20th to work for the RC) and to simplify the output... IE pin1 goes high means the wheel went forward, pin 2 goes high, and it went backwards.

Initial tests look promising (they should... it is a fairly simple circuit, after all)... we're hoping to get something written up and posted here in the next few weeks.

Jason

[TimCraig]

Since you said last year's encoders, I assume you're using the PIC 8722 based 2006 controller. There are a number of silicon flaws in that chip that some of us never got our interrupt and timer routines to work properly with in spite of the published "fixes". I was using Timers 1 and 3 to do pulse with timing similar to what you're doing and at least one of the timers never worked properly. The same code executes perfectly on a PIC 8520 based 2005 controller however. At least one other team I know of had similar results. So my advice would be to try it on an 8520 before you blame your code.

[intellec7]

What if the timer overflows during this? I don't know if you are accounting for that in your code, but that would definitely cause glitches.Maybe if you reset the timer every edge, reading before the reset, that might work better.

[Kevin Sevcik]

I vote blaming your code, contrary to TimCraig's opinion. The glitches you're seeing are almost certainly due to data sections getting clobbered in your interrupt handler, as you suspect. The only solution is unfortunately to trim and tighten your handler code until you don't need to save those extra sections. Posting your entire handler would help in this endeavour. You can read the PIC C18 compiler user's guide to find out more about how context saving works and what you might be doing to need so much. Specifically, section 2.9.2.

[charrisTTI]

Yes, please post the code. The Allegro gear tooth counter generates a 45us pulse (38min-52max) for CW rotation and 90us pulse (76min-104max) for CCW rotation (relative to the sensor). 10 us resolution for the timer should be adequate which is 100kHz. If the timer is to be shared among multiple gear tooth sensors then you do not want to reset the timer, just let it free run. The port b interrupts may be adequate. They fire on both transitions.

[mluckham]

Great feedback, thanks for all suggestions.

I have it measuring pulses from both left and right encoders as small as 15 usec, with leading edges down to 33 usec apart. Interrupt service delay is 15-17 usec. However for these tiny pulses so close together, if both encoders interrupt near simultaneously, the second pulse is sometimes missed. In reality though with the larger 25 usec pulses, spaced MUCH farther apart (milliseconds) I don't expect much of an issue with missed pulses.

This is WITHOUT the extra sections saved in the #pragma and with zero print or servo glitches.

/*******************************************************************************
*
*	FUNCTION:		Left_Encoder_Int_Handler()
*	PURPOSE:		If enabled, the interrupt 1 handler is called when the
*					interrupt 1 pin changes logic level. The edge that the 
*					interrupt 1 pin reacts to is programmable (see comments
*					in the Initialize_Encoders() function, above)
*	CALLED FROM:	user_routines_fast.c/InterruptHandlerLow()
*	PARAMETERS:		None
*	RETURNS:		Nothing
*	COMMENTS:
*
*******************************************************************************/

//#define MEASURE_PERIOD_FREQUENCY
//#define MEASURE_ISR
#define DBGOUT_LPULSESTART		rc_dig_out16 = 1
#define DBGOUT_LPULSEEND		rc_dig_out16 = 0
#define DBGOUT_RPULSESTART		rc_dig_out15 = 1
#define DBGOUT_RPULSEEND		rc_dig_out15 = 0

void Left_Encoder_Int_Handler(void)
{
#ifdef PULSE_WIDTH_METHOD

#ifdef MEASURE_ISR
	unsigned int time_interruptend;
#endif
												// values to be preserved between calls to this handler
#ifdef MEASURE_PERIOD_FREQUENCY
	static unsigned long _PrevPulseStartLong = 0;
	static unsigned long _PulseStartLong = 0;
#endif

	unsigned int time_interruptstart;
	unsigned int time_pulselength;			// unsigned int for faster comparison
	static unsigned int _PulseStart = 0;


	// minimize function() calls to allow this routine to run as fast as possible
	// on a 40 MHz PIC, this code takes approx 13 microseconds
	// ************************************************************************************************
	// DEBUG BY PUTTING LOGIC ANALYZER ON DIGOUT16 (must be configured for OUTPUT in user_routines.c) *
	// WHICH MIMICS THE STATE OF THE OBSERVED PULSE                                                   *
	// ************************************************************************************************

	time_interruptstart = Timer_3_ReadTicks();

	if (INTCON2bits.INTEDG2 == LEFT_ENCODER_PULSE_EDGE)
	{
		// pulse start

		DBGOUT_LPULSESTART;

		_PulseStart = time_interruptstart;							// for pulse duration measurement

#ifdef MEASURE_PERIOD_FREQUENCY
		_PulseStartLong = Timer_3_ReadTicksLong();					// for pulse-to-pulse measurement
#endif

	}
	else
	{
		// pulse end

		DBGOUT_LPULSEEND;

		time_pulselength = (time_interruptstart - _PulseStart);			// units=tenths of microseconds

		encoder_pulselen = time_pulselength;	// for debugging

		// adjust encoder count, based on pulse lengths.  Manifest Constant is used because encoders
		// on left and right sides of robot probably rotate in opposite directions.

		if (time_pulselength > PULSE_LENGTH_FORWARD)
		{
			// reversing

			_Left_Direction = -LEFT_ENCODER_FORWARD_DELTA;
			_Left_NetPulses -= LEFT_ENCODER_FORWARD_DELTA;
		}
		else
		{
			// moving forward

			_Left_Direction = LEFT_ENCODER_FORWARD_DELTA;
			_Left_NetPulses += LEFT_ENCODER_FORWARD_DELTA;
		}

		_Left_PulseLength = time_pulselength;

#ifdef MEASURE_PERIOD_FREQUENCY
		_Left_PulsePeriod = (_PulseStartLong - _PrevPulseStartLong);		// units=tenths of microseconds
		 _Left_PulseFreq = (_Left_PulsePeriod == 0) ? 0 : (10000000#/_Left_PulsePeriod);	// calculate Hertz (VERY SLOW CALCULATION!)

		// set up for next time
		_PrevPulseStartLong = _PulseStartLong;
#endif
	}

	// set to interrupt on opposite edge of pulse

	INTCON2bits.INTEDG2 = INTCON2bits.INTEDG2 ^ 1;		// flip the bit with Exclusive OR (XOR)

#ifdef MEASURE_ISR
	time_interruptend = Timer_3_ReadTicks();
	encoder_isrtime = (time_interruptend - time_interruptstart);
#endif

#else

	// The encoder's phase-A signal just transitioned
	// from zero to one, causing this interrupt service
	// routine to be called. We know that the encoder just
	// rotated one count or "tick" so now check the logical
	// state of the phase-B signal and increment or decrement
	// the _Encoder_Count variable.
	if (LEFT_ENCODER_PHASE_B_PIN == LEFT_PHASE_B_FORWARD)
	{
		Left_Encoder_Count += LEFT_ENCODER_FORWARD_DELTA;
	}
	else
	{
		Left_Encoder_Count -= LEFT_ENCODER_FORWARD_DELTA;
	}
#endif

}

#pragma code
#pragma interruptlow InterruptHandlerLow save=PROD /* You may want to save additional symbols. */
void InterruptHandlerLow ()     
{                               
  unsigned char int_byte;       

	if (PIR1bits.TMR1IF && PIE1bits.TMR1IE)	// wallclock.c timer interrupt?
	{
		PIR1bits.TMR1IF = 0;		// clear the interrupt flag
	    Timer_1_Int_Handler();
	}

	else if (PIR2bits.TMR3IF && PIE2bits.TMR3IE)	// measure.c timer interrupt?
	{
		PIR2bits.TMR3IF = 0;		// clear the interrupt flag
    	Timer_3_Int_Handler();		// counts the number of overflows
	}

	else if (INTCON3bits.INT2IF && INTCON3bits.INT2IE) // left encoder interrupt?
	{ 
		INTCON3bits.INT2IF = 0; // clear the interrupt flag [91]
		Left_Encoder_Int_Handler(); // call the left encoder interrupt handler (in encoder.c)
	}

	else if (INTCON3bits.INT3IF && INTCON3bits.INT3IE) // right encoder interrupt?
	{
		INTCON3bits.INT3IF = 0; // clear the interrupt flag [91]
		Right_Encoder_Int_Handler(); // call right encoder interrupt handler (in encoder.c)
	}

The glitches you're seeing are almost certainly due to data sections getting clobbered in your interrupt handler, as you suspect.

Indeed, there were two - but not in the encoder code.

I found and removed a % (MOD) operator in my wallclock code (it interrupts every millisecond). AND I took at look at the printf_lib.c code we had been using (modified to print LONG variables) before I installed Kevin's serial_port.c interrupt-driven code, and found it uses / 10 (divide by 10) to extract the lower-order digit.

So without the MOD and DIVIDE operators I was able to remove the both the section (".tmpdata") and section("MATH_DATA") context saves from the #pragma, and successfully measure the small pulses And still print without glitches, and no servo glitches - perfect!

What if the timer overflows during this? I don't know if you are accounting for that in your code, but that would definitely cause glitches.Maybe if you reset the timer every edge, reading before the reset, that might work better.

I am using Timer3 free-running at 1:1 clock rate (so 1/10 microsecond resolution) which interrupts on overflow to update an overflow counter. I don't want to reset the timer, because I want to measure pulses from multiple sources (two wheels, at least!) and don't want to dedicate a timer to each, even if there were enough of them.

I assume you're using the PIC 8722 based 2006 controller. There are a number of silicon flaws in that chip that some of us never got our interrupt and timer routines to work properly with in spite of the published "fixes".

I am debugging on the 8520 already. I will find out tonight whether the code transfers successfully to the 8722 controller.

use a custom circuit using a dedicated PIC to monitor the output from each gear tooth sensor.

The two-phase encoders with A and B pulses are dead simple to code (see Kevin Watson's encoder code) ... so I would think a custom electronic circuit should provide the same kind of output. I don't think there is an absolute need to oversample (report every 20 teeth, as suggested). You should be able to handle a thousand or two pulses per second - far more than a typical encoder on a wheel will output.

There are lots of places to use encoders on a robot, and a limited number of places to plug them in on the controller. If you are going to the trouble of writing a PIC preprocessor, why not have it handle multiple detectors?

The port b interrupts may be adequate. They fire on both transitions.

If I can do this it will make it possible to reduce the number of missed pulses, and speed up the ISR code due to not having to reprogram the edge detection.

[Mike Bortfeldt]

Your problem definitely sounds like it is somewhere within your interrupt code. Unfortunately without actually seeing your code, it is very difficult to guess where the problem may lie. While this may seem obvious, are you using a printf from within one of your ISRs to debug your code? If so that could be causing some of your problems. Also, as Tim Craig mentioned, make sure you are using all the updated code IFI posted as they do fix issues in the silicon (updated libraries, modified linker script). Unlike his team though, we never saw any problems after applying the updates that was not attributable to our own code.

You may find that even after these issues are resolved; your GTS code will not always count properly as there will be some uncontrollable delays in interrupt processing when the User Processor is communicating with the Master Processor that may throw off your pulse calculation. You may want to think of an alternative way to determine pulse width. Just for reference, as kind of a fun assembly language project, I used a PIC 12F508 (< $1) to calculate the pulse width and then toggle the state of a digital output on a forward pulse, and a different output on a reverse pulse. This allowed for total control over the timing and the output could be easily read by interrupts 3-6 on the RC. It worked fine during some initial bench testing, but as we didn’t use any gear tooth sensors on our robot, I never got a chance to fully test it. You are welcome to the code if you would like it.

Mike

Oops, a couple of minutes late replying...

[mluckham]

Never too late for good ideas Mike!

[Astronouth7303]

There are 4 CCP modules available as PWM13-PWM16, IIRC. You shouldn't be using them if you're using interrupts anyway.

The signals for PWM13-PWM16 are generated by the user proc, using Timer 0. If an interrupt fires while it's doing that, it screws up the signal.

If you go into User_Initialize(), there should be a comment about setting the pins for CCP mode.

[charrisTTI]

Using the port b interrupts will help.

Remove all of the extraneous calculations from the interrupt routine. These can be done in the main line code. ( or code redesigned so that it can be done there )

When accessing interrupt variables in main line code, be sure to disable/enable the interrupt. Otherwise, the data can change in the middle of your code. Keep the disable/enable short.

In unsigned math, overflow of the counter is not a problem, trying to measure a time period longer than the period of the counter is a problem. The difference between two unsigned counter values will be correct even if the first value is larger than the second value.

[mluckham]

Thanks for further comments and suggestions. This is a great discussion because it is important to have all these points discussed - writing interrupt handlers is not for the faint of heart

There are 4 CCP modules available as PWM13-PWM16, IIRC. You shouldn't be using them if you're using interrupts anyway.

I read somewhere that the CCP feature cannot be used for input capture, because (a) they are used for generating the outbound PWM signals for the motors, but more important (b) the EduRobot does have pin connections to the CCP lines on the PIC and you 'could' use them for input, but that the full-size controller' connections to external pins (PWM out) cannot be used as inputs due to buffer circuits. If true, this is one case where code could be developed on the EduRobot but would not work on the full-size controller. On the full-size, it is possible to use CCP for custom user-written PWM outputs only, but not for incoming Event Capture.

It's a moot point, but if the CCP was available, I could have measured pulses by configuring a timer and the CCP. External signal lines would be connected to pins leading to the CCP feature. The time at an incoming pulse edge would be captured in CCP registers and there would be a CCP interrupt. Very precise time measurement would then be possible, because the CCP hardware captures the time - no need to 'read a timer' in software, or to be subject to interrupt response latency. Another thing, there are 'only' four CCP devices available so only 4 incoming signals could be measured in this way.

When accessing interrupt variables in main line code, be sure to disable/enable the interrupt. Otherwise, the data can change in the middle of your code. Keep the disable/enable short.

And also use the 'volatile' directive where appropriate.

According to the PIC manual, when accessing byte (8-bit char) or word (16-bit integer) variables in mainline code it is not necessary to disable interrupts to obtain the complete contents of a variable because the PIC hardware transfers data in these units with single instructions. But for doubleword (32-bit long or float) it is necessary to disable interrupts, lest the mainline code receive the first word - then there is an interrupt that changes the long value - then the mainline code receives the second word.

The best technique is to use object-oriented techniques and implement functions to retrieve variables modified by the interrupt handlers. The functions can contain the interrupt disable/enable code if needed. The variables themselves would be declared static in the interrupt handler .C file, and not directly readable except through the functions provided.

In unsigned math, overflow of the counter is not a problem, trying to measure a time period longer than the period of the counter is a problem. The difference between two unsigned counter values will be correct even if the first value is larger than the second value.

I've been meaning to do a test on this using the compiler 'unsigned int' type. The test would prove whether (2 - 65000) and (65000 - 2) produced the same result.

Remove all of the extraneous calculations from the interrupt routine. These can be done in the main line code. ( or code redesigned so that it can be done there )

Yes, that is a common technique, we are using it.

In our case the encoders are used to provide wheel speed information. The Process_Data_From_Local_IO() routine in user_routines_fast.c is called by the master processor very rapidly - I have heard "as fast as possible" or "every 2 msec" - in any case, this is not interrupt-driven code. We check the time and when 100 msec has elapsed, call a routine to calculate current wheel speed and acceleration in that time interval.

[BradAMiller]

I found the same issues with not saving those sections. And saw the same timing latency. It's pretty annoying for me - I was working on WPILib which allows user written interrupt handlers, so it has to save everything since it doesn't know what the user might do in their driver.

The gear tooth sensor was impossible to use for direction because there was no guarantee on the latency for the interrupt handler for the gear tooth sensor. And it was entering that interrupt service routine that started the timing. With the interrupt save/restore time being so long, and only one priority available to us, if there was another interrupt routine running when the gear tooth sensor interrupted, the time to get to the ISR was too variable to accurrately time the pulse.

Hence, no direction output available.

Brad

[dcbrown]

According to the PIC manual, when accessing byte (8-bit char) or word (16-bit integer) variables in mainline code it is not necessary to disable interrupts to obtain the complete contents of a variable because the PIC hardware transfers data in these units with single instructions.

Any value greater than 1 byte is at risk if it is also a variable changed within the interrupt routine and thus would need to be protected. This is because the PIC18F only has an 8bit data path so reading anything more than 1 byte requires multiple instructions. Interrupts should be disabled for 16 bit variable access if the variable is also possibly changed within the interrupt routine.

For example; there is a unsigned 16 bit clock variable that is updated so many times per second. The current value is 0x01FF. If we reference the variable, the code first reads say the lower 'FF' byte. During this read an interrupt is raised and serviced after the lower byte read but before the upper byte is read. The interrupt routine increments the clock variable count to 0x0200 and returns to user level code. The user level code then continues and reads the upper byte which is now '02'h. The user code would have just read a clock value of 0x02FF instead of 0x01FF. Without turning off interrupts to guarentee that all data bytes are read together there is no guarantee that something like this doesn't happen.

Some hardware registers like TMR1H:TMR1L have additional hardware to help reading and writing them in a manner that both upper and lower bytes are read/written together. These still are not protected if interrupts are not turned off across upper and lower byte access. There is still the chance that the interrupt routine could access the same registers and change the buffered values.

Bud

[mluckham]

Thanks for the latest couple of comments, especially Brad's. Although these PIC are extremely powerful and can be pushed a long way, there is a limit.

I did some testing with a logic analyzer where both channels were interrupting simultaneously with same-size pulses ... I'll have to do some more!

Our team has decided we will use quadrature encoders wherever possible.

For those that are interested, I have published the final encoder, measure, and wallclock code at http://www.sunsys.net/frc

Mike