Go Down

Topic: Help: Interrupts and arrays/pointers (Read 777 times) previous topic - next topic

memotick

Hello,
     I am not sure if I am approaching this correctly.  Basically I have a buffer array that I want populated by an interrupt.  I made the buffer a global pointer and thus do not pass it to the interrupt.  Is this the preferred method?  I've read that variables changed by interrupts should be made volatile, but I can't seem to get it to work ("volatile float* buff;" is not accepted).

Please see the function "ITR1" in the code below.  Thanks!

Code: [Select]

// sbi is to set a bit
// cbi is to clear a bit
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif

int buffSize=60;
int TOP=50; // this sets OCR1A for the CTC counter
float* buff;
volatile uint8_t low, high;

// Read a Samp and store it in the buff
// This is called at a fixed Samprate via the Interrupt
// Each time a Samp is made, this routine is called
void ITR1() {
  while (bit_is_set(ADCSRA, ADSC)); 
  low  = ADCL;
  high = ADCH;

  // I know this is wasteful, I'm converting it to a
  // circular buffer soon
  for (int i= 0 ; i < buffSize - 1 ; i++){
    buff[i] = buff[i+1];
  }
  buff[buffSize - 1] = (high<<8) | low;
}

//Interrupt Service Routine (ISR)
ISR(TIMER1_COMPA_vect) {
  sbi(ADCSRA, ADSC);
  ITR1();
}

// This gets called at startup
void setupIntr(){

  // initialize Timer1
  cli();          // disable global interrupts
  TCCR1A = 0;     // set entire TCCR1A register to 0
  TCCR1B = 0;     // same for TCCR1B

  // set compare match register to desired timer count:
  // also known as the TOP
  OCR1A = TOP;

  TCCR1B  |= (1 << CS12);
  // turn on CTC mode:
  TCCR1B |= (1 << WGM12);

  // enable timer compare interrupt:
  TIMSK1 |= (1 << OCIE1A);
 
  // page 264
  ADMUX |= (1 << REFS0); //set reference voltage
  // ADCSRA: page 265
  // ADEN: Enable ADC
  sbi(ADCSRA, ADEN);
  // ADATE: ADC auto trigger enable
  cbi(ADCSRA, ADATE);

  // ADMUX page 264
  // select ADC1 channel
  cbi(ADMUX, MUX3);
  cbi(ADMUX, MUX2);
  cbi(ADMUX, MUX1);
  cbi(ADMUX, MUX0);

  // Quiet ADC inputs
  // Check page 266 of the ATmega328p datasheet
  // for more information.
  sbi(DIDR0,ADC5D);
  sbi(DIDR0,ADC4D);
  sbi(DIDR0,ADC3D);
  sbi(DIDR0,ADC2D);
  sbi(DIDR0,ADC1D);
  sbi(DIDR0,ADC0D);

  // start the conversion
  sbi(ADCSRA, ADSC);
}

void loop() {
}

void setup() {
  //setup Interrupt routine
  setupIntr();
  Serial.begin(57600); 

  buff = (float *) malloc(sizeof(float)* buffSize);

  sei();//enable interrupts

  delay(1000);
  TIMSK1=0;

  for(int index=0; index < buffSize; index++) {

    Serial.write("buff[");
    Serial.print(index);
    Serial.write("]=");
    Serial.println(buff[index]);
  }
}

holmes4

The buffer should be int not float.

You are not changing the address pointer so no need to make that a volatile.

Change this to
Code: [Select]
float* buff;


Code: [Select]
int buff[buufSize];

and drop the line with the malloc completely.

Mark

AWOL

Code: [Select]
while (bit_is_set(ADCSRA, ADSC));this probably has no place in interrupt context.
Please sort out your circular buffer and repost.
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

memotick


The buffer should be int not float.

You are not changing the address pointer so no need to make that a volatile.

Change this to
Code: [Select]
float* buff;


Code: [Select]
int buff[buufSize];

and drop the line with the malloc completely.

Mark

Why should buff be made int*?  If I need the contents of buff to be float couldn't I leave it as is?  Or is it too wasteful since the output of the ADC can be fit in int variables.

Good point about the malloc.

holmes4

The output of the ADC in an int. If you need to convert it do it out side the ISR.

Mark

memotick


The output of the ADC in an int. If you need to convert it do it out side the ISR.

Mark


Makes sense, thanks for the explanation.

Go Up