ADC Interrupt code doesn't works!

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

Hi, I read [TUT] [C] Newbie's Guide to the AVR ADC..
Very good toturial, Now I can use ADC to read value but just in loop..
I use ATMega 8
Question 1.
I want to do it better and use Interrupt.
I have written a ADC driver, and Main code..
Very easy but this code doesn't do anything, and nobody of the LEDs turning on..
I think my program never run ISR function..

ADC Driver:

void ADC_init(uint8_t _ADPS2, uint8_t _ADPS1, uint8_t _ADPS0){
	if(_ADPS2 == 1){
		setHigh(&ADCSRA, ADPS2);
	}else{
		setLow(&ADCSRA, ADPS2);
	}
	if(_ADPS1 == 1){
		setHigh(&ADCSRA, ADPS1);
	}else{
		setLow(&ADCSRA, ADPS1);
	}
	if(_ADPS0 == 1){
		setHigh(&ADCSRA, ADPS0);
	}else{
		setLow(&ADCSRA, ADPS0);
	}
}

//Starts conversation
void ADC_start(){
	setHigh(&ADCSRA, ADSC);
}

void ADC_refs(uint8_t refs1, uint8_t refs0){
	if(refs1 == 1){
		setHigh(&ADMUX, REFS1);
	}else{
		setLow(&ADMUX, REFS1);
	}
	if(refs0 == 1){
		setHigh(&ADMUX, REFS0); 
	}else{
		setLow(&ADMUX, REFS0);
	}
}

//Enable ADC
void ADC_enable(void){
	setHigh(&ADCSRA, ADEN);
}

//Enbale ADC Interrupt
void ADC_enableInt(void){
	setHigh(&ADCSRA, ADIE);
}

And the main code:

int main (void) 
{ 
	//Setup I/O
	setOutput(&DDRB, 0);
	setOutput(&DDRB, 1);
	setLow(&PORTB, 0);
	setLow(&PORTB, 1);

	//Choose Prescaler
	ADC_init(1, 1, 1);
	//Choose REFS
	ADC_refs(0, 1);
	//Set ADLAR high for Left Adjust Result for easy to read
	setHigh(&ADMUX, ADLAR);
	//Set on Free-Running modus
	setHigh(&ADCSRA, ADFR);
	//Enable ADC
	ADC_enable();
	//Enable ADC Interrupt
	ADC_enableInt();
	//Enable global Inerrupt
	sei();
	//Start conversation
	ADC_start();

	while(1){
	}

}

ISR(ADC_vect){
	if(ADCH >= 127){
		setHigh(&PORTB, 0);
		setLow(&PORTB, 1);
	}else{
		setHigh(&PORTB, 1);
		setLow(&PORTB, 0);
	}
}

setOutput, setHigh and setLow is very simple function to choose output,
set value high or low and they works fine..

Question 2.
In ISR function, how is possible to check wich of MUX is running..
If I get value from ADC, how can I know this value is from PortC.x / MUX number x?

Last Edited: Thu. Jul 28, 2011 - 01:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

to check wich of MUX is running

If you are using multiple channels you don't want to use ADFR - trust me!

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

clawson wrote:
Quote:

to check wich of MUX is running

If you are using multiple channels you don't want to use ADFR - trust me!

Maybe I explain bad for you,
The thing is I going to connect detector to two ADC..
So 2detector DetecorA and DetectorB connect to ADC0 and ADC1..
And on ISR function I want to see on the moment I read the value, which detector is value from
and write it on LCD..
for example DetectorA have value 1V and DetectorB 3.4V..
That is why is important to know wich ADC input is running..

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

The way you handle two channel reading is that you set the MUX for one channel, you set the ADIE bit and then you set the ADSC bit. After 65us+ the interrupt will fire and you can take the reading from the first channel. Now set the MUX to the new channel (but leaving REFS bits in place) and set ADSC again and leave the interrupt. Get on with other work, 65us+ later the interrupt fires. Take the reading for the second channel. Change MUX back to the first channel, set ADSC, leave the interrupt. Rinse and repeat.

If you are simply toggling between two channels you can easily track which reading you just took but if in any doubt just read the MUX bit when you enter the ISR and they'll tell you which channel was selected that has triggered this interrupt.

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

Thanks clawson.. I think I understand what you mean..
But before that ADC-Interrupt must work (even for one channel)..
When I fix it so I can try your technik...

Have someone some idea how can I fix problem to question1? Thanks

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

I would check the watch dog Enable fuse if I were you..

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

ganeshskandha wrote:
I would check the watch dog Enable fuse if I were you..

In tutorial nothing stand about watchdog! But I tried what you say

setHigh(&WDTCR, WDE); //Enable WatchDog

But still no LED turning on / off..

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

Quote:

I would check the watch dog Enable fuse if I were you..

Wow that comes out of the left field - what makes you think WDT has anything to do with operation here or were you just trying to mislead OP?

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

I thot as the system is waiting in the while loop. It resets before the interrupt from ADC comes. Correct me if I am wrong.

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

Of I don't really understand it..
Now I use code from the tutorial..
https://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&t=56429
and the code works fine.. the LED turning on and off when I change potentiometer..

My code and the code from tutorial looks VERY similar,
but still my code doesn't work!

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

I think that one problem is the fact that you pass I/O ports as arguments to functions. Read this part from avr-libc: http://www.nongnu.org/avr-libc/u...

EDIT: Sorry I take it back. Looks like you are doing it the correct way!

EDIT 2: maybe you could show all the code, i.e. including setHigh and setLow functions?

/Jakob Selbing

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

jaksel wrote:
I think that one problem is the fact that you pass I/O ports as arguments to functions. Read this part from avr-libc: http://www.nongnu.org/avr-libc/u...

EDIT: Sorry I take it back. Looks like you are doing it the correct way!

EDIT 2: maybe you could show all the code, i.e. including setHigh and setLow functions?

Hi
I'm pretty sure about I/O methods works fine..
altso setOutput(), setHigh() and setLow().. becouse I have writing a lot of programs and they works like the should works..
But of course, I add my io driver here:

void setOutput(int *DDR, int PIN){
	*DDR |= _BV(PIN);
}

void setInput(int *DDR, int PIN){
	*DDR &= ~(_BV(PIN));
}

void setHigh(int *PORT, int PIN){
	*PORT |= (1<<PIN);
}

void setLow(int *PORT, int PIN){
	*PORT &= ~(1<<PIN);
}

And one other thing.. I have edit my main code..
I forget to put Free-Running Mode, but now is there,
BUT still, my code doesn't works

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

Your posted functions above mine should be 8 bit not int in the parameter lists . May not be the problem, but it would be correct code. Post the non-ISR version that works, and your OP code is for channel 0, so is your sensor connected to it ?

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

Last Edited: Thu. Jul 28, 2011 - 05:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If I was you I'd at least make them static inline functions or macros - though preferably simply throw them away - you are making it close to impossible for the compiler to use CBI/SBI just in the interest of obfuscating your code.

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

Quote:
I'm pretty sure about I/O methods works fine...
If yours is similar to the tut. but yours DOESN'T work, then your macros must be wrong somewhere. Try hooking an led up to light as soon as the ISR fires to see if CPU gets there.

You obviously don't know what the w.dog is, and so you shouldn't enable a system that you don't understand ( esp. not THAT one ! ) on someone else's say so , unless they give you a good reason to do it. Remove that line, quick :wink: ! What freq. is MCU running at ( this info should have be in your OP ) ? The interrupt header file must be in there or you would've gotten an error, so it's not that ...

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

I'm not sure why you are making your ADC initialization so obtuse. Take this, for example:

void ADC_init(uint8_t _ADPS2, uint8_t _ADPS1, uint8_t _ADPS0){
   if(_ADPS2 == 1){
      setHigh(&ADCSRA, ADPS2);
   }else{
      setLow(&ADCSRA, ADPS2);
   }
   if(_ADPS1 == 1){
      setHigh(&ADCSRA, ADPS1);
   }else{
      setLow(&ADCSRA, ADPS1);
   }
   if(_ADPS0 == 1){
      setHigh(&ADCSRA, ADPS0);
   }else{
      setLow(&ADCSRA, ADPS0);
   }
} 

First, it is a poor name for the function since it does not, in fact, init the ADC, it merely sets the ADC pre-scaler. Second, sending in the pre-scaler bits in separate variables is senseless. Someone looking at that function would have no clue what the values mean. It also makes the code in the function far more complex than necessary. If you really want a function that simply sets the pre-scaler (which is not a very useful thing to begin with), you could do something like this:

typedef enum
{
   ADC_Factor_2 = 0,
   ADC_Factor_4 = 2,
   ADC_Factor_8 = 3,
   ADC_Factor_16 = 4,
   ADC_Factor_32 = 5,
   ADC_Factor_64 = 6,
   ADC_Factor_128 = 7
} ADC_Prescaler;

void SetADCPrescaler(ADC_Prescaler factor)
{
   ADCSRA &= 0x07;
   ADCSRA |= factor << ADPS0;
}

// And use it like this
SetADCPrescaler(ADC_Factor_32);

Quote:
then your macros must be wrong somewhere.
But they are functions, not macros, which is why I don't like macros that look like functions.
Quote:
Your posted functions above mine should be 8 bit not int in the parameter lists .
And should be marked as volatile as well.

Regards,
Steve A.

The Board helps those that help themselves.

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

Oops, yes a macro would use #define directive. OP, I do my adc_init() as a 2 line write to the 2 main adc registers. I also have a bit more code in it to do a 1st dummy convert. I pass in parameters like

adc_init( ( REF_SOURCE | INITIAL_CHAN ), ADC_DIV )

and each of those three is #define in adc.h. You shouldn't use a function/macro to individually set multiple bits in a common config. reg. It adds to code size, it's more complex, etc.

Think about how hard it is for you to change config. combo. for the adc, you have to look up names/positions for each bit each time you make a change.

Quote:
And should be marked as volatile as well
*DDR, and *PORT, OP, not the 2nd param. of each list .

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

Your code confusing the Mega8... and it looks like secret code or somethings...

Just try one way at a time... (with simple and readable code of course)
ADC.0 result on PORT 0
ADC.1 result on PORT 1
Then multiplex it using interrupt...

And usually someone "misslook" at the LED polarity...

You said it very easy... but you make the code looks like you controlling a robot project...

Make code as simple as possible and more readable!

I don't know why I'm still doing this hobby

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

indianajones11:
You have right, the I/O driver was my first header I write, so very bad code maybe.. But I think uint_8 is better to use here.
I use AtMega8, PortC.0, and yes I think this is channel 0.
Since I just prototyping, I dont use sensor but I use potentiometer to give 0 - 5V to PORTC.0

You asking me about what happend when ISR fired, but ISR never fires, and nothing in ISR block runs..
I think that is the problem, ISR never runs, but I dont know why..

I dont know what is watchdog, yes. I think next topic I want to learn is the watchdog..

The mC have internal frquency.. and if I dont take wrong is 1Mhz.
I have all headers like interrupt.h or io.h etc. But I dont take it here just for save some place..
But I dont get any compiler errors...

Koshchi:
Im agree with you, ADC_init() is not the best name, but the ting is ADC_init() do other thins before
like enable ADC, but I remove Enable ADC to own function to be sure everything is fine..
I going to change it to ADC_prescaler() or something like that..

To all:
I know about maybe my I/O driver is not the good but until now work fine..
I write LCD driver and work.. and I use same driver to ADC without interrup, and get good result again..
But I know, of course I have some problem here with my code.. But I/O fcuntion have works fine until now..

I can post the orginal code from the toturial, I just change some port (PORTB for LED), and the works fine on mC..
SO THIS CODE WORKS: but this is from the toturial..

//This code is orginaly from penquissciguy 
//I just change some functions and PORTs and PINs.


#include  
#include  

int main (void) 
{ 
   setOutput(&DDRB, 0); // Set LED1 as output 
   setOutput(&DDRB, 1); // Set LED2 as output 

   ADCSRA |= (1 << ADPS2) | (1 << ADPS1) | (1 << ADPS0); // Set ADC prescaler to 128 - 125KHz sample rate @ 16MHz 

   ADMUX |= (1 << REFS0); // Set ADC reference to AVCC 
   ADMUX |= (1 << ADLAR); // Left adjust ADC result to allow easy 8 bit reading 

   // No MUX values needed to be changed to use ADC0 

   ADCSRA |= (1 << ADFR);  // Set ADC to Free-Running Mode 
   ADCSRA |= (1 << ADEN);  // Enable ADC 

   ADCSRA |= (1 << ADIE);  // Enable ADC Interrupt 
   sei();   // Enable Global Interrupts 

   ADCSRA |= (1 << ADSC);  // Start A2D Conversions 

   for(;;)  // Loop Forever 
   { 
   } 
} 

ISR(ADC_vect) 
{ 
   if(ADCH >= 127){ 
      setHigh(&PORTB, 0); 
      setLow(&PORTB, 1); 
   }else{ 
      setHigh(&PORTB, 1); 
      setLow(&PORTB, 0); 
   } 

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

MicroGyro wrote:
Your code confusing the Mega8... and it looks like secret code or somethings...

Just try one way at a time... (with simple and readable code of course)
ADC.0 result on PORT 0
ADC.1 result on PORT 1
Then multiplex it using interrupt...

And usually someone "misslook" at the LED polarity...

You said it very easy... but you make the code looks like you controlling a robot project...

Make code as simple as possible and more readable!

funny, Im robotic student :lol: you have right there..
Before this project, I use ADC without interrupt..
Altso start conversation..
Read the ADC number 0, write it on LCD,
and again (while loop).. and the works fine..

Now the next level for me is with Interrupt..

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

Guys sorry again for my bad code..
My background in programing is from Java SE and Java EE.. over 3years I have write code in Java..
So I have still the perspectiv from Java..
But I try to learn more C and with mC perspectiv

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

I tryed now just CUT all function and just use setHigh() function...

#include  
#include  
#include 

int main (void) 
{ 
	setOutput(&DDRB, 0); // Set LED1 as output 
	setOutput(&DDRB, 1);

	//Use AtMega8 with 1Mhz, Presclaer with 8 then we get 125Khz
	setHigh(&ADCSRA, ADPS1);
	setHigh(&ADCSRA, ADPS0);
	
	//AD AVCC and REFS
	setHigh(&ADMUX, REFS0)
	//Left Adjust Result for more easy read of value
	setHigh(&ADMUX, ADLAR);

	//Free-Running mod
	setHigh(&ADCSRA, ADFR);
	//ADC Enable
	setHigh(&ADCSRA, ADEN);
	//ADC Inerrupt Enable
	setHigh(&ADCSRA, ADIE);
	//Global Interrupt Enable
	sei();

	//Start Conversion
	setHigh(&ADCSRA, ADSC);
	
	while(1){

	}

} 

ISR(ADC_vect){
	setHigh(&PORTB, 0); 
}
 

The setHigh() & setOutput() function:

void setHigh(int *PORT, int PIN){ 
   *PORT |= (1<<PIN); 
} 
void setOutput(int *DDR, int PIN){ 
   *DDR |= _BV(PIN); 
} 
 

Put everything we need High, and if any signal comes, ISR turn on LED once to se ISR fires...
BUT the code doesnt work..
So i think problem is from setHigh() ???
WHY setHigh() works fine until now in other programs(like LCD, BlinkLED, ADC) but now doesnt??

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

Quote:

So i think problem is from setHigh() ???

Then why bother? It's rubbish anyway for the many reasons already listed.

EDIT: I just built this (for atmega8):

#include 
#include 

void setHigh(int *PORT, int PIN){
   *PORT |= (1<<PIN);
}
void setOutput(int *DDR, int PIN){
   *DDR |= _BV(PIN);
}

int main (void)
{
   setOutput(&DDRB, 0); // Set LED1 as output
   setOutput(&DDRB, 1);

   //Use AtMega8 with 1Mhz, Presclaer with 8 then we get 125Khz
   setHigh(&ADCSRA, ADPS1);
   setHigh(&ADCSRA, ADPS0);
   
   //AD AVCC and REFS
   setHigh(&ADMUX, REFS0)
   //Left Adjust Result for more easy read of value
   setHigh(&ADMUX, ADLAR);

   //Free-Running mod
   setHigh(&ADCSRA, ADFR);
   //ADC Enable
   setHigh(&ADCSRA, ADEN);
   //ADC Inerrupt Enable
   setHigh(&ADCSRA, ADIE);
   //Global Interrupt Enable
   sei();

   //Start Conversion
   setHigh(&ADCSRA, ADSC);
   
   while(1){

   }

}

ISR(ADC_vect){
   setHigh(&PORTB, 0);
}

and got:

../test.c: In function 'main':
../test.c:13: warning: passing argument 1 of 'setOutput' from incompatible pointer type
../test.c:14: warning: passing argument 1 of 'setOutput' from incompatible pointer type
../test.c:17: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:18: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:21: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:23: error: expected ';' before 'setHigh'
../test.c:26: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:28: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:30: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c:35: warning: passing argument 1 of 'setHigh' from incompatible pointer type
../test.c: In function '__vector_14':
../test.c:44: warning: passing argument 1 of 'setHigh' from incompatible pointer type

So (a) you posted code you've never successfully compiled (there's no way you can avoid the missing semi-colon) and (b) you clearly ignore possibly serious build warnings.

Last Edited: Fri. Jul 29, 2011 - 11:21 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Quote:

So i think problem is from setHigh() ???

Then why bother? It's rubbish anyway for the many reasons already listed.

I understand..
I want to use function for High and Low..
Can you give me one good example for this??
Do you self use function or Macro?
As you see Im very new in this..

BUT still I wonder WHY my old function works sometimes and sometimes NOT..

EDIT:
clawson:
honestly I never think about warnings..
YES, I get alot of them becouse of my function but everytime I get hex file to transfer to mC and the most of time, the works!
But I understand now, warning is more importent then I tink..

Can you give me one good example how should I do it?

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

I still hate the idea of these completely pointless functions but as you are making the huge mistake of defining them in the .h file you'll actually get away with it. Throwing the .h away I built this without any warnings or error and it works in the simulator:

#include 
#include 

void setHigh(volatile uint8_t *PORT, uint8_t PIN){
   *PORT |= (1<<PIN);
}
void setOutput(volatile uint8_t *DDR, uint8_t PIN){
   *DDR |= _BV(PIN);
}

int main (void)
{
   setOutput(&DDRB, 0); // Set LED1 as output
   setOutput(&DDRB, 1);

   //Use AtMega8 with 1Mhz, Presclaer with 8 then we get 125Khz
   setHigh(&ADCSRA, ADPS1);
   setHigh(&ADCSRA, ADPS0);
   
   //AD AVCC and REFS
   setHigh(&ADMUX, REFS0);
   //Left Adjust Result for more easy read of value
   setHigh(&ADMUX, ADLAR);

   //Free-Running mod
   setHigh(&ADCSRA, ADFR);
   //ADC Enable
   setHigh(&ADCSRA, ADEN);
   //ADC Inerrupt Enable
   setHigh(&ADCSRA, ADIE);
   //Global Interrupt Enable
   sei();

   //Start Conversion
   setHigh(&ADCSRA, ADSC);
   
   while(1){

   }

}

ISR(ADC_vect){
   setHigh(&PORTB, 0);
}
  

Note however that if you program this into an AVR and run it you will not be able to tell it is working as PORTB.0 (in the ISR) will be set high 151 cycles after you apply power and nothing further will be observed. So for all intents and purposes it will simply look like the LED comes on the moment you apply power (humans cannot "see" 151 cycles!) and after that nothing visible will change.

Quote:
honestly I never think about warnings..

Then I pray to God you never come to work for me. I'd want to employ professional engineers only.

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

clawson:
Thank you for all of the help..
I did what you say.. and everything works..
"Build succeeded with 0 Warnings..."

I change my I/O driver to this:

void setOutput(volatile uint8_t *DDR, uint8_t PIN){ 
   *DDR |= _BV(PIN); 
}

void setInput(volatile uint8_t *DDR, int PIN){ 
   *DDR &= ~(_BV(PIN)); 
} 

void setHigh(volatile uint8_t *PORT, uint8_t PIN){ 
   *PORT |= (1<<PIN); 
} 

void setLow(volatile uint8_t *PORT, int PIN){ 
   *PORT &= ~(1<<PIN); 
} 

And ADC with Interrupt works FINE :D

Quote:
Then I pray to God you never come to work for me. I'd want to employ professional engineers only.

haha, dont be so sure..
No but I thought warning was just *HINT* or something.. And I get all of my programs (until now) to work with my old function..
But I understand now, I must fix warning before is to late..
I think I get on time up to 50warning because of my bad function..
But thank you again..
Im very new in mC, and I learn thing on my own(no at University) so you going to see me more in this forum :P

To all:
Thanks for all the help..
Everything works fine now.. thanks

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

Quote:

No but I thought warning was just *HINT* or something..

Serious question: Even if you are trying to learn it yourself did your C manual not tell you how important it was to never ignore a warning?

You should never ignore any warning until you know exactly why it's being produced and usually you would still do whatever is necessary to prevent the warning occurring - professional code (for example the Linux kernel) builds (almost!) clean with no warning or errors.

In fact if you get a job programming professionally you may have to use a tool that is far far worse than the C compiler for showing warnings/errors and that is called "lint". It can find 50 things wrong with a .c file that the compiler gives no warnings or errors for. In a professional build it's likely that the compiler will not even be invoked until the code can first pass though lint without warning or error. So if you think things are bad at the moment just wait until you get a professional programming job.

(there is a GOOD reason why professional code must lint cleanly - we don't do it through pure masochism - lint finds many many implementation faults that might otherwise go un-noticed)

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

OT:

Quote:

we don't [lint] through pure masochism

But admit that you do moan slightly at times, ehhh? 8)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

Quote:

But admit that you do moan slightly at times, ehhh?

Yesterday I edited a CMakeLists.txt to change:

set(LINTING FALSE)

to

set(LINTING TRUE)

and the entire thing "blew up"! I know the code has been done wrong in that it should have been linting from day one - but part of the source was inherited and wasn't lintable. All I have to do now is make it lint. In fact not just lint but lint to MISRA rules - gulp!

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

ok, clawson thanks for your good explanation...

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

Quote:
I going to change it to ADC_prescaler() or something like that..
Don't, drop it entirely. There is no use in having several functions to change things that simply aren't going to change. Rarely does the reference voltage change dynamically in an app, and I have never seen an app that required the pre-scaler to be changed dynamically. And if either of those things really does need to be changed, you do not want to change them without disabling the ADC and re-enabling after.
Quote:
My background in programing is from Java SE and Java EE.. over 3years I have write code in Java..
The code is poorly designed no matter what the language. The biggest problem is sending in individual bits to set the values, not the names of the functions or how many there are.

Regards,
Steve A.

The Board helps those that help themselves.

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

indianajones11 wrote:
Your posted functions above mine should be 8 bit not int in the parameter lists . May not be the problem, but it would be correct code. Post the non-ISR version that works, and your OP code is for channel 0, so is your sensor connected to it ?

I'm not sure how the bytes would be assigned in an AVR, but if the wrong one is being assigned to the register, then it wouldn't work. He'd get 0 being assigned all the time.

Joe

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

arian88 wrote:
honestly I never think about warnings..
:shock: :shock: :shock:

/Jakob Selbing

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

I'm newbie too... and I still learning using ADC application...
I tried to multiplex the ADC inputs using interrupt... I knew it's not "good" but it works for lighting LEDs... with the interrupt...

I'm using ADC0 and ADC1 for the inputs... with VRs for adjust the voltages in each ADC input...
PortB3 and 4 for ADC0 result, PortB0 and 2 for ADC1 result...
here's the code I used...(CVAVR)

Chip type           : ATmega8
Program type        : Application
Clock frequency     : 4.000000 MHz
Memory model        : Small
External SRAM size  : 0
Data Stack size     : 256
*****************************************************/

#include 
#include 
#define ADC_VREF_TYPE 0x60


// ADC interrupt service routine
interrupt [ADC_INT] void adc_isr(void)
{
unsigned char adc_data;
// Read the 8 most significant bits
// of the AD conversion result
adc_data=ADCH;
// Place your code here
 
    PORTB.5^=1; 
}

void main(void)
{
      
// Input/Output Ports initialization
// Port B initialization
// Func7=Out Func6=Out Func5=Out Func4=Out Func3=Out Func2=Out Func1=Out Func0=Out 
// State7=0 State6=0 State5=0 State4=0 State3=0 State2=0 State1=0 State0=0 
PORTB=0x00;
DDRB=0xFF;

// Port C initialization
// Func6=In Func5=In Func4=In Func3=In Func2=In Func1=In Func0=In 
// State6=T State5=T State4=T State3=T State2=T State1=T State0=T 
PORTC=0x00;
DDRC=0x00;

// ADC initialization
// ADC Clock frequency: 31.250 kHz
// ADC Voltage Reference: AVCC pin
// Only the 8 most significant bits of
// the AD conversion result are used
ADMUX=ADC_VREF_TYPE & 0xff;
ADCSRA=0x8F;

// Global enable interrupts
#asm("sei")

while (1)
      {
     
//ADC0 result
    
    if (PORTB.5==0)
    {
    delay_us(50);
    ADMUX=0X60;
    if (ADCH<=127)
    {
    PORTB.0=1;
    PORTB.2=0;
    delay_us(100);
    ADCSRA=0xCF;
    }
    else
    {
    PORTB.0=0;
    PORTB.2=1;
    delay_us(100);
    ADCSRA=0xCF;
    }
    }
    
//ADC1 result
   
    if (PORTB.5==1)
    {
    delay_us(50);
    ADMUX=0x61;
    if (ADCH<=128)
    {
    PORTB.3=1;
    PORTB.4=0;
    delay_us(100);
    ADCSRA=0xCF;
    }
    else
    {
    PORTB.3=0;
    PORTB.4=1;
    delay_us(100);
    ADCSRA=0xCF;
    }
     } 
      };
}

I don't know why I'm still doing this hobby

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

If you are using interrupts why are you accessing ADCH in the main() code? Surely the whole reason to use interrupts is to red the value into a global (not a local as you have). Then access that global from the main() code?

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

clawson:

My bad with interrupt... :)

I'm accessing ADCH in "main" to determine the different ADC value for each ADC input, and take the results to light the LEDS... (I didn't know how to do it in the right way, it's just simple thought... :) )

I used the local for repeating the actions...

I'm still less knowledge with interrupt... :)

I don't know why I'm still doing this hobby

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

You have two choices. Either you learn about interrupts and use those for doing the ADC readings which lets the main() code get on and do other things while readings are being made. Or you forget interrupts all together and just have the main() code trigger the readings then wait for the result then continue on its way. The latter is probably easier for a beginner (and is well illustrated in the Tutorial Forum).

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

clawson:

Thanks for the info... :)
No tutorial in CVAVR code... so I had to translate the tutorial code one by one... (and hope it's done right)...
Anyway... thanks again for your attention... :)

I don't know why I'm still doing this hobby

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

Quote:

No tutorial in CVAVR code... so I had to translate the tutorial code one by one...

It is good to understand what is going on.

Now, have the Wizard generate an interrupt-driven ADC program for you ...

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.