Trying to read I2C bus in timer interrupt handler and I get a core panic

I am using the Arduino IDE to program an ESP32 S3 DevKitC. The ESP32, through a DAC on the SPI bus, controls a current source to deliver 1-3 A for about 1 second to a load. I am using an INA260A from Adafruit and the I2C bus to measure the high side current through the load. I am using the Adafruit INA260A library to communicate with the INA260A board. The board waits for a push button to be depressed before starting the current and make measurements.

I have 1 hardware interrupt timer, activated by the push button, to control how long the current is ON (~1 seconds). I want to make some measurements using the INA260A and the ADC channels on the ESP32 while the current is running.

In my first attempt, I created two functions to read the ADC channels and print the values to the serial interface and another function to read the INA260A and print those values. However, the processor panic'ed at different points in the code.

Current: Guru Meditation Error: Core 1 panic'ed (Interrupt wdt timeout on CPU1).

I thought the measurements and printing might be taking too long inside the timer interrupt, so in my next iteration, I removed the printing code from the measurements and printed after the interrupt timer was done. In this code base I only get the core panic when I try to read a measurement from the INA260A. The ADC measurements and printing work.

Is there something special that has to be done to read values from the I2C bus in an interrupt timer?

This code base panics if run. If the lines with ******* are commented out, then the code runs.

#include <esp_timer.h>
#include <Adafruit_INA260.h>
#include <ezButton.h>
#include <stdbool.h>
#include "mcp4921.h"

#define CURRENT_1A_GPIO    15
#define CURRENT_2A_GPIO    16
#define CURRENT_3A_GPIO    17

#define CURRENT_3A_VALUE   3723;  
#define CURRENT_2A_VALUE   2482;
#define CURRENT_1A_VALUE   1241;

#define LAUNCH_BUTTON_GPIO     18
ezButton launchButton(LAUNCH_BUTTON_GPIO);  
#define DEBOUNCE_TIME 50  // msec

#define ADC_CHANNEL_OPAMP 1
#define ADC_CHANNEL_GATE  2
#define ADC_CHANNEL_REF   4
#define ADC_CHANNEL_DAC   6

#define REF_VOLTAGE 3.53
//Adafruit_INA260 ina260; = Adafruit_INA260();
Adafruit_INA260 ina260;

bool DONE = false;

// measurements
typedef struct {
  float op_amp_value;
  float gate_value;
  float ref_value;
  float dac_out_value;
  float ina260_current;    *************
  // float ina260_bus_voltage;
  // float ina260_power;
} Measurement;

Measurement measurements[2];
#define INDEX_START 0
#define INDEX_END 1

// timer interrupt
hw_timer_t *launch_current_timer = NULL;
portMUX_TYPE timerMux = portMUX_INITIALIZER_UNLOCKED;
int64_t start_time;
int64_t end_time;
int64_t duration;
uint64_t current_duration = 1000000;    // microseconds, default 1 second

void enableCurrent(int16_t dac_value) {
  Serial.println("Enabling current");
  mcp4921_write(dac_value);
}

void disableCurrent() {
  Serial.println("Disabling current");
  mcp4921_write(0);
}

void makeMeasurements(int index) {
  float op_amp_value = analogRead(ADC_CHANNEL_OPAMP);
  float gate_value = analogRead(ADC_CHANNEL_GATE);
  float ref_value = analogRead(ADC_CHANNEL_REF);
  float dac_out_value = analogRead(ADC_CHANNEL_DAC);
  float ina260_current = ina260.readCurrent();        *****************
  // float ina260_bus_voltage = ina260.readBusVoltage();
  // float ina260_power = ina260.readPower();
  //measurements[index] = {op_amp_value, gate_value, ref_value, dac_out_value, }; //ina260_current, ina260_bus_voltage, ina260_power};    ******** un-comment this line to stop
  measurements[index] = {op_amp_value, gate_value, ref_value, dac_out_value, ina260_current, }; //ina260_bus_voltage, ina260_power};   ******** comment out this line
}

void printMeasurements() {
  Measurement m;
  for(int i=0; i<=INDEX_END; i++) {
  m = measurements[i];
  Serial.print("Measurements ");
  Serial.print(i);
  Serial.print(": ");
  Serial.print("  op_amp_value: ");
  Serial.print(m.op_amp_value  * (REF_VOLTAGE / 4096));
  Serial.print("   gate_value: ");
  Serial.print(m.gate_value  * (REF_VOLTAGE / 4096));
  Serial.print("   ref_value: ");
  Serial.print(m.ref_value  * (REF_VOLTAGE / 4096));
  Serial.print("   dac_out_value: ");
  Serial.print(m.dac_out_value  * (REF_VOLTAGE / 4096));

  Serial.print("   INA260a_current: ");  
  Serial.print(m.ina260_current);        **********
  // Serial.print("   INA260A_bus_voltage: ");
  // Serial.print(m.ina260_bus_voltage);
  // Serial.print("   INA260A_power: ");
  // Serial.print(m.ina260_power);

  Serial.println();
  }
}

void launchTimerStart(uint64_t duration, int16_t dac_value) {
  Serial.println("***Launch Timer Start");
  //portENTER_CRITICAL(&timerMux);
  enableCurrent(dac_value);
  makeMeasurements(INDEX_START);
  //portEXIT_CRITICAL(&timerMux);
  DONE = false;
  timerWrite(launch_current_timer, 0);
  timerAlarm(launch_current_timer, duration, false, 0);
  start_time = esp_timer_get_time(); // Get current time in microseconds
  Serial.println("***Launch Timer Started");
}

void IRAM_ATTR launchTimerEnd() {
  Serial.println("***Launch Timer END");
  //portENTER_CRITICAL_ISR(&timerMux);
  makeMeasurements(INDEX_END);
  disableCurrent();
  //portEXIT_CRITICAL_ISR(&timerMux);
  end_time = esp_timer_get_time(); // Get current time in microseconds
  duration = (end_time - start_time) / 1000;   // elapsed time in milliseconds
  Serial.print("***duration = ");
  Serial.println(duration);
  DONE = true;
}

void timerSetup() {
  launch_current_timer = timerBegin(1000000);    // 1 MHz clock
  timerAttachInterrupt(launch_current_timer, &launchTimerEnd);
}

uint16_t get_current_selected() {
  uint16_t current = 0;
  int current_1A = digitalRead(CURRENT_1A_GPIO);
  int current_2A = digitalRead(CURRENT_2A_GPIO);
  int current_3A = digitalRead(CURRENT_3A_GPIO);

  if (current_3A == LOW) {
    current = CURRENT_3A_VALUE;
  }
  else if (current_2A == LOW) {
    current = CURRENT_2A_VALUE;
  }
  else if (current_1A == LOW) {
    current = CURRENT_1A_VALUE;
  }
  return current;
}


void setup() {
  launchButton.setDebounceTime(DEBOUNCE_TIME); // set debounce time to 50 milliseconds
  timerSetup();
  ina260 = Adafruit_INA260();

  Serial.begin(115200);
  // Wait until serial port is opened
  while (!Serial) { delay(10);  }

  
  if (!ina260.begin()) {
    Serial.println("Couldn't find INA260 chip");
    while (1);
  }
  Serial.println("Found INA260 chip");

  mcp4921_begin();
  disableCurrent();

  Serial.println("Igniter Test");
}

void loop() {
  launchButton.loop(); // MUST call the loop() function first

  if (launchButton.isPressed()) {
    Serial.println("******Launch Button Pressed");
    uint16_t dac_value = get_current_selected();
    Serial.print("Current level selected: ");
    Serial.print(dac_value);
    Serial.print("  ");
    Serial.println(dac_value * (3.3 / 4096));
    launchTimerStart(current_duration, dac_value);
  }
  if (DONE) {
    printMeasurements();
    DONE = false;
  }
}

The output if the INA260A measurements are commented out:

Disabling current
Igniter Test
******Launch Button Pressed
Current level selected: 3723  3.00
***Launch Timer Start
Enabling current
***Launch Timer Started
***Launch Timer END
Disabling current
***duration = 1000
Measurements 0:   op_amp_value: 0.02   gate_value: 3.53   ref_value: 1.67   dac_out_value: 3.25
Measurements 1:   op_amp_value: 0.27   gate_value: 3.53   ref_value: 1.67   dac_out_value: 3.25
******Launch Button Pressed
Current level selected: 3723  3.00
***Launch Timer Start
Enabling current
***Launch Timer Started
***Launch Timer END
Disabling current
***duration = 1000
Measurements 0:   op_amp_value: 0.00   gate_value: 3.53   ref_value: 1.67   dac_out_value: 3.25
Measurements 1:   op_amp_value: 0.27   gate_value: 3.53   ref_value: 1.66   dac_out_value: 3.25

If the INA260A measurements are run:

Found INA260 chip
Disabling current
Igniter Test
******Launch Button Pressed
Current level selected: 3723  3.00
***Launch Timer Start
Enabling current
***Launch Timer Started
***Launch Timer END
Guru Meditation Error: Core  1 panic'ed (Interrupt wdt timeout on CPU1). 

Core  1 register dump:
PC      : 0x4037dc4c  PS      : 0x00060e34  A0      : 0x8037e758  A1      : 0x3fc96c30  
A2      : 0x3fc9fd04  A3      : 0x3fca3fd4  A4      : 0x00000000  A5      : 0x00060e23  
A6      : 0x3fc994dc  A7      : 0x0000abab  A8      : 0x3fca3fd4  A9      : 0x00000018  
A10     : 0x00000018  A11     : 0x00000000  A12     : 0xb33fffff  A13     : 0x00060e23  
A14     : 0x00000005  A15     : 0x0000abab  SAR     : 0x00000000  EXCCAUSE: 0x00000006  
EXCVADDR: 0x00000000  LBEG    : 0x40056f5c  LEND    : 0x40056f72  LCOUNT  : 0x00000000  
Core  1 was running in ISR context:
EPC1    : 0x4202e23f  EPC2    : 0x00000000  EPC3    : 0x00000000  EPC4    : 0x4037dc4c


Backtrace: 0x4037dc49:0x3fc96c30 0x4037e755:0x3fc96c50 0x4037ca96:0x3fc96c70 0x420167f2:0x3fc96cb0 0x42016a6a:0x3fc96ce0 0x42016b2d:0x3fc96d20 0x4201776d:0x3fc96d50 0x420047dd:0x3fc96de0 0x42002db7:0x3fc96e10 0x4202dce3:0x3fc96e40 0x420029e9:0x3fc96e60 0x42002a19:0x3fc96e80 0x4200284b:0x3fc96ea0 0x420028c1:0x3fc96ed0 0x4200272e:0x3fc96ef0 0x420022be:0x3fc96f30 0x403753ba:0x3fc96f60 0x40379b39:0x3fc96f80 0x40379c42:0x3fc96fa0 0x40377901:0x3fc96fd0 0x40379485:0x3fc96ff0 0x40381767:0x3fca3c80 0x40378066:0x3fca3cb0 0x4200588e:0x3fca3cd0 0x42003408:0x3fca3cf0 0x42002538:0x3fca3d10 0x42007f50:0x3fca3d30 0x4037cf72:0x3fca3d50


Core  0 register dump:
PC      : 0x4037d120  PS      : 0x00060034  A0      : 0x8037e245  A1      : 0x3fc96730  
A2      : 0x3fc95218  A3      : 0xffffffff  A4      : 0x400f8f7e  A5      : 0x00060023  
A6      : 0xb33fffff  A7      : 0x0000cdcd  A8      : 0x8037d129  A9      : 0x3fc96710  
A10     : 0x00000000  A11     : 0xb33f5454  A12     : 0x0000abab  A13     : 0xb81f8000  
A14     : 0x00060d23  A15     : 0x00000000  SAR     : 0x0000000e  EXCCAUSE: 0x00000006  
EXCVADDR: 0x00000000  LBEG    : 0x00000000  LEND    : 0x00000000  LCOUNT  : 0x00000000  


Backtrace: 0x4037d11d:0x3fc96730 0x4037e242:0x3fc96760 0x4037d7ab:0x3fc96780 0x4037d828:0x3fc967a0 0x40379485:0x3fc967c0 0x40379d87:0x3fca0230 0x4200c6ad:0x3fca0250 0x4037e10f:0x3fca0270 0x4037cf72:0x3fca0290

Given that I2C I/O in the ESP32 seems to require interrupt handlers (after a cursory reading of the docs), I would be quite surprised to hear that I2C I/O could be done in an interrupt context.

Try just setting a switch in the ISR, then in a normal state machine way examine that switch in the main loop and when on do whatever you need to do and turn off the switch.

I would also not use I2C within an interrupt service routine.

I'd probably set a flag here:

float ina260_current = ina260.readCurrent();        *****************

and do the actual reading and subsequent processing of the result in the loop() then unset the flag.

EDIT
. . . as @sonofcy has already just said more or less.

@6v6gt and @sonofcy I do not understand your recommendation, but it sounds a lot better than what I have been trying to do. Can you give me a bit more information about how to implement your recommendation?

What I do in these cases is just set a bool in the ISR, then in the loop, check the state of that bool (remember to declare it volatile), if it is set, then do all the code you now have in the ISR. You probably will miss some interrupts since you have so much there. Are the Serial. Prints really needed? And what is the need for the duration report?

Maybe something like this:

volatile bool measEndFlag = false ;  // global volatile 
. . .
. . .
void IRAM_ATTR launchTimerEnd() { 
  . . .
  // makeMeasurements(INDEX_END);  // moved from ISR to loop()
  measEndFlag = true;
  . . .
}

void loop() {
   . . .
   if ( measEndFlag == true ) {
       measEndFlag = false;
       makeMeasurements(INDEX_END);
   }
   . . .
}

This is what I did conceptually after some reflection on your suggestion. I am using the variable take_measurements as both a flag to indicate it is time to take a measurement and as the index for the measurement in the measurements array. (Long day and my creativity on variable names is waning....)

volatile int take_measurements = -1;

void launchTimerStart(uint64_t duration, int16_t dac_value) {
// existing code w/o makeMeasurements() call
take_measurements = 0;
}

void IRAM_ATTR launchTimerEnd() {
// existing code w/o makeMeasurements() call
take_measurements = 1;
}

void loop() {
// existing code
if (take_measurements != -1) {
    makeMeasurements(take_measurements);
    take_measurements = -1;
  }
}

I think this is what you meant? It seems to work now.

Output:

Found INA260 chip
Disabling current
Igniter Test
******Launch Button Pressed
Current level selected: 3723  3.00
***Launch Timer Start
Enabling current
***Launch Timer Started
***Launch Timer END
Disabling current
***duration = 1000
Measurements 0:   op_amp_value: 0.03   gate_value: 3.53   ref_value: 1.67   dac_out_value: 3.25   INA260a_current: -1.25   INA260A_bus_voltage: 20.00   INA260A_power: 0.00
Measurements 1:   op_amp_value: 0.00   gate_value: 0.00   ref_value: 0.00   dac_out_value: 0.00   INA260a_current: 0.00   INA260A_bus_voltage: 0.00   INA260A_power: 0.00

Don't worry too much about the actual values of the measurements. I don't have the power applied to the 3A circuit, so the INA260 is reading garbage.

A parallel question. I am accessing the SPI bus in the ISR to set the DAC value. Is that a mistake as well? It seems to work without complaining.

Thanks for the clarification! I ended up doing something similar as I wrote in my last post.

Do I need to do something similar for writing to the DAC with the SPI bus?

Yes, but make the variable take_measurements a bool.
Then use bool operations appropriately as in if (take_measurements) etc.

This is not the same thing. I2c was used to send a command to a device, waited for the result to be ready, then collected it. SPI is simply being used here to send a parameter to the DAC chip and SPI should be OK in an ISR.

I have read data from I2C based sensors in timer interrupt service routines, e.g. reading a LSM9DS1 9-axis iNEMO inertial module data every 20mSec into a double buffer (the main loop then transmits the data over BLE to a mobile phone)
it depends on how long the I2C sensor read takes and what else you are doing, e.g. processing the data - you need to experiment
in particular avoid output to the serial monitor, TFT displays, etc in ISRs - such IO can crash the program as in post 1 - place output data in volatile variables to be printed in loop()