Some clarification about using noInterrupts() please

Dr_Nic

Member
I'm using Joshua Austill's AD7606 library and he makes use of
Code:
noInterrupts()
when reading from theAD7606 in parallel.

In his basic example code he does the ADC read directly in the main loop. So the timing of the acquisition is dependent on the other logic in the loop, the more additional logic, the slower the acquistion speed.

To overcome this I tried to implement an
Code:
IntervalTimer
to sample every 10uS and pushing the samples into a ring-buffer to be dealt with in the main loop as I see fit.


C++:
void samplingISR()
{
  adc.getVoltages(voltageData);

  // Check ring buffer has space
  if(rb.bytesFree() >= sizeof(voltageData))
  {
    rb.write((const uint8_t*)&voltageData, sizeof(voltageData));
    totalSamplesLogged++;
  }
  else
  {
    bufferOverflowed = true;
  }
}

When I put the ADC read into the ISR the code just freezes.

The
Code:
getVoltages()
turns off interrupts, reads the ADC then turns them back on again.


C++:
void AD7606p16_t4::getVoltages(float* voltages) {
    if (lastIsrCall > 10) {
        this->reset();
    }
    noInterrupts(); // Disable interrupts to ensure atomic read
    for (uint8_t i = 0; i < 8; i++) {
        voltages[i] = (instance->channels[i] * instance->vRef) / 65536.0f;
    }
    interrupts(); // Re-enable interrupts
}

Could this be why my code stalls?
 
A perfect testcase why noInterrupts(); is bad and "the most simple solution" is not always the best solution.
 
"This pattern is always bad," isn't useful either. Depending on how the hardware works, specifically the interrupt priority and nesting logic, this might be precisely what's needed in the larger context. For example, if there are only two levels of priority and you need three, then you might do this to prevent a nest and effectively make itself the top priority.

And as I said in a different thread, normally it's better to save and restore the actual state of the global interrupt flag, instead of unconditionally turning it on at the end. But in the Arduino framework specifically, it's always on anyway, so an unconditional re-enable is just fine and results in less code.

Anyway, I don't see an obvious reason why it should stall. What's inside of noInterrupts()?

And while you're chasing code, see if you can find where the hardware register is actually read. In a lot of chips, *that* is what clears the interrupt flag, and if the flag is not cleared when the ISR returns (either never cleared or set again, doesn't matter which), it immediately reenters the ISR. Continuously reentering an ISR can make it appear stalled.

Also for troubleshooting, you might put some digitalWrites or digitalToggles to some unused pins in some strategic places, to see if you're getting there at all, or more often than you should, or at weird times, etc. That, plus an oscilloscope or logic analyzer, can tell you a LOT!
 
Back
Top