Help: Interrupts and arrays/pointers

Using Arduino Project Guidance
1

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!

// 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]);
  }
}
2

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

float* buff;

int buff[buufSize];

and drop the line with the malloc completely.

Mark

3

while (bit_is_set(ADCSRA, ADSC));this probably has no place in interrupt context.
Please sort out your circular buffer and repost.

4

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

float* buff;

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.

5

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

Mark

6

holmes4:
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.