next topic »

[on:]

Hello everyone,

I hope someone can help me with my current Arduino project. I am trying to make a Digitaly Controlled Oscillator for a synthesizer, and as one part of that I need to be able to read an analogue voltage from the arduino.

I want to trigger an update of the ADC every time Timer0 overflows, and once the ADC has updated, have it throw and interupt.
From reading the data sheet I belive that this is possible but it doesnt work in my code. It works in free-running mode, but that will poll the ADC too fast for my needs. When triggering from the Timer0 overflow, the interupt routine never gets called, and I cant for the life of me seem to find out why.

I should point out that I am quite new to dealing with registers on the arduino so perhaps that will factor into this.

Here is my code so far (sorry for the mess :

Code:

/*  */
#include <math.h>
#include "pins_arduino.h"

/*  */
namespace PreScaler
{
    enum ePsk
    {
        C_PSK_1    = 0b00000001,
        C_PSK_8    = 0b00000010,
        C_PSK_64   = 0b00000011,
        C_PSK_256  = 0b00000100,
        C_PSK_1024 = 0b00000101
    };
};

/*  */
int lSt = 0;
ISR( TIMER1_COMPA_vect )
{
    /* change this to use bitwize xor
     * on the output port
     * some funny errors with this
     */
    if ( lSt == 0 ) digitalWrite( 9, HIGH );
    else            digitalWrite( 9, LOW  );
    lSt = 1 - lSt;
}

/*  */
void set_pll_freq( uint16_t aFreq )
{
    int32_t lOCR = (F_CPU / (aFreq * 2)) - 1;
    int8_t  lPsk = PreScaler::C_PSK_1;
    /* find prescaler and set ctc max */
    if ( lOCR > 0xFFFF )
    {
        lOCR = (F_CPU / (aFreq * 2 * 64)) - 1;
        if ( lOCR > 0xFFFF )
             lOCR = 0xFFFF;
        lPsk = PreScaler::C_PSK_64;
    }
    /* update the registers */
    TCCR1B = (TCCR1B & 0b11111000) | lPsk;
    OCR1A  = lOCR;
}

/*  */
void start_pll( void )
{
    /* setup the output pin */
    pinMode( 9, OUTPUT );
    /* setup 16bit timer */
    TCCR1A = 0b10000000;
    TCCR1B = 0b00001000;
    TCCR1B = TCCR1B | (PreScaler::C_PSK_1);
    /* enable the interupt */
    TIMSK1 = 0b00000010;
}

/*  */
uint16_t gAPM = 0;
bool gPitchChange = false;
ISR( ADC_vect )
{
    uint16_t lAPM = 0;
//    if ( (ADCSRA | 0b00010000) != 0 )
//    {
      lAPM  = ADCL;
      lAPM += ADCH << 8;
      /* compare without jitter */
      if ( (lAPM>>1) != (gAPM>>1) )
      {
          gPitchChange = true;
          gAPM = lAPM;
      }
//    }
    /* re-enable the interupt */
//    ADCSRA = 0b10101111;
}

/*  */
void start_apm( void )
{
    /* setup 8bit timer */
    TCCR0A = 0b00000000;
    TCCR0B = 0b00000000;
    TCCR0B = TCCR0B | (PreScaler::C_PSK_1024);
    TIMSK0 = 0b00000000;
    /* setup the d-a converter */
    DIDR0  = 0b00111111;
    ADMUX  = 0b01000000;
//    ADCSRB = 0b00000000; // free running mode
    ADCSRB = 0b00000100; // timer 0 overflow
    ADCSRA = 0b11101111;
}

/*  */
extern
void setup( void )
{
    /* analogue pitch modulation */
    start_apm( );
    /* phase locked loop output */
    start_pll( );
    /* enable the output */
    gPitchChange = true;
}

/*  */
uint32_t note = 1;

/*  */
extern
void loop( void )
{
    /* nothing yet */
//    set_pll_freq( (uint16_t)(pow( 2.0f, (float)note/12.0f )) + (lA/7));
    if ( gPitchChange )
    {
        set_pll_freq( gAPM );
        gPitchChange = false;
    }
}

Timer 0 and the ADC are setup in the start_apm function.
Currently, the program should read an analogue voltage on Analogue Pin 0 and output a square wave of proportional frequency on pin 9.

I would be most greatfull for some assistance with this problem.

I am using the Arduino Duemilanove with the Atmega328p chip.

[Reply #1 on:]

Couple of weird things:

* Your program starts with gAPM=0 which is a problem when you first call set_pll_freq() with a parameter of 0. That causes a division-by-error when computing lOCR.

* Watch out for the number 0xFFFF. By default that's a signed 16-bit integer which is sign-extended to -1 as an int32_t (I think). So the comparison:

Code:

    if ( lOCR > 0xFFFF )

could be interpreted as:

Code:

    if (lOCR > -1)

Use 0xFFFFU to interpret this as an unsigned number.

* Since the variables gPitchChange and gAPM are modified within an ISR they should be declared 'volatile'.

--
Beat707: MIDI drum machine / sequencer / groove-box for Arduino

[Reply #2 on:]

Thanks Rugged, you seem to have a realy good eye for spotting potential bugs.
While these suggestions didnt fix my problem, they are great in their own right.

Here is the code including Ruggeds suggestions:

Code:

/*  */
#include <math.h>
#include "pins_arduino.h"

/*  */
namespace PreScaler
{
    enum ePsk
    {
        C_PSK_1    = 0b00000001,
        C_PSK_8    = 0b00000010,
        C_PSK_64   = 0b00000011,
        C_PSK_256  = 0b00000100,
        C_PSK_1024 = 0b00000101
    };
};

/*  */
volatile int lSt = 0;
ISR( TIMER1_COMPA_vect )
{
    /* change this to use bitwize xor
     * on the output port
     * some funny errors with this
     */
    if ( lSt == 0 ) digitalWrite( 9, HIGH );
    else            digitalWrite( 9, LOW  );
    lSt = 1 - lSt;
}

/*  */
void set_pll_freq( uint16_t aFreq )
{
    if ( aFreq == 0 ) aFreq = 1;
    uint32_t lOCR = (F_CPU / (aFreq * 2)) - 1;
    uint8_t  lPsk = PreScaler::C_PSK_1;
    /* find prescaler and set ctc max */
    if ( lOCR > 0xFFFFU )
    {
        lOCR = (F_CPU / (aFreq * 2 * 64)) - 1;
        if ( lOCR > 0xFFFFU )
             lOCR = 0xFFFF;
        lPsk = PreScaler::C_PSK_64;
    }
    /* update the registers */
    TCCR1B = (TCCR1B & 0b11111000) | lPsk;
    OCR1A  = lOCR;
}

/*  */
void start_pll( void )
{
    /* setup the output pin */
    pinMode( 9, OUTPUT );
    /* setup 16bit timer */
    TCCR1A = 0b10000000;
    TCCR1B = 0b00001000;
    TCCR1B = TCCR1B | (PreScaler::C_PSK_1);
    /* enable the interupt */
    TIMSK1 = 0b00000010;
}

/*  */
volatile uint16_t gAPM = 1;
volatile bool gPitchChange = false;
ISR( ADC_vect )
{
    uint16_t lAPM = 0;
//    if ( (ADCSRA | 0b00010000) != 0 )
//    {
      lAPM  = ADCL;
      lAPM += ADCH << 8;
      /* a little historesis compare ?? */
      if ( (lAPM>>1) != (gAPM>>1) )
      {
          gPitchChange = true;
          gAPM = lAPM;
      }
//    }
    /* re-enable the interupt */
//    ADCSRA = 0b10101111;
}

/*  */
void start_apm( void )
{
    /* setup 8bit timer */
    TCCR0A = 0b00000000;
    TCCR0B = 0b00000000;
    TCCR0B = TCCR0B | (PreScaler::C_PSK_1024);
    TIMSK0 = 0b00000000;
    /* setup the d-a converter */
    DIDR0  = 0b00111111;
    ADMUX  = 0b01000000;
//    ADCSRB = 0b00000000; // free running mode
    ADCSRB = 0b00000100; // timer 0 overflow
    ADCSRA = 0b11101111;
}

/*  */
extern
void setup( void )
{
    /* analogue pitch modulation */
    start_apm( );
    /* phase locked loop output */
    start_pll( );
    /* enable the output */
    gPitchChange = true;
}

/*  */
uint32_t note = 1;

/*  */
extern
void loop( void )
{
    /* nothing yet */
//    set_pll_freq( (uint16_t)(pow( 2.0f, (float)note/12.0f )) + (lA/7));
    if ( gPitchChange )
    {
        set_pll_freq( gAPM );
        gPitchChange = false;
    }
}

[Reply #3 on:]

You're configuring your timer in mode 4 (b0100) which causes an overflow on the MAX value, which is 0xFFFF, not OCR1A. So really, there's never going to be any overflow. Maybe try mode 11 (b1011) or mode 15 (b1111).

--
The Ruggeduino: compatible with Arduino UNO, 24V operation, all I/O's fused and protected

[Reply #4 on:]

I am puzzled,
I think you could be looking at my timer1 code?
I say this because you quoted 0xFFFF as a max value.
The timer 1 code seems to work fine and generates a set frequency the same way as the Tone library does.

Having said that perhaps I have missed your point, but I cant find anything in the datasheet that
reffers to any timer having a mode > 7. This is the datasheet I have been looking at:
http://www.atmel.com/dyn/resources/prod_documents/doc8161.pdf

[edit]
Ok, I see that timer 1 has modes > 7 so I am sure that you are looking at my timer 1 code instead.
[/edit]

Just to clarrify Im having problems with my Timer0 code:

Code:

   /* setup 8bit timer */
    TCCR0A = 0b00000000;
    TCCR0B = 0b00000000;
    TCCR0B = TCCR0B | (PreScaler::C_PSK_1024);
    TIMSK0 = 0b00000000;

It is my understanding that Timer0 will increment until MAX, in this case 0xFF. At which point I assume that because its unsigned it will naturaly wrap around to 0 again. Will this not trigger an overflow? I have dissabled the Output Compare Units because I dont think they are needed, the timer still runs and overflows without them (?). In fact the only step I have taken to run the timer is to set the pre-scaler, since no prescaler is quoted in the data sheet as, TimerHalted or something like that.

I would have put in my own interupt routine to test for it overflowing but it seems some other part of the arduino library has already defined this interupt and it complains.

[Reply #5 on:]

Good point, I think I confused Timer 1 with Timer 0.

So...the point of Timer 1 is just to generate a variable frequency waveform on pin 9? If so I think the code is overly complicated since pin 9 is OC1A and you can just drive this pin directly by connecting up the output compare through the TCCR1A register.

Anyways...you say that code works so no point picking on it.

I can't see why you wouldn't be getting A/D interrupts based on Timer 0. Perhaps put together a simpler sketch that just tests this one behavior. This will be easier to play with, read, and debug.

--
The Gadget Shield: accelerometer, RGB LED, IR transmit/receive, speaker, microphone, light sensor, potentiometer, pushbuttons

[Reply #6 on:]

Yep timer 1 is just making a variable frequency on pin 9.

Thanks for pointing that out about OC1A, i'll try to hook it up to drive the pin directly, that would be more efficient.

Thats again some sound advice about testing just the A/D and timer 0, as another sketch.
I'll try that out tommorow since its bed time for me now. Thanks very much Rugged for your help so far.