Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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:



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?

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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. :p





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!



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 am debugging on the 8520 already. I will find out tonight whether the code transfers successfully to the 8722 controller.


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?



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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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...

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

Never too late for good ideas Mike!

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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 :eek:



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.



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.



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.


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.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

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

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

[This is in reply to WPILib and interrupt latency issue re: GTS sensor]

One way of simulating multiple levels of interrupt priority is to utilize multiple ISRs tied to through the same interrupt vector.

static unsigned char slow_isr = 0;
static unsigned char slow_isr_0 = 0;
static unsigned char slow_isr_1 = 0;
static unsigned char slow_isr_2 = 0;
:
static unsigned char slow_isr_n = 0;

#pragma code
#pragma interruptlow InterruptHandlerLow save=PROD
void InterruptHandlerLow ()
{
static unsigned char fast_isr;

// check "fast" interrupts here, like gts or other
// h/w time critical interrupts
// "fast" interrupts only use trusted interrupt drivers
// if "fast" interrupt serviced, set fast_isr flag
// these drivers will be from a trusted source, such
// as the default system drivers provided by the library.

<appropriate code for "fast" devices goes here>

// if we serviced a "fast" interrupt, return now.
// if there is another interrupt pending, we'll come right back.
if (fast_isr != 0) return;

// No fast interrupts processed, are there any "slow" ones pending?

// Now on to "slow" interrupt services
// determine if or which isr flags are set
// associated with "slow" type interrupt services,
// such as the system clock timer and capture
// them for the slow_isr routine's use/query.
<appropriate code to check, accumulate flag and clr IF>

// if slow isr routine is already running, then we're done - exit
if (slow_isr != 0) return;

// Turn interrupts back on and call
// the "Slow" service routine. Hardware
// dispatch of the h/w interrupt has occurred
// so we won't get immediately vectored again
// until the next interrupt.
while (0==0)
{
slow_isr = 0xFF; // show we're running slow isr
INTCONbits.GIEL = 1; // turn interrupts back on
InterruptHandlerLow_Slow(); // process bottom half of slow isr
INTCONbits.GIEL = 0;
if (slow_isr_0 != 0) continue;
if (slow_isr_1 != 0) continue;
:
slow_isr = 0;
break;
}
return;
}

#pragma code
#pragma interruptlow InterruptHandlerLow save=PROD, TABLAT, TBLPTR, section(".tmpdata"), section=("MATH_DATA")
void InterruptHandlerLow_Slow()
{
while (0==0)
// The only routines that can run while this
// routine is executing is high priority interrupt
// service routine and the low priority (fast) drivers.
if (slow_isr_0 != 0)
{
slow_isr_0 = 0;
<process interrupt>
}
if (slow_isr_1 != 0)
{
slow_isr_1 = 0;
<process interrupt>
}
:

// Ok, at this point we believe we've processed all the
// pending slow type interrupts, time to leave!
INTCONbits.GIEL = 0;
// Check one more time with interrupts off,
// to see if another slow device interrupted
// requested service while or after we processed the
// current batch...
if (slow_isr_0 != 0) continue;
if (slow_isr_1 != 0) continue;
:
// nope, no new slow isr requests pending...
// Turn interrupts back on, this is a gamble in that interrupts
// requesting slow isr service can happen now, but the problem is
// restoring context just takes too long. We'll check again after
// context is restored.
INTCONbits.GIEL = 1;
return;
}
}

In this way interrupts requiring low interrupt latency are handled pretty much right away, even the save=PROD may not be needed. If the device interrupt is to be service by a user written routine, then it's servicing is deferred to the slow isr section. A small amount of application programmers interface needs to be documented for slow isr driver writers since the h/w bits such as the interrupt flag have already been cleared for it during fast device processing, but it will work and prevent the additional save context required from effecting latency. Essentially with this method you have two levels of IPL with minimal latency for the class of "fast" devices requiring minimal latency.

Bud

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

Hmm - recursive logic - you are turning interrupts on as you process the 'slow' ISR, and deferring the return-from-interrupt at the end of an original invocation of InterruptHandlerLow (). Very hairy indeed ... if your sample is from running code, I bet you debugged it using an ICE (in-circuit emulator)!

Could you not accomplish the same effect without the recursion, by simply calling 'slow' ISR routines using your technique of the separate _Slow() routine which has it's own #pragma for saving additional data sections?

And how about checking for and servicing multiple interrupt sources during the same interrupt?


Responding to Brad, I finally 'got' what he was waying - that he started this timer when the pulse began. I didn't implement that way because I was always planning to measure multiple pulse sources and didn't want to devote a separate timer to each.

I see that this year's gear tooth sensors are the same as last year (well no, I haven't ready the documentation on them yet!!). Although our team is going with quadrature encoders, I see no reason why the pulse-measurement approach should be abandoned by everyone.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

As far as "custom" circuts go BaneBots this year is offering both encoders and an Encoder Divider Kit ($10.00), encoders are a little more. This would get you one signal line for forward, backward and another for a string pulses that which can be straight or divided. Link to Bane's First page http://www.banebots.com/c/FIRST
Good luck to all
Biff

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

Read the documentation and you'll find a reason. The Allegro sensor devices provided in the kit this year are not the fancy kind that give pulse-width-encoded information on gear tooth direction. They only provide tooth transition information. You will be able to measure how fast the gear is turning, but not which way.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

The sample code is only slightly recursive and only then to allow the fast isr to be called.

Q. Could you not accomplish the same effect without the recursion, by simply calling 'slow' ISR routines using your technique of the separate _Slow() routine which has it's own #pragma for saving additional data sections?

No. But I'll leave you to ponder why. Think latency. If you want more info, ask and I'll explain.

Q. And how about checking for and servicing multiple interrupt sources during the same interrupt?

Actually the code is ambiguous as to whether only one interrupt is processed or multiple. With the slow isr, all currently pending interrupts need to be serviced or they will be lost otherwise since they have already been dispatched (cleared) in hardware. In the fast isr it comes down to how many times overlapping interrupts are present with the time required to check for them vs the exit/entrance time into the isr. If overlapping interrupts only occur in 1:n cases where n is fairly large, then you are chewing up processor bandwidth for no gain.

As far as being hairy? Nah, after 3 decades of dealing with hardware this ain't hairy. Hairy is a 256bit ultrawide instruction word, 2 128bit input pipes, 1 256bit output pipe, 8-12 simultaneous operations, multi-delay branching all running at <10ns. Course that technology was 20 years ago now.

Grins,
Bud

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

Thanks Alan, I've read the sensor manual now. And noted that the Gyro and Accelerometer are to be mailed to us.


Does anyone think that any rule prevents use of last year's gear tooth sensors in this year's robot??


Bud - yes, your routine is quite clever and provides a two-priority interrupt priority scheme, that is not quite as recursive as I had thought on first glance, and that would reduce jitter caused by ISRs that consume larger quantities of instruction cycles (by placing less-frequently-called ISRs containing large code-blocks inside your slow_isr loop).

The result could be more accurate pulse-length measurements - the topic of this thread. Thanks for sharing the code with us.

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

Last year's GTS board is not available for purchase. It does not meet the requirement for an off-the-shelf component or assembly. But you could create your own equivalent custom circuit this year using the actual sensor from Allegro. You'd have to account for the cost of it, though (even if you got it donated).

Re: Glitches Eliminated using Interrupt Context Saving, BUT ...
 

To be even more clever...

Now that you have code executing asynchronously from user code driven by interrupts it is an easy matter to also add a general event handler, to say run something every 10ms or to run a routine every 10th time a sonar sensor completes a measurement... You can still starve off the real user code that is waiting for all this processing to complete, but it does make some things like collision detection a lot easier without having to poll and without further impacting interrupt latency. And besides, by the time slow isr is run you've already paid the penalty for saving off the .tmpdata and other sections.

I happen to have an asynchronous event handler written and bench tested...

Bud