ISR procedure length

Log in or register to post comments
Go To Last Post
13 posts / 0 new
Author
Message
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
#1
Posted by Psychlow: Tue. Dec 4, 2007 - 05:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Is there a limit to how large interrupt procedures can be (aside from the normal limits of flash RAM?)

Reason I ask, is because if I seem to make it too long (for example on my ADC interrupt, if I exceed about 10-15 lines of C code) the ADC conversion just seems to stop working altogether - I end up with zero on the conversion regardless of the actual input voltage.

I know you want to reduce interrupt code as much as possible to allow processing cycles for your main program, but I'm trying to figure out if it's just exceeding the interrupt's "size limit" or if I'm just programming a logic error somewhere.

Thanks

Tags:

Tools, Compilers and General Programming
Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104960 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Tue. Dec 4, 2007 - 05:31 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No there's no size limit but there can (very often) be a time limit (which is why ISRs should be lean and mean). If it's an ISR for an ADC why should it need to do anything more than collecting the value just read and stroing it into a ring buffer? All the "slow work" (especially things like printf()s and LCD stuff etc.) would then be done in main() that just processes the data when it gets the chance and notices that the ring is not empty (read/write pointers aren't the same)

Top
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
Posted by Psychlow: Tue. Dec 4, 2007 - 05:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson, thank you for the reply.

Yeah, definitely no polling or LCD code going on in the interrupt....

All I'm attempting to do in the ISR at this point is decide whether or not two measurements have been made successively on one channel (for averaging, to reduce noise susceptibility) and if so, store the averaged value, change to the next MUX channel and flop a couple of booleans. I wouldn't think that this would be faster than my ADC clock (which I have set to CLK/128).

I could probably move this logic to the main program code, but I figure changing the MUX in the interrupt would help to ensure that I'm not going to get another reading/interrupt before the MUX has been changed...

Top
glitch
glitch's picture
Level: Raving Lunatic
Joined: Sat. Jan 12, 2002
Posts: 8620 View posts
Location: Canada
Posted by glitch: Tue. Dec 4, 2007 - 05:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

my guess is that something else is going on in your code, and it's not the length of the ISR that's teh problem. Perhaps posting some example code showing what works and what doesn't will help us see what is going on.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

Top
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
Posted by Psychlow: Tue. Dec 4, 2007 - 06:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ADCref just switches between ADC1 and ADC2, while ADCcount increments on successive reads on the same channel.

vtarget and voutput should only change when 2 successive reads have occurred and the result has been averaged. Thinking about it now, I'm betting my error is in the math (dividing by two).

uint16_t volatile vtarget;
uint16_t volatile voutput;

uint16_t volatile vtargettemp;
uint16_t volatile voutputtemp;
uint8_t volatile ADCcount;
bool volatile ADCref = false;	//false = ADC2 (vtarget), true = ADC1 (voutput)


ISR(ADC_vect)
{
	if (ADCref == false)
	{
		if (ADCcount < 1)
		{
			vtargettemp = ADCW;
			ADCcount++;
		}
		else
		{
			ADCcount = 0;
			vtarget = (ADCW + vtargettemp) / 2;
			ADMUX = (1 << REFS0)|(1 << MUX0);	//Change to ADC1 (voutput) for next conversion
			ADCref = true;
		}
	}
	else
	{
		if (ADCcount < 1)
		{
			voutputtemp = ADCW;
			ADCcount++;
		}
		else
		{
			ADCcount = 0;
			voutput = (ADCW + voutputtemp) / 2;
			ADMUX = (1 << REFS0)|(1 << MUX1);	//Change to ADC2 (vtarget) for next conversion
			ADCref = false;
		}
	}
}

EDIT: I know I've got a few variables that can be static to the interrupt instead of shared, but I'm just sticking to what I know works for the moment until I get the rest figured out.

Top
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
Posted by Psychlow: Tue. Dec 4, 2007 - 07:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah, the more I look at this, the more I'm thinking I need to do a cast:

voutput = (ADCW + voutputtemp) / 2;

Maybe as:

voutput = (ADCW + voutputtemp) / (uint16_t)2;

?

Perhaps the unsigned int divided by the signed int is causing problems?

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104960 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Tue. Dec 4, 2007 - 10:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Err no.

Top
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
Posted by Psychlow: Tue. Dec 4, 2007 - 10:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Heh.

Then any idea what the issue is?

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 104960 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. Dec 5, 2007 - 11:03 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No, while the ISR code is a little convoluted (personally I'd have done it as an FSM with an enum tracking state and a switch statement) there's nothing I can see wrong there so I'm guessing your problem actually exists elsewhere in some unseen part of the program.

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3696 View posts
Posted by curtvm: Wed. Dec 5, 2007 - 03:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I could be wrong, but...

Aren't you reading the wrong pin after changing to the other mux? (assuming free running mode)

datasheet

Quote:
If these bits are changed during a conversion, the change will not go in effect until this
conversion is complete (ADIF in ADCSRA is set).
so does that mean that when you switch mux, the conversion of the previous pin is still in progress, and the next irq will actually be reading the wrong pin?

If not free running mode, then disregard, I think.

Top
Lajon
Lajon's picture
Level: Raving Lunatic
Joined: Fri. Mar 12, 2004
Posts: 2932 View posts
Location: Linköping, Sweden
Posted by Lajon: Wed. Dec 5, 2007 - 04:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
so does that mean that when you switch mux, the conversion of the previous pin is still in progress, and the next irq will actually be reading the wrong pin?

If not free running mode, then disregard, I think.


It is a valid concern (when using free running mode), there is some info about it in this thread: https://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=54206

But I guess that issue would not give the "zero on the conversion" problem OP is having.
/Lars

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3696 View posts
Posted by curtvm: Wed. Dec 5, 2007 - 05:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You could try something different for kicks. Should still show the same problem, though. Look around in your code for vtarget and voutput, as those are the variables you are apparently seeing as '0'.

uint16_t volatile vtarget,voutput;
ISR(ADC_vect)
{
    static uint16_t adc_temp;
    static uint8_t ADCount;
    
    if(!(ADCount & 0x01)){ //for 0 and 2
        adc_temp = ADCW;
    }
    else if(ADCount==1){
        vtarget = (ADCW + adc_temp) / 2;
        ADMUX = (1 << REFS0)|(1 << MUX0);
    }
    else if(ADCount==3){
        voutput = (ADCW + adc_temp) / 2;
        ADMUX = (1 << REFS0)|(1 << MUX1);
    }
    ADCount++;
    ADCount &= 0x03;
}
Top
Psychlow
Psychlow's picture
Level: Wannabe
Joined: Tue. Mar 1, 2005
Posts: 91 View posts
Posted by Psychlow: Wed. Dec 5, 2007 - 06:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curt, that is definitely a MUCH more efficient way of doing things, thank you.

I'll do some more investigation with the help you all have provided so far and see what comes up. It COULD be the ADMUX not updating before the next conversion starts, but if that were the case, I would expect to get a little different result set than I have been getting so far - and it was WORKING when I was only doing one conversion per channel.

Top