Comparing two ADC values on an ATTiny13

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

Hi.
Been going at this all afternoon, and can't work out what I'm doing wrong.
I have a circuit that has two 10k potentiometers hooked into the ATTiny13. There are also two LEDs. What's supposed to happen is when one pot (the follower) is less than the other (the leader), the first LED lights up. If the follower is greater, the other LED lights up.

What's actually happening is that the second LED is staying lit most of the time, with the occasional flash to the first one.
I may have the entire ADC thing wrong without realising it, as I haven't had much experience with it.

Note: The ultra-long delays after the conversion are only there to make it clearer to me which LED was lit. They can be as short as needed once the code's debugged.

/* This code takes two analog inputs, converts them to digital
	between 0 and 255, then compares them. */
#include 

#define LED1 1 //PB1
#define LED2 0 //PB0
#define ALEAD 3 //PB3
#define AFOLLOW 4 //PB4

int lead, follow;

void delay(long int ticks) {
	unsigned long int countdown;

	while (ticks != 0) {
	for (countdown=0; countdown <= 200; countdown++); //  about 1ms @9mhz
	ticks--;
	}
}

int main(void) {
	//Setup LEDs
	DDRB |= (1 << LED1); //Output to LED1
	DDRB |= (1 << LED2); //Output to LED2

	//Setup ADC hardware
	ADCSRA |= (1 << ADPS0) | (1 << ADPS1); //Prescaler/8 for 125kHz 
	ADMUX |= (0 << REFS0); //Use Vcc as reference
	ADMUX |= (1 << ADLAR); //8-bit precision from ADCH

	ADCSRA |= (1 << ADEN); //Enable ADC conversion


	while(1){ //Forever
		ADMUX |= (1 << MUX1) | (1 << MUX0); //Switch to PB3
		ADCSRA |= (1 << ADSC); //convert ADC
		delay(30);
		lead = ADCH;

		ADMUX |= (1 << MUX1) | (0 << MUX0); //Switch to PB4
		ADCSRA |= (1 << ADSC); //convert ADC
		delay(30);
		follow = ADCH;
		
		if(follow < lead){
			PORTB |= (1 << LED1); //Turn on LED1
			PORTB &= ~(1 << LED2); //Turn off LED2
		}
		else{
			PORTB &= ~(1 << LED1); //Turn off LED1
			PORTB |= (1 << LED2); //Turn on LED2
		}
	}//End While
}//End Main
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

void delay(long int ticks) {
   unsigned long int countdown;

   while (ticks != 0) {
   for (countdown=0; countdown <= 200; countdown++); //  about 1ms @9mhz
   ticks--;
   }
}

If you have optimizaton enabled, the compiler will recognize this function as doing nothing usefull and therefore throw it into the trash can.

It's recommend to use the delay functions which come with your compiler. They're proved of not being optimized away. It looks like you're using WinAVR. In this case you find all relevant information here.

Regards
Sebastian

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

Where did you get the concept of (0<<..) ? It actually does very little and does not clear the required bit, thus with your adc stuff, you're not selecting the adc inputs you expect. Write yourself a little function called char getadc(char input) and have it select the correct input and add the adlar bit,start the conversion, poll the adc complete bit for completion the return the adc result. You will then get the result you expect.

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

S-Sohn wrote:
If you have optimizaton enabled, the compiler will recognize this function as doing nothing usefull and therefore throw it into the trash can.

It's recommend to use the delay functions which come with your compiler. They're proved of not being optimized away. It looks like you're using WinAVR. In this case you find all relevant information here.

Regards
Sebastian

I have turned the optimisation off. I read in another thread that including the library can make the code larger with optimization off, so I just used an example posted somewhere.
But thankyou anyway, I'll look into that and probably go with what you said once I have the core code done.

EDIT: I just remembered why I used delay. Further down the track, the program must be able to make one of the LEDs blink, and change the blink times according to the value of one of the Analog readings. Using the delay library causes massive bloat if the delay times aren't set at compile time.

Kartman wrote:
Where did you get the concept of (0<<..) ? It actually does very little and does not clear the required bit, thus with your adc stuff, you're not selecting the adc inputs you expect. Write yourself a little function called char getadc(char input) and have it select the correct input and add the adlar bit,start the conversion, poll the adc complete bit for completion the return the adc result. You will then get the result you expect.

I know that 0<< doesn't actually do anything, but I put it in there to make sure I knew what steps I had taken in setup, basically to stop me getting confused. When I do it in setting up the ADMUX register, it was just a result of lazy copy/paste.
I'll try writing the getadc function like you said. Only thing I'm worried about is space considerations. I suppose that's why I should be using the delay library and turning optimisation back on.

Thanks guys, I'll give those a go.
rmvvwls

Last Edited: Wed. Jun 30, 2010 - 01:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ADMUX |= (1 << MUX1) | (0 << MUX0);

This won't reset mux0, so you read the same adc channel both times. Would this explain your problem?

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

Kartman wrote:
ADMUX |= (1 << MUX1) | (0 << MUX0);

This won't reset mux0, so you read the same adc channel both times. Would this explain your problem?

... That's probably it.
How would I go about resetting mux0? I need it to switch between the two.
Would "ADMUX = ADMUX & 0b11111110;" work, seeing as MUX1 and MUX0 are the two least significant bits?

Edit: I just tried it, and it worked. However, I can see why it's a horrible way to doing it, as it depends on MUX1 (second-most right bit) being set to 1 before hand. Does anyone know of a sure-fire way to set whichever bits in a byte to exactly what I want?
I can't think of one, and google's not being very helpful.

Also Kartman, thankyou very much for pointing that out. I would not have seen that on my own.

Last Edited: Wed. Jun 30, 2010 - 01:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Suggestion: just rebuild ADMUX from scratch every operation. Avoids any possible RMW situation; takes less code and cycles; avoids this situation.

https://www.avrfreaks.net/index.p...
https://www.avrfreaks.net/index.p...
https://www.avrfreaks.net/index.p...

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
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
Suggestion: just rebuild ADMUX from scratch every operation. Avoids any possible RMW situation; takes less code and cycles; avoids this situation.

https://www.avrfreaks.net/index.p...
https://www.avrfreaks.net/index.p...
https://www.avrfreaks.net/index.p...


Whoops, should've had a look through the forum huh? :oops:
Thanks for that, will do.

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

When you have such intractable problems, a little time with the simulator can help to show you what is really happening. Whilst you have to fake up the adc readings, you would be able to narrow down where the problem is.

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

What is the system clock speed? The Tiny13 has internal clocks at 9.6 and 4.8MHz. It can also use external clock, but not a crystal. The factory default is 1.2MHz (9.6 MHz divided by 8 due to the CKDIV8 fuse being active).

The comment in the delay says 9 MHz, but the comment in the ADC_init section says that the pre-scaler divided_by_8 gives 125 KHz for ADC clock. If the pre-scaler is set too low, then the ADC will not give valid readings ( in the lower bits). Always make sure that the comments are correct.

The ADSC bit changes state when the conversion is finished. Don't use a delay to time out the ADC; always read the ADSC.

I was led to believe that (0 << CONSTANT) actually would clear a bit in C, but not in assembler. Speaking the 'A' word, why aren't you doing this in assembler? The device has only 1K of program memory, so there isn't space to do non-trival applications. With Assembler there are no unusual unseen conditions like having the compiler decide to ignore your code (excuse me, 'optimizing' it) on a whim that force the poor fool student to spend hours analyzing the source and casting voodoo spells over the keyboard to debug simple routines. Using Assembler is like fine-tuning a BMW; using C is like trying to sweet-talk a camel. Both will get you to where you need to go; one is a more straight-forward process.

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

Quote:

I was led to believe that (0 << CONSTANT) actually would clear a bit in C,

(0 << anything) is just a constant, value 0. Just as in ASM, it depends what you do with that 0. Other than for completeness or clarity when e.g. setting all prescaler bits TCCxxx = ... | (1 << PS3) | (0 << PS2) | (1 << PS1) | (0 << PS0); I see no practical use for it. 0 is 0.

I'll skip the C vs. ASM for now. But I would like to again ask about the fascination with the Tiny13. Yes, the '13A has most modern AVR features. But in modest quantities the Tiny25 has more "modern" features and twice the resources at virtually the same price.

Perhaps the pricing might be significant on qty. 100k toys or holiday ornaments. Not in normal pricing. (IIRC a couple years ago we got a quote for qty. 10k and the '25 was less than the '13.)

Is there a particular feature of the '13A that grabs people?

Lee

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

Quote:
Using the delay library causes massive bloat if the delay times aren't set at compile time.
But you can avoid that by simply changing your delay function to this:

void delay(long int ticks) { 
   while (ticks != 0) { 
   _delay_ms(1); //This is very close to 1ms no matter what cpu frequency you use
   ticks--; 
   } 
} 

Regards,
Steve A.

The Board helps those that help themselves.

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

Simonetta wrote:
What is the system clock speed? The Tiny13 has internal clocks at 9.6 and 4.8MHz. It can also use external clock, but not a crystal. The factory default is 1.2MHz (9.6 MHz divided by 8 due to the CKDIV8 fuse being active).

The comment in the delay says 9 MHz, but the comment in the ADC_init section says that the pre-scaler divided_by_8 gives 125 KHz for ADC clock. If the pre-scaler is set too low, then the ADC will not give valid readings ( in the lower bits). Always make sure that the comments are correct.

The ADSC bit changes state when the conversion is finished. Don't use a delay to time out the ADC; always read the ADSC.

I was led to believe that (0 << CONSTANT) actually would clear a bit in C, but not in assembler. Speaking the 'A' word, why aren't you doing this in assembler? The device has only 1K of program memory, so there isn't space to do non-trival applications. With Assembler there are no unusual unseen conditions like having the compiler decide to ignore your code (excuse me, 'optimizing' it) on a whim that force the poor fool student to spend hours analyzing the source and casting voodoo spells over the keyboard to debug simple routines. Using Assembler is like fine-tuning a BMW; using C is like trying to sweet-talk a camel. Both will get you to where you need to go; one is a more straight-forward process.

Sorry, the comment there was leftover from when I borrowed code from another website. I've since removed it, the actual system clock is 1MHz.
I was unaware that the ADSC bit changed state when conversion was finished. I suspected it, but adding the delay seemed an easier thing to do at the time.
This project is a massive learning experience for me. The only experience I've ha so far is with an Arduino.
As for why I'm not using assembler, well it's basically the experience thing. I'd like to learn to program these in C first, then try to port that to assembler later on to see how it works.

theusch wrote:
But I would like to again ask about the fascination with the Tiny13. Yes, the '13A has most modern AVR features. But in modest quantities the Tiny25 has more "modern" features and twice the resources at virtually the same price.

Perhaps the pricing might be significant on qty. 100k toys or holiday ornaments. Not in normal pricing. (IIRC a couple years ago we got a quote for qty. 10k and the '25 was less than the '13.)

Is there a particular feature of the '13A that grabs people?


Not really... I actually meant to buy the Tiny25, but added the wrong product to the shopping cart by mistake. I figured I may as well use it, and it might help me learn more about optimising the code as I write it, rather than rely on the compiler to do it for me.

Koshchi wrote:
But you can avoid that by simply changing your delay function to this:

Since the other tips were posted last night, I noticed in one of the threads theusch linked to that the author had done it this way, and it made sense to me. I've since changed the code to do it this way.