Bit set inside an ISR - SOLVED

Go To Last Post
42 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have a mega328 with Timer0 triggering an interrupt every 10ms.  In the ISR I have the following:

 

ISR(TIMER0_COMPA_vect)
{
	TCNT0 = 0x49;
	delay = delay + 1
	ADCSRA = ADCSRA | 1 << ADSC;
}

Where I want to start a single ADC conversion, but when I compile the code I get an error:

Error    1    called object is not a function or function pointer   

 

And studio is telling me the issue is the ADCSRA line.

 

Since I have the AVR I/O.h included this should be a global variable, or do I have to declare this as volatile?

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

Last Edited: Tue. Jan 20, 2015 - 04:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Does semicolon after "delay + 1" help?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Missing semicolon cheeky.

 

Edit: Too slow.

And remember when you write ADCSRA, the compiler sees something like (*(volatile uint8_t *)(0x7A)).

(Which may look like a function call if you put something unterminated in front of it).

Last Edited: Sun. Jan 18, 2015 - 09:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A helicopter pilot got lost in a thick morning fog that occasionally happens in the Seattle winter.   He couldn't see the ground to find his location.  He found a skyscraper and hovered next to floor with people.  He wrote a sign saying "I'm lost. Where am I?".  The office workers quickly made a sign back saying: "You're in a helicopter!". 

 

 The grateful pilot knew immediately that he was over Microsoft headquarters in Redmond and was able to regain his bearings.  After all, who else but the creators of Microsoft software could take a simple and obvious question and give a stupid, completely useless, but absolutely truthful answer? 

 

Only the people who write compilers for a living and think that it's perfectly normal to give an error message like the one received for the obvious and common programming mistake of leaving off a semi-colon from a C code line.   Although the error message is technically correct, it is completely useless because it doesn't tell you what you need to know, which is what caused the error.  Instead it leads anyone who isn't a professional compiler writer on a wild goose chase.  Any good compiler would told the user that error was 99.9% likely to have been caused by a missing semi-colon in the previous line.

 

Unfortunately, even after 60 years of compiler research and development and having all the industry and financial structure of the world being based on these software programs, there are NO good compilers available at any price that give simple and useful error messages.  Least of all the free ones that everybody and his mother depend on to get the world's work done.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh for............. blush

 

Thank you

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Error messages get better when you switch from C to a high level language other than c??.

 

Part of the Problem is the C language with lots of things replaces by #defines and similar before the normal complier sees the code.
 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@Simonetta, I am sure glad the compiler Jim is using that gave the "error message like the one received" was not written by Microsoft!  Where do you get your facts from?

 

Simonetta wrote:
Any good compiler would told the user that error was 99.9% likely to have been caused by a missing semi-colon in the previous line.

 

This is what I get from the Microsoft C++ compiler in Visual Studio Community 2013 when a semicolon is missing:

Error 2 error C2146: syntax error : missing ';' before identifier 'c' ...

Oh, and that "good compiler" is free too.

"I may make you feel but I can't make you think" - Jethro Tull - Thick As A Brick

"void transmigratus(void) {transmigratus();} // recursio infinitus" - larryvc

"It's much more practical to rely on the processing powers of the real debugger, i.e. the one between the keyboard and chair." - JW wek3

"When you arise in the morning think of what a privilege it is to be alive: to breathe, to think, to enjoy, to love." -  Marcus Aurelius

Last Edited: Sun. Jan 18, 2015 - 11:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am going off topic on this. While the semicolon fixes the error, a delay inside the ISR may have side effects.

A comm on technique is in ISR, set next compare event by adding delay ticks to OCR0A, read ADC from last trigger, then trigger ADC. Let the timer take care of sampling interval.

It all starts with a mental vision.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

larryvc wrote:

nbsp;error C2146: syntax error : missing ';' before identifier 'c'

Not that agree with Simonetta ... but what is 'c' in your case? Is it something similar to ADCSRA? I.e., (*(volatile uint8_t *)(0x7A))?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ezharkov wrote:
Not that agree with Simonetta ... but what is 'c' in your case? Is it something similar to ADCSRA? I.e., (*(volatile uint8_t *)(0x7A))?

 

With the following code  (excuse the code, just pulled up a demo to test with)

#define ADCSRA (*(volatile unsigned char *)(0x7A))

int _tmain(int argc, _TCHAR* argv[])
{

 int a = 3, b = 4;
 decltype(a) c = a;
 decltype(a = b) d = a;
 ++c;
 ++d;
 cout << d << endl;
 ++d
 ADCSRA = 0xaa;

 return 0;
}

It gives this

Error 1 error C2064: term does not evaluate to a function taking 1 arguments ...
2 IntelliSense: expression preceding parentheses of apparent call must have (pointer-to-) function type ...

and the cursor lands right on the d of the ++d missing the semicolon.

"I may make you feel but I can't make you think" - Jethro Tull - Thick As A Brick

"void transmigratus(void) {transmigratus();} // recursio infinitus" - larryvc

"It's much more practical to rely on the processing powers of the real debugger, i.e. the one between the keyboard and chair." - JW wek3

"When you arise in the morning think of what a privilege it is to be alive: to breathe, to think, to enjoy, to love." -  Marcus Aurelius

Last Edited: Mon. Jan 19, 2015 - 12:22 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Simonetta wrote:
 Any good compiler would told the user that error was 99.9% likely to have been caused by a missing semi-colon in the previous line.

But the compiler doesn't know about "lines" - does it?

 

Look at the offending code:

jgmdesign wrote:

	delay = delay + 1
	ADCSRA = ADCSRA | 1 << ADSC;

which, to the compiler, is the same as

Quote:

	delay = delay + 1 ADCSRA = ADCSRA | 1 << ADSC;

and now you see that it's not at all clear that there is an obvious missing semicolon!

 

Further,  presumably,  ADCSRA is a #define of the form

#define ADCSRA	_SFR_IO8(0x06) 

So the code becomes

delay = delay + 1 _SFR_IO8(0x06) = _SFR_IO8(0x06) | 1 << ADSC;

Now the parenteses are the "call" operator in 'C' - so now we can see why it's talking about "called objects"...

 

This is another case where looking at the preprocessor output would help...

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

KitCarlson wrote:

I am going off topic on this. While the semicolon fixes the error, a delay inside the ISR may have side effects.

A comm on technique is in ISR, set next compare event by adding delay ticks to OCR0A, read ADC from last trigger, then trigger ADC. Let the timer take care of sampling interval.

Kit,

Not to get too deep into something that does not need to , I am not using a delay inside the ISR, I am incrementing a variable I call delay.  The 'delay' is just the name I used for the variable.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

You think missing semicolons yield arcane compiler error messages?

Wait 'till you get into C++ classes and inheritance!

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
and the cursor lands right on the d of the ++d missing the semicolon.
Indeed.  However nowhere in the issued error message appears the word 'semicolon'.  If you were the programmer who forgot to put in the semi in the first place, how likely is it that a flashing cursor next to where the semi should be will remind me of what you forgot to do?

 

The absence of a pointed error message is not the fault of the compiler.  As already mentioned, lines are of no importance to a C compiler.  A newline is (with a few exceptions) just whitespace.  A C compiler is in this respect hobbled by the standard which governs the language.

 

IMO this kind of thing is the purview of the IDE, and therefore (if anyone's) the responsibility those who write IDEs.  But the functionality in question amounts to an expert system trained to look for common programmer errors.  That really has no business in compiler proper.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jim,

 

Do you have Naggy and Margin of Error installed in your copy of AS6? If you did you would have seen this as you typed:

 

Now I know this error may not be the "obvious one" but surely if you saw this red underlying as you typed and as you hoevered over the yellow triangle you were given that description your first thought would be "what on earth is it talking about? Why has it turned red?". Hopefully you'd just be naturally drawn to look at the line above as it's often the case that "previous errors" have a knock on effect at which point you might have noticed that semi-colon.

 

To install "Naggy" and "Margin of Error" use Tools-Extension Manager...

 

Apart from "Margin of Error" Dean and Morten have written some other very useful tools in that gallery so I'd explore everything there.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Good idea! I will look into it.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am going off topic on this. ...

...and now I will as well.

 

ISR(TIMER0_COMPA_vect)
{
	TCNT0 = 0x49;

Let's just say that this is an "interesting" sequence.  Even out of context, I'm at a loss trying to think of a case where I'd want a timer to run from 0x49 to OCR0A.

 

And depending on the timer prescaler, the timer is likely to have moved past OCR0A by the time that the assignment to TCNT0 takes effect.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Actually I was thinking about the Naggy thing and presumably the missing semi-colon error would become evident earlier than before you have typed the entire ADCSRA line. In fact as you end the delay= line and start typing "A..." on the line below you see this:

 

 

Now hover the cursor on the first yellow triangle there and guess what:

 

 

So perhaps these IDE's and C compilers are not that bad after all? cheeky

 

BTW the "A" line there is showing a second error of "use of undeclared identifier 'A'" but that's because you have only just started to type ADCSRA. It's as soon as you have typed the whole of  ADCSRA (a known symbol) that all this clears to a single error about "called object is not a function or pointer"

 

For those that do not know the way Naggy works is that it actually compiles the code as you type. It doesn't use avr-gcc, instead it uses the Clang/LLVM compiler (that understands GCC syntax) and just feeds the "text so far" to that, compiles it then uses the error output to decide whether to put a squiggly red line under what you have just typed. Bloody clever stuff if you ask me - like interactive/interpreted C!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jgmdesign wrote:
I have a mega328 with Timer0 triggering an interrupt every 10ms.  In the ISR I have the following:

 

ISR(TIMER0_COMPA_vect)
{
	TCNT0 = 0x49;
	delay = delay + 1
	ADCSRA = ADCSRA | 1 << ADSC;
}

Where I want to start a single ADC conversion,

As a rule, assigning to a timer count is evidence of bad design.

CTC mode would seem simpler.

If you set your ADC flags right, there is no need to explicitly trigger a conversion within the ISR.

Moderation in all things. -- ancient proverb

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No, this works just fine.  The ADC does is conversion and then sits and waits for another trigger that the ISR provides.  The ADC is set up for single conversion mode. and the timer is indeed in CTC mode.

 

AS I said earlier, I should use a different descriptor for the variable 'delay' and have changed it accordingly.  The variable is for something else.

 

Thanks for the input.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

and the timer is indeed in CTC mode.

Then why, in in CTC mode with TCNT0 cleared on compare match, are you setting TCNT0 to 0x49?

 

And while it will indeed "work fine", many AVR models have auto-trigger on the ADC, thus eliminating jitter due to interrupt-servicing latency.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

skeeve wrote:

 

jgmdesign wrote:

I have a mega328 with Timer0 triggering an interrupt every 10ms.  In the ISR I have the following:

 

 

ISR(TIMER0_COMPA_vect)
{
	TCNT0 = 0x49;
	delay = delay + 1
	ADCSRA = ADCSRA | 1 << ADSC;
}

Where I want to start a single ADC conversion,

As a rule, assigning to a timer count is evidence of bad design.

 

CTC mode would seem simpler.

I wondered about that too.  I've never seen it before, that is, setting TCNT in CTC mode.

 

I've seen setting TCNT in overflow mode (not good)

I've seen adding to TCNT in overflow mode (better but not perfect)

And I've seen straight CTC mode (perfect in that no clock ticks are ever lost)

 

But I've never seen #1 and #3 combined.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah, in the original post that number was there.  THat was an error I found after I got the code to compile then run.  It actually now reads:

 

TCNT0 = 0x00;

and it is the last line of the ISR to make sure the counter is at zero when it exits the ISR.

 

I should have posted that after the solution(the missing semicolon) was brought to light.

 

Thanks for pointing that out.  I should have cleared it up.

 

And while it will indeed "work fine", many AVR models have auto-trigger on the ADC, thus eliminating jitter due to interrupt-servicing latency.

I only want a single conversion, and I wanted it every 10ms.  I am curious as to where there would be jitter when the ISR simply starts a conversion and exits? The ADC result is read in another routine when the conversion is complete.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

and it is the last line of the ISR to make sure the counter is at zero when it exits the ISR.

And why is that?  If indeed your goal here is to start an ADC conversion every 10ms, then you are virtually forcing jitter in the 10ms period.

 

I only want a single conversion, and I wanted it every 10ms. 

Then use auto-trigger.

 

Now, in a [trivial] test program, and/or with a small ISR and a large prescaler, you will indeed have a fighting chance of more-or-less exactly 10ms, most of the timer or nearly all the time.

 

In a [full] real app, with other interrupt sources enabled and varying interrupt servicing latency, you have a slim chance of multiple exact periods.  In other words, jitter.

 

In many apps it may not matter if there is a little jitter.  But it appears that you desire (if not require) regular sampling.  If so, then AVR models such as yours have ADATE designed exclusively for you.  (Other 'Freaks are only borrowing your feature.)

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok Lee,

The purpose of the lat line is to make sure the counter is reset to zero on ISR exit.

 

I don't want/need the ADC to continuously auto-trigger.  it completes the conversion and in the micro's world sits idle for an eternity until the next trigger comes in, which is fine.

 

And yes, this is a trivial application and the 10ms is not an exact requirement as the user will never notice if the end result is 10ms, or even 30ms off for that matter.  it's just a simple proof of concept for something else.  That's all.

 

if it were a time critical app then I agree this is not the ideal solution by any means.  Like I just said, it's a little demo.

 

(Other 'Freaks are only borrowing your feature.)

?????

 

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

(Other 'Freaks are only borrowing your feature.)

?????

ADATE was added >>just for you<<, for this application.  We all are just borrowing "your" feature when we use it.

 

I don't want/need the ADC to continuously auto-trigger.  it completes the conversion and in the micro's world sits idle for an eternity until the next trigger comes in, which is fine.

Now it is my turn for "?????".  From the code you have shown, and the discussion, you apparently want to trigger the start of an ADC conversion every 10ms. ADATE is really more for "repeated auto-trigger".  (What do you mean by "continuously auto-trigger"?)

 

The respondents here are still curious about the assignment to TCNT0 in the compare-match ISR, especially in the light of the fact that CTC is being used.  As mentioned, in a full app this will introduce jitter.

 

I don't think you've explored

 

Table 24-6. ADC Auto Trigger Source Selections
ADTS2 ADTS1 ADTS0 Trigger Source
0 0 0 Free Running mode
0 0 1 Analog Comparator
0 1 0 External Interrupt Request 0
0 1 1 Timer/Counter0 Compare Match A
1 0 0 Timer/Counter0 Overflow
1 0 1 Timer/Counter1 Compare Match B
1 1 0 Timer/Counter1 Overflow
1 1 1 Timer/Counter1 Capture Event

 

Would you like me to mock up some code? ADATE is a nice useful feature that wasn't available during the Mega8 era.

 

// Timer/Counter 0 initialization
// Clock source: System Clock
// Clock value: 7.200 kHz
// Mode: CTC top=OCR0A
// OC0A output: Disconnected
// OC0B output: Disconnected
// Timer Period: 10 ms
TCCR0A=(0<<COM0A1) | (0<<COM0A0) | (0<<COM0B1) | (0<<COM0B0) | (1<<WGM01) | (0<<WGM00);
TCCR0B=(0<<WGM02) | (1<<CS02) | (0<<CS01) | (1<<CS00);
TCNT0=0x00;
OCR0A=0x47;
OCR0B=0x00;
...assumes 7.3728MHz AVR clock
... and that you set up your ISR
...
... ADC setup with ADATE; arbitrary reference and ADC clock
// ADC initialization
// ADC Clock frequency: 115.200 kHz
// ADC Voltage Reference: AREF pin
// ADC Auto Trigger Source: Timer0 Compare Match
// Digital input buffers on ADC0: On, ADC1: On, ADC2: On, ADC3: On
// ADC4: On, ADC5: On
DIDR0=(0<<ADC5D) | (0<<ADC4D) | (0<<ADC3D) | (0<<ADC2D) | (0<<ADC1D) | (0<<ADC0D);
ADMUX=ADC_VREF_TYPE;
ADCSRA=(1<<ADEN) | (0<<ADSC) | (1<<ADATE) | (0<<ADIF) | (1<<ADIE) | (1<<ADPS2) | (1<<ADPS1) | (0<<ADPS0);
ADCSRB=(0<<ADTS2) | (1<<ADTS1) | (1<<ADTS0);
.. and then set up an ADC-complete ISR

 

If you have nothing else to do in the ISR, then the ISR isn't really necessary.  It appears you do have other stuff such as your "delay" counter.  Fair enough.  If you run without the ISR then you need to clear the compare match flag manually; most generally that is done in the ADC-complete ISR (or when you pick up the result if polled.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

all this talk about his jitter being too large...

But we don't know what his jitter requirements are do we?

 

I see premature optimizing.

Keith Vasilakes

Firmware engineer

Minnesota

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The respondents here are still curious about the assignment to TCNT0 in the compare-match ISR, especially in the light of the fact that CTC is being used. 

Let them be confused then, the OP was about my overlooking not putting in a ';'

 

I don't think you've explored

 

Table 24-6. ADC Auto Trigger Source Selections
ADTS2 ADTS1 ADTS0 Trigger Source
0 0 0 Free Running mode
0 0 1 Analog Comparator
0 1 0 External Interrupt Request 0
0 1 1 Timer/Counter0 Compare Match A
1 0 0 Timer/Counter0 Overflow
1 0 1 Timer/Counter1 Compare Match B
1 1 0 Timer/Counter1 Overflow
1 1 1 Timer/Counter1 Capture Event

You are correct, I have not explored that, as I  just needed get this little demo running.  As I said TRIVIAL app.  Any jitter will never be seen.  If I end up doing anything more than this little display then I will certainly keep this in mind.

 

Would you like me to mock up some code?

Not necessary, but I see you could not help yourself smiley

 

 

all this talk about his jitter being too large...

But we don't know what his jitter requirements are do we?

  Exactly.  And I said there is a large margin of error that will never be seen.

 

I see premature optimizing.

OK.

 

Thanks for all the advice, but the little demo is running just fine for the intended use, and it has just been dropped off at UPS for delivery. 

 

Jim

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see premature optimizing.

Yahbut--

 

-- Now, in a real app, OP will have in the back of the mind about fussing with TCNTn

-- OP indeed was >>not<< familiar with the facilities of ADATE.  Now he is.

...and so on in a similar vein.  We don't know much (only inferred) about AVR's clock speed, and what the prescaler might be.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One of the advantages of ADATE is smaller simpler source code.

If timer0 is in CTC mode, the assignment to TCNT0 is gratuitous.

Moderation in all things. -- ancient proverb

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, now that I have a few minutes, I read up on the ADATE feature, other than it saves the line in my ISR that starts the conversion, and as such saves a few clock cycles which in a critical application my be needed I see nothing different as far as the line that starts a conversion I used.  If I use ADATE, it's doing the trigger in the background, using the way I did it, the ISR does it.

 

Now where I will agree that there is a line that is not needed is the setting of TCNT0 to 0x00 as the last line in the ISR.  It really is not needed.

 

Thanks

 

 

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

as a sidenote...

ADCSRA = ADCSRA | 1 << ADSC;

this probably will give  good code or you would have found troubles already, but i find it interesting that nobody tolled that for readability one better uses brackets and write the code like:

 

ADCSRA = ADCSRA | (1 << ADSC);

or perhaps even:

ADCSRA |= (1 << ADSC);

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, now that I have a few minutes, I read up on the ADATE feature, other than it saves the line in my ISR that starts the conversion, and as such saves a few clock cycles which in a critical application my be needed I see nothing different as far as the line that starts a conversion I used.  If I use ADATE, it's doing the trigger in the background, using the way I did it, the ISR does it.

The point you appear to be missing though is that using either ADATE or a timer in CTC and NOT touching TCNT leads to a jitter free solution. Messing with TCNT will introduce jitter as the entry to an ISR will jitter (because the AVR finishes a 1/2/3/4 cycle opcode before entry) and you are then tying the timer to the ISR entry time.

Last Edited: Tue. Jan 20, 2015 - 09:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see that Cliff, what I did say later on was that the line:

TCNT0 = 0x00;

was indeed not needed and should be removed, thus reducing the jitter.  My point is that with the removal of that one line, and leaving the rest of the ISR as is, essentially is the same as using the ADATE feature no?

 

 

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ISR as is, essentially is the same as using the ADATE feature no?

 

Agreed, it has the same functionality.  As mentioned, in a full app and desiring to sample a line at regular intervals ADATE will give a jitter-free sampling interval.

 

Hmmm--I can recall only using ADATE a couple times over the years; now, why is that?  I guess that in nearly all of my production apps that use ADC I have more than one channel, so channel selection might get a bit more hairy.  And when I have a "don't care" app, small app that repeatedly samples e.g. raw power looking for loss, I will just poll the ADC when the timer tick flag is set.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Jan 20, 2015 - 02:29 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmmmm,

This has me thinking as well. 

QUESTION:

Let's say I set this up to use ADATE and set up the Timer to run in CTC mode(I have to anyway to use ADATE) there should be no problem with jitter or anything else by my putting two or three lines of code in the Timer ISR correct?  for example my 'delay' variable being incremented by 1 each time the ISR is launched.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Right.  As long as your total possible latency to get to the ISR is less than the timer period.  In your case, 10ms right?

 

(As I mentioned above, if you don't have an ISR then you'd need to clear OCF0A manually.  Typically, that would be done in the ADC complete ISR, or when the ADC result is "picked up".)

 

10ms is indeed forgiving.  The discussion becomes much more pertinent at 1ms/1ksps or say 250us/4ksps.  Then e.g. a USART interrupt could skew start of sample by some tens of microseconds. 

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Let's say I set this up to use ADATE and set up the Timer to run in CTC mode(I have to anyway to use ADATE) there should be no problem with jitter or anything else by my putting two or three lines of code in the Timer ISR correct? 

More than that, there will be no jitter ever. With starting the conversion inside the ISR there is always a chance of jitter since there is no guarantee that the ISR will be called immediately when the compare match happens. If you have other ISRs or there is any point in your code that you disable global interrupts, the ISR handling can be delayed by an indeterminate amount of time. With ADATE, the conversion is always started when the compare match happens, even if you have not enabled the ISR at all. 

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes in my case it is 10ms.  I also can see where one would use the ADC interrupt to clear the flag as opposed to the ISR. 

 

Ok, Looks like we have everything at a common understanding.

 

I am going to put SOLVED on the OP as I should have earlier.

 

Thanks everyone.

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I guess a summary for me is that modern AVR models have added features from previous generations.  Many are perhaps rarely used, but can be vary useful when the needs of the app dictate. 

 

One can write Mega88 code more-or-less like AT90S4433 code, or Mega8 code.  But starting with Mega8, now we have CTC in the timers, so much less fussing with TCNT in ISRs.  FOC bits and more timer modes.

 

For the ADC, ADATE adds more options than just "free running".  Still the same old primitive ADC, ;) 10 bits and slow, but a few more toyz to play with.

 

Now we have a USART, and it now can be an SPI master with some buffering.  Again, useful in certain situations.  The new PB has "start bit detection" to awaken from sleep modes with UART activity.  Yes, it could be done before but was a lot trickier.

 

'4433 datasheet:  80 pages of "test" before the tables/graphs at the end.  Mega8 240 pages.  Mega88 300 pages.  I'm sure that indeed some of the growth has to do with more explanation, but I'm also sure that some of it has to do with more features such as those mentioned above. 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I also can see where one would use the ADC interrupt to clear the flag as opposed to the ISR. 

To clear what flag? If you mean the compare interrupt flag, this is not necessary. The triggering of the conversion is not triggered by the compare match flag, it is triggered by the compare match itself. The only thing that is needed is that you start the timer (assuming that you don't do something silly that prevents the timer from actually reaching the value that the OCR0A register is set to).

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

To clear what flag? If you mean the compare interrupt flag, this is not necessary. The triggering of the conversion is not triggered by the compare match flag, it is triggered by the compare match itself.

Now you are going to make me dig.  IIRC, and that's why I repeated the mention of it, the conversion will only start when the flag OCF0A in this case gets set.  Not on the compare match itself.

 

 

Alternatively, a conversion can be triggered automatically by various sources. Auto Triggering is enabled by
setting the ADC Auto Trigger Enable bit, ADATE in ADCSRA. The trigger source is selected by setting the ADC
Trigger Select bits, ADTS in ADCSRB (See description of the ADTS bits for a list of the trigger sources). When
a positive edge occurs on the selected trigger signal, the ADC prescaler is reset and a conversion is started.
This provides a method of starting conversions at fixed intervals. If the trigger signal still is set when the
conversion completes, a new conversion will not be started. If another positive edge occurs on the trigger signal
during conversion, the edge will be ignored. Note that an Interrupt Flag will be set even if the specific interrupt is
disabled or the Global Interrupt Enable bit in SREG is cleared. A conversion can thus be triggered without
causing an interrupt. However, the Interrupt Flag must be cleared in order to trigger a new conversion at the
next interrupt event.

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.