"ADC + Timer1" on ATmega328p ask for help!

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

I made a test project: adjust the blinking frequency of LED on PB0 of ATmega328p by potentiometer on PC0 of it, my code is:

/*
 * testAVR-libcATmega328pBlink.c
 *
 */ 

/*
 * The program for adjusting the LED blinking frequency by a potentiometer
 * chip: ATmega328p
 * potentiometer --> ADC0
 * (ATmega328p)PB0 --> 1.5kOhm --> LED
 */

#define F_CPU 16000000UL
#define VREF 5
#define POT 10000 // potentiometer - 10Kohm

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>

volatile uint16_t potVal = 0;

void PB0_init();
void PC0_init();
void initADC();
uint16_t readADC(uint8_t);

int main(void)
{
    // inintialize Timer1
    timer1_init();

    // initialize ADC
    initADC();

    // LED connected to PB0/D8
    PB0_init();

    sei(); // Enable global interrupt

    // loop forever
    while(1)
    {
      PB0_toggle(potVal);
    }
}

void initADC()
{
  // Select V ref=AVcc
  // Bit 7:6 – REFS1:0: Reference Selection Bits
  ADMUX |= (1<<REFS0);
  //Bit 5 – ADLAR: ADC Left Adjust Result
  ADMUX &= !(1<<ADLAR);  // ADLAR = 0 - Right Aligned

  // Bits 2:0 – ADPS2:0: ADC Prescaler Select Bits
  // These bits determine the division factor between the system clock frequency and the input clock to the ADC.
  //set prescaler to 128
  ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);

  // Bit 7 – ADEN: ADC Enable - Writing this bit to one enables the ADC.
  ADCSRA |= (1<<ADEN);

  // This bit stays high as long as the conversion is in progress and will be cleared by hardware when the conversion is completed.
  // Bit 6 – ADSC: ADC Start Conversion
  // The first conversion after ADSC has been written after the ADC has been enabled,
  // or if ADSC is written at the same time as the ADC is enabled,
  // will take 25 ADC clock cycles instead of the normal 13.
  // This first conversion performs initialization of the ADC.
  ADCSRA |= (1<<ADSC);
}

uint16_t readADC(uint8_t ADCchannel)
{
  //select ADC channel with safety mask
  ADMUX = (ADMUX & 0xF0) | (ADCchannel & 0x0F);

  // PRR – Power Reduction Register
  // The power reduction ADC bit, PRADC, must be disabled by writing a logical zero to enable the ADC.
  // A single conversion is started by disabling the power reduction ADC bit, PRADC, by writing a logical zero to it
  // and writing a logical one to the ADC start conversion bit, ADSC.
  // Bit 0 - PRADC: Power Reduction ADC
  // Writing a logic one to this bit shuts down the ADC. The ADC must be disabled before shut down.
  PRR &= !(1<<PRADC);
  ADCSRA |= (1<<ADSC);

  // wait until ADC conversion is complete
  while( ADCSRA & (1<<ADSC) );

  // Bit 0 - PRADC: Power Reduction ADC
  // Writing a logic one to this bit shuts down the ADC. The ADC must be disabled before shut down.
  // Bit 7 – ADEN: ADC Enable - Writing this bit to one enables the ADC.
  ADCSRA &= !(1<<ADEN);
  PRR |= (1<<PRADC);

  return ADC;
}

void PB0_init(void){
   DDRB |= (1<<DDB0);
}

void PB0_toggle(int t){
   PORTB ^= (1<<PORTB0); // toggles the led
   for(int i = 0; i < t; i++){
     _delay_ms(1);
   }
}

// initialize timer, interrupt and variable
void timer1_init()
{
    // TCCR1B – Timer/Counter1 Control Register B
    // set up timer with prescaler = 1024
    // Now, Timer1 is 16M/1024=15.625kHz Cycle=64us
    TCCR1B |= (1 << CS12)|(1 << CS10);
    // and CTC mode:
    // TOP = OCR1A
    // Update of OCR1A at Immediate
    // TOV1 Flag Set on "MAX"
    TCCR1B |= (1 << WGM12);

    // The output compare registers contain a 16-bit value that is continuously compared with the counter value (TCNT1).
    // A match can be used to generate an output compare interrupt, or to generate a waveform output on the OC1x pin.
    // The output compare registers are 16-bit in size.
    OCR1A = 0xFFF; // 64*4096=262.144ms - almost 4 times sampling per second

    // TIMSK1 – Timer/Counter1 Interrupt Mask Register
    // Bit 0 – TOIE1: Timer/Counter1, Overflow Interrupt Enable
    TIMSK1 |= (1<<TOIE1);    

    // initialize counter
    TCNT1 = 0;
}

// Vector No.   Program Address   Source            Interrupt Definition
// 14           0x001A            TIMER1 OVF        Timer/Counter1 overflow
ISR(TIMER1_OVF){
   potVal = readADC(0);
   //potVal = (potVal/1023.0)*500;
   reti();
}

the ADC output was always ZERO, anyone know why?

This topic has a solution.
Last Edited: Tue. Sep 20, 2022 - 11:29 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MianQi wrote:

ISR(TIMER1_OVF){
   potVal = readADC(0);
   //potVal = (potVal/1023.0)*500;
   reti();
}

 

Do not do ADC read in ISR.

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

More to the point NEVER use reti() in an ISR. There is only one very isolated use for reti() and this sure ain't it! 

 

The usual route out of an ISR is just the closing brace at the end of the function. Nothing more. For cosmetic effect you could add a "return;" in there if you like but never reti();! 

 

The user manual explains the only use of reti() :

 

https://www.nongnu.org/avr-libc/user-manual/group__avr__interrupts.html#ga3b991e8168db8fc866e31f9a6d10533b

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

Thanks you two, and I found there is also one "ADCSRA |= (1<<ADEN);" lacked inside "readADC()", now these works:

/*
 * testAVR-libcATmega328pBlink.c
 *
 */ 

/*
 * The program for adjusting the LED blinking frequency by a potentiometer
 * chip: ATmega328p
 * potentiometer --> ADC0
 * (ATmega328p)PB0 --> 1.5kOhm --> LED
 */

#define F_CPU 16000000UL
#define VREF 5
#define POT 10000 // potentiometer - 10Kohm

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>

volatile uint16_t potVal = 0;

void PB0_init();
void initADC();
uint16_t readADC(uint8_t);

int main(void)
{
    // initialize ADC
    initADC();

    // LED connected to PB0/D8
    PB0_init();

    // loop forever
    while(1)
    {
      potVal = readADC(5);
      PB0_toggle(potVal);
    }
}

void initADC()
{
  // Select V ref=AVcc
  // Bit 7:6 – REFS1:0: Reference Selection Bits
  ADMUX |= (1<<REFS0);
  //Bit 5 – ADLAR: ADC Left Adjust Result
  ADMUX &= !(1<<ADLAR);  // ADLAR = 0 - Right Aligned

  // Bits 2:0 – ADPS2:0: ADC Prescaler Select Bits
  // These bits determine the division factor between the system clock frequency and the input clock to the ADC.
  //set prescaler to 128
  ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);

  // Bit 7 – ADEN: ADC Enable - Writing this bit to one enables the ADC.
  ADCSRA |= (1<<ADEN);

  // This bit stays high as long as the conversion is in progress and will be cleared by hardware when the conversion is completed.
  // Bit 6 – ADSC: ADC Start Conversion
  // The first conversion after ADSC has been written after the ADC has been enabled,
  // or if ADSC is written at the same time as the ADC is enabled,
  // will take 25 ADC clock cycles instead of the normal 13.
  // This first conversion performs initialization of the ADC.
  ADCSRA |= (1<<ADSC);
}

uint16_t readADC(uint8_t ADCchannel)
{
  //select ADC channel with safety mask
  ADMUX = (ADMUX & 0xF0) | (ADCchannel & 0x0F);

  // PRR – Power Reduction Register
  // The power reduction ADC bit, PRADC, must be disabled by writing a logical zero to enable the ADC.
  // A single conversion is started by disabling the power reduction ADC bit, PRADC, by writing a logical zero to it
  // and writing a logical one to the ADC start conversion bit, ADSC.
  // Bit 0 - PRADC: Power Reduction ADC
  // Writing a logic one to this bit shuts down the ADC. The ADC must be disabled before shut down.
  PRR &= !(1<<PRADC);
  ADCSRA |= (1<<ADEN);
  ADCSRA |= (1<<ADSC);

  // wait until ADC conversion is complete
  while( ADCSRA & (1<<ADSC) );

  // Bit 0 - PRADC: Power Reduction ADC
  // Writing a logic one to this bit shuts down the ADC. The ADC must be disabled before shut down.
  // Bit 7 – ADEN: ADC Enable - Writing this bit to one enables the ADC.
  ADCSRA &= !(1<<ADEN);
  PRR |= (1<<PRADC);

  return ADC;
}

void PB0_init(void){
   DDRB |= (1<<DDB0);
}

void PB0_toggle(int t){
   PORTB ^= (1<<PORTB0); // toggles the led
   for(int i = 0; i < t; i++){
     _delay_ms(1);
   }
}

But, it could only convert several times, then the ADC value would be fixed in "1023", what maybe the reason?

Last Edited: Wed. Sep 21, 2022 - 12:30 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why would you need ADEN in the "read" function? Normally you would have "init" and "read" and you would enable the ADC in the init function. Once enabled it should stay enabled. So is the suggestion here that something is mysteriously clearing ADEN after it has been initially set?

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

ADMUX, PRR, as well. Do Init and leave in peace.

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

I would advise you to pare your program down to the bare essentials. Get a really simple program working well first, only then build in more functionality.

 

I see you've already dropped out the TIMER. That's good.

Let's drop out readADC also:

 

  1. Leave PRR well alone.
  2. Configure the ADC in Free Running Mode.
  3. Change your forever loop thus:
     

        // loop forever
        while (1) {
            potVal = ADC;
            PB0_toggle(potVal);
        }

 

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

N.Winterbottom wrote:

 

  1. Configure the ADC in Free Running Mode.

 

 

I used Free Running Mode at the first, I found the serial port was blocked - each time after "reset", it just print "hhe" or "hehe" of my tested "hello, world!" then stop there, so I altered to single mode, I have thought this would probably resolve the presumed "latency" trouble。

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

clawson wrote:
Why would you need ADEN in the "read" function?

 

I read this in datasheet:

The ADC must be disabled before shut down.

so I thought I need to enable and disable ADC alternately so as to run in single conversion mode.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
void PB0_init(void){
   DDRB |= (1<<DDB0);
}

What good is this?? Other than giving some name to hunt all around the listing for, it does nothing.  Just put that line of code in your program and it will be both fine and immediately viewable for inspection WITH NO EFFORT REQUIRED.  It might be a different story if this was something you were doing from a lot of different places, or it had some complexity to it.  Otherwise, a waste of both usable screen space and typing has occurred. No gain, only losses.

The less you have to go searching around to find things, the easier you will be able to spot troubles.  Keep everything as simple direct and visible as possible, you will find your mistakes faster, than burying them .  When programs get larger, yes, you will divide into larger chunks that go into different suitcases, hiding their details somewhat.  Those large chunks can be heavily tested, then hidden away, known working.

  

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Thu. Sep 22, 2022 - 04:05 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MianQi wrote:
I used Free Running Mode at the first, I found the serial port was blocked

OK I should have been more specific.

I'm suggesting minimal bare bones code so that means No Interrupts. It should take no more than about 10 lines of code.

 

BTW: None of your posted code shows any UART activity.

 

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

avrcandies wrote:
What good is this??

 

I want the code looks regularly and clearly, since I would pile up other module, this would marked those past testing.

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

I want the code looks regularly and clearly, since I would pile up other module, this would marked those past testing.

That is a noble goal, and will help you when things get complex, but it won't help you at all there.  Use it when needed, if you overdo it at every possible turn, it becomes harder to follow, not easier. You could end up making every line of your program a subroutine/function. That function doesn't have much complexity to say "this block has been tested, let's put it aside to add clarity to the overall flow".

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

Here is the entire test/demo program written for the ATmega328P in Arduino format.

Notice how straightforward and easy it is to write and modify.

 

===================================================

// toggle LED on PortB0 according to the pot value on ADC0

unsigned long  myToggleInterval = 500;

 

void setup() {

  pinMode( D8, output);   // LED PortB0.  D0 to D7 is PortD

}

 

void loop() {

    if (myToggleInterval < millis()) {

       myToggleInterval = 250 + analogRead(0) + millis();

       digitalWrite(D8, !digitalRead(D8) );

    }

}

 

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

Yeah, 106 lines of text to blink an LED seems a bit over the top...can be a lot more efficient with the description, so you don't get buried in it. ..you want to see the actual code being used

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Thu. Sep 22, 2022 - 10:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:
BTW: None of your posted code shows any UART activity.

 

My UART couldn't show the decimal data:

void TX(unsigned char TXData) {
    //do nothing until UDR0 is ready for more data to be written to it; wait for USART UDRE flag
    while ((UCSR0A & (1 << UDRE0)) == 0) {}; 
    //when flag is set send data by placing the byte into UDR0
    UDR0 = TXData; 
}

unsigned char regToDigitAndTransmit(uint16_t valueADC){
  
  digit_0 = valueADC/1000;
  //digit_0 = '1';
  digit_0 = digit_0 - '48';
  digit_1 = (valueADC - (digit_0 * 1000))/100;
  //digit_1 = digit_1 - '48';
  digit_2 = (valueADC - (digit_0 * 1000) - (digit_1 * 100))/10;
  //digit_2 = digit_2 - '48';
  digit_3 = (valueADC - (digit_0 * 1000) - (digit_1 * 100) - (digit_2 * 10));
  //digit_3 = digit_3 - '48';
  promptPrint();
  TX(digit_0);
  //TX(digit_1);
  //TX(digit_2);
  //TX(digit_3);
  TX('.');
  TX('\n');
}

 

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

MianQi wrote:

  digit_0 = digit_0 - '48';
  digit_1 = (valueADC - (digit_0 * 1000))/100;

 

How Digit 1 can be calculated when Digit 0 is spoiled, please?

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

Thanks for your hint, now these works:

unsigned char regToDigitAndTransmit(uint16_t valueADC){
    digit_0 = valueADC/1000;
    unsigned char number_0 = digit_0 + 48;
    digit_1 = (valueADC - (digit_0 * 1000))/100;
    unsigned char number_1 = digit_1 + 48;
    digit_2 = (valueADC - (digit_0 * 1000) - (digit_1 * 100))/10;
    unsigned char number_2 = digit_2 + 48;
    digit_3 = (valueADC - (digit_0 * 1000) - (digit_1 * 100) - (digit_2 * 10));
    unsigned char number_3 = digit_3 + 48;

    promptPrint();
    TX(number_0);
    TX(number_1);
    TX(number_2);
    TX(number_3);
  TX('.');
  TX('\n');
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 digit_0 = valueADC/1000;
 valueADC -= 1000* digit0;
 digit_0 +=  0x30;

 digit_1 = valueADC/100;
 valueADC -= 100* digit1;
 digit_1 +=  0x30;

 digit_2 = valueADC/10;
 digit_3 -= 10* digit2 + 0x30;
 digit_2 += 0x30; //must come last

 

Digit 0 should really prob be the LSD , like:  dig3 dig2 dig1 dig0 ...though not a big deal

4376  "6" should be called digit 0

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Fri. Sep 23, 2022 - 06:30 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The way I'd approach this:

void TXstring(char * str) {
    while(*str) {
        TX(*str++);
    }
}

unsigned char regToDigitAndTransmit(uint16_t valueADC){
    char buff[8];
    itoa(valueADC, buff, 10);
    TXstring(buff);
}

I don't see any particular merit in trying to reinvent your own version when itoa() already exists.

 

For a more complex version:

unsigned char regToDigitAndTransmit(uint16_t valueADC){
    char buff[8];
    sprintf(buff, "%u.%02u", valueADC / 100, valueADC % 100);
    TXstring(buff);
}

 

Last Edited: Fri. Sep 23, 2022 - 08:15 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:
Digit 0 should really prob be the LSD
  got it!

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

clawson wrote:

sprintf(buff, "%u.%02u", valueADC / 100, valueADC % 100);

How could this one line print 4 digits?

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

MianQi wrote:

clawson wrote:

sprintf(buff, "%u.%02u", valueADC / 100, valueADC % 100);

How could this one line print 4 digits?

time to break out your C manual!

 

If valueADC were 1234 I'm kind of hoping this would print "12.34"