Using I2C inside of ISR causes program to stop running

I have three debounced push buttons, two of them increment/decrement a counter that gets displayed on the serial monitor and also toggles an LED every time one of them is pressed. The other push button resets my counter to 127. Additionally, every time one of the increment/decrement push buttons is pressed it uses SPI to increase the resistance of a potentiometer in a chip by 39 ohms. All of this is working fine.

Then I added I2C into my ISR functions. I want to increase/decrease the voltage on the Vout pin of the DAC chip by 19.5mV according to which push button is pressed. My program starts up fine, and is printing the counter on the serial monitor, but when I press any button my program seems to stop. I can tell because the serial monitor stops printing the counter and the LED's won't toggle anymore. What could be the issue here?

#include <SPI.h>
#include <Wire.h>

byte pot_address = 0x00; // Address for wiper 0
int CS = 4; // Chip select
int pot_value = 0x00; // Variable to send data to MCP4162-010 potentiometer
int dac_value = 0x00; // Variable to send data to MCP4725 I2C DAC


volatile int sw_count = 127; // Variable to keep track of the number of times the up and down push buttons have been pressed

#define green_led_toggle 31 // LED will change state any time the increment or decrement push button is pressed
#define yellow_led_indicator 33 // LED will turn on whenever the counter equals 127
#define red_led_indicator 35 // LED will indicate when the counter is either less than 0 or greater than 255

#define PB_UP  2  // Increment push button
#define PB_DOWN  3 // Decrement push button
#define PB_CLEAR  18 // Push button to reset sw_count to 127

void setup() {
  pinMode(CS, OUTPUT);
  SPI.beginTransaction(SPISettings(1000000, MSBFIRST, SPI_MODE0));
  SPI.begin();

  Wire.begin();
 
  Serial.begin(9600);  // Establish serial communication at 9600 bps

  pinMode(yellow_led_indicator, OUTPUT); pinMode(red_led_indicator, OUTPUT); pinMode(green_led_toggle, OUTPUT); // Declare output pins to turn on LED's
  
  pinMode(PB_UP, INPUT); pinMode(PB_DOWN, INPUT); pinMode(PB_CLEAR, INPUT); // Declare input pins to read push buttons

  // Declare interrupts for detecting button presses
  attachInterrupt(digitalPinToInterrupt(18),sw_count_clr,FALLING); // Interrupt for clear button
  attachInterrupt(digitalPinToInterrupt(2),sw_up,RISING); // Interrupt for increment button
  attachInterrupt(digitalPinToInterrupt(3),sw_down,RISING); // Interrupt for decrement button
}
 
void loop() {

  Serial.println(sw_count); // Print count to serial monitor

  if(sw_count == 127){ // Turn on yellow LED if counter equals 127
    digitalWrite(yellow_led_indicator, HIGH);
  }
  else{
    digitalWrite(yellow_led_indicator, LOW);
  }

  if((sw_count < 0) || (sw_count > 255)){ // Turn on red LED if counter is less than zero or greater than 255
    digitalWrite(red_led_indicator, HIGH);
  }
  else{
    digitalWrite(red_led_indicator, LOW);
  }
 
  delay(1000);

}
  
// -------------------- Functions ------------------------- //

void sw_count_clr(){ // Interrupt service routine triggerd by PB_CLEAR push button
  sw_count = 127;  
  digitalWrite(green_led_toggle, LOW);
}

void sw_up(){        // Interrupt service routine triggerd by PB_UP (incrementor) push button
  sw_count = sw_count + 1;
  digitalWrite(green_led_toggle, !digitalRead(green_led_toggle));

  pot_value = pot_value + 0x01; // Increase resistance by 39 ohms ( (39 / 10,000) * 255 = 1 = 01 in HEX )
  digitalWrite(CS, LOW);
  SPI.transfer(pot_address); // Send to wiper 0

  SPI.transfer(pot_value); // Send value to chip
  digitalWrite(CS, HIGH);

/* Program only works when commented out

  dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
  Wire.beginTransmission(0x60); // Address for MCP4725
  Wire.write(0x40); // Command to update the DAC
  Wire.write(dac_value); // Send value to DAC
  Wire.write(0x00);
  Wire.endTransmission();
  
*/
 
}

void sw_down(){ // Interrupt service routine triggerd by PB_DOWN (decrementor) push button
  sw_count = sw_count - 1;
  digitalWrite(green_led_toggle, !digitalRead(green_led_toggle));

  pot_value = pot_value - 0x01; // Increase resistance by 39 ohms ( (39 / 10,000) * 255 = 1 = 01 in HEX )
  digitalWrite(CS, LOW);
  SPI.transfer(pot_address); // Send to wiper 0

  SPI.transfer(pot_value); // Send value to chip
  digitalWrite(CS, HIGH);


/* Program only works when commented out

  dac_value = dac_value - 0x10;  // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
  Wire.beginTransmission(0x60);  // Address for MCP4725
  Wire.write(0x40);  // Command to update the DAC
  Wire.write(dac_value);  // Send value to DAC
  Wire.write(0x00);
  Wire.endTransmission();
  
*/ 

}

1. The MCU disables the interrupt logic before entering into an ISR routine; whereas, I2C bus needs enabled interrupt logic for its operation. Also, delay() function needs enabled interrupt logic for its functioning.

2. Therefore, re-organize your program logic and codes so that I2C codes are handled separately.

3. You should keep minimum codes in the ISR like setting up just a flag. Perform the bulk works in the loop() function based on the flag value.

Are you sure that you need to use interrupts to detect button presses ?

Have not heard of the concept of flags before. Do you mean putting a variable inside the ISR functions like this:

void sw_up(){
up_flag = 1;

}

So every time the ISR is executed, the flag will become true and I can execute my code in the main loop using an if statement?

Hi,
Why do you need to use interrupts?
How fast do you need to detect a button press?

What is your project for, what is its application?

Thanks.. Tom... :slight_smile:

TomGeorge:
Hi,
Why do you need to use interrupts?
How fast do you need to detect a button press?

What is your project for, what is its application?

Thanks.. Tom... :slight_smile:

I am required to use interrupts to detect the push button presses. I need to detect the push buttons instantaneously it seems, based on what it says in the assignment. It's a school assignment combining debouncing, interrupts, SPI and I2C. This program is to demonstrate that we understand these concepts. All it does is toggle some LED's based on push button presses, tracks an incrementing/decrementing counter and uses SPI/I2C to change resistance/voltage on chips. The interrupts are triggered by debounced push buttons.

I added flags into my ISR functions, took the I2C code out of my functions and put it into the main loop to be executed when the flags are true. My program works now and doesn't stop functioning when I press a button. My problem now is the voltage is incrementing by 300mV instead of 19.5mV like I want. I am certain my math checks out for converting 19.5mV to a HEX value that is sent to the I2C DAC:

( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX ) **** 12 bits: 2^12 = 4095 ****

  if(up_flag == 1){
    dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
    Wire.beginTransmission(0x60); // Address for MCP4725
    Wire.write(0x40); // Command to update the DAC
    Wire.write(dac_value); // Send value to DAC
    Wire.write(0x00);
    Wire.endTransmission();

    delay(200);
  }

Not sure where I'm going wrong here. Here is my entire program updated with I2C code in main loop now:

#include <SPI.h>
#include <Wire.h>

byte pot_address = 0x00; // Address for wiper 0
int CS = 4; // Chip select
int pot_value = 0x00; // Variable to send data to MCP4162-010 potentiometer
int dac_value = 0x00; // Variable to send data to MCP4725 I2C DAC

volatile int up_flag = 0;
volatile int down_flag = 0;
volatile int sw_count = 127; // Variable to keep track of the number of times the up and down push buttons have been pressed

#define green_led_toggle 31 // LED will change state the increment or decrement push button is pressed
#define yellow_led_indicator 33 // LED will turn on whenever the counter equals 27
#define red_led_indicator 35 // LED will indicate when the counter is either less than 0 or greater than 255

#define PB_UP  2  // Increment push button
#define PB_DOWN  3 // Decrement push button
#define PB_CLEAR  18 // Push button to reset sw_count to 127

void setup() {
  pinMode(CS, OUTPUT);
  SPI.beginTransaction(SPISettings(1000000, MSBFIRST, SPI_MODE0));
  SPI.begin();

  Wire.begin();
 
  Serial.begin(9600);  // Establish serial communication at 9600 bps

  pinMode(yellow_led_indicator, OUTPUT); pinMode(red_led_indicator, OUTPUT); pinMode(green_led_toggle, OUTPUT); // Declare output pins to turn on LED's
  
  pinMode(PB_UP, INPUT); pinMode(PB_DOWN, INPUT); pinMode(PB_CLEAR, INPUT); // Declare input pins to read push buttons

  // Declare interrupts for detecting button presses
  attachInterrupt(digitalPinToInterrupt(18),sw_count_clr,FALLING); // Interrupt for clear button
  attachInterrupt(digitalPinToInterrupt(2),sw_up,RISING); // Interrupt for increment button
  attachInterrupt(digitalPinToInterrupt(3),sw_down,RISING); // Interrupt for decrement button
}
 
void loop() {
  Serial.println(sw_count); // Print count to serial monitor

  if(sw_count == 127){ // Turn on yellow LED if counter equals 127
    digitalWrite(yellow_led_indicator, HIGH);
  }
  else{
    digitalWrite(yellow_led_indicator, LOW);
  }

  if((sw_count < 0) || (sw_count > 255)){ // Turn on red LED if counter is less than zero or greater than 255
    digitalWrite(red_led_indicator, HIGH);
  }
  else{
    digitalWrite(red_led_indicator, LOW);
  }

  if(up_flag == 1){
    dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
    Wire.beginTransmission(0x60); // Address for MCP4725
    Wire.write(0x40); // Command to update the DAC
    Wire.write(dac_value); // Send value to DAC
    Wire.write(0x00);
    Wire.endTransmission();

    delay(200);
  }

  if(down_flag == 1){
    dac_value = dac_value - 0x10;  // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
    Wire.beginTransmission(0x60);  // Address for MCP4725
    Wire.write(0x40);  // Command to update the DAC
    Wire.write(dac_value);  // Send value to DAC
    Wire.write(0x00);
    Wire.endTransmission();

    delay(200);
  }

  up_flag = 0;
  down_flag = 0;
  
  delay(1000);
}
  
// -------------------- Functions ------------------------- //

void sw_count_clr(){ // Interrupt service routine triggerd by PB_CLEAR push button
  sw_count = 127;  
  digitalWrite(green_led_toggle, LOW);
}

void sw_up(){        // Interrupt service routine triggerd by PB_UP (incrementor) push button  
  up_flag = 1;
  
  sw_count = sw_count + 1;
  digitalWrite(green_led_toggle, !digitalRead(green_led_toggle));

  pot_value = pot_value + 0x01; // Increase resistance by 39 ohms ( (39 / 10,000) * 255 = 1 = 01 in HEX )
  digitalWrite(CS, LOW);
  SPI.transfer(pot_address); // Send to wiper 0

  SPI.transfer(pot_value); // Send value to chip
  digitalWrite(CS, HIGH); 
}

void sw_down(){ // Interrupt service routine triggerd by PB_DOWN (decrementor) push button  
  down_flag = 1;
  
  sw_count = sw_count - 1;
  digitalWrite(green_led_toggle, !digitalRead(green_led_toggle));

  pot_value = pot_value - 0x01; // Increase resistance by 39 ohms ( (39 / 10,000) * 255 = 1 = 01 in HEX )
  digitalWrite(CS, LOW);
  SPI.transfer(pot_address); // Send to wiper 0

  SPI.transfer(pot_value); // Send value to chip
  digitalWrite(CS, HIGH);
}

@Artemis7 - yes, that is how you use flags in interrupt routines. Get in, do the minimum you need to do (set a flag etc) and get out again. Let the main code detect the flag(s) being set.

Don't forget to declare your flag(s) as 'volatile', which basically tells your code to check the value of the variable/flag every time it uses it.

@op
I would first check if the input is bouncing.(incrementing multiple time).
Also would you mind to use the internal pull-up resistors and change the ISR from rising to falling.
By pulling-up the inputs you reduce noise.
Thanks

Checked the input for bouncing. It's not, I can tell because my sw_count variable is printed on to the serial monitor and gets incremented/decremented by 1 each time a push button is pressed.

Artemis7:
Checked the input for bouncing. It's not, I can tell because my sw_count variable is printed on to the serial monitor and gets incremented/decremented by 1 each time a push button is pressed.

I think you will find when you eventually make your program work, that you will detect bounces. At the moment, there is inherent debouncing because there is code running while the switch bounces. It's been explained to you that you can't run that code, so the problem will probably appear.

It was my math :-[ 8) . The DAC is 12 bits but I am only using 8 bits to write to it. Instead of 2^12=4095 I switched it to 2^8=255 and used that.

(19.5mV / 5V) * 255 = 1 = 0x01 in HEX. Changed 0x10 to 0x01 in my code. Problem solved. Thanks for the help with the I2C in ISR problem.

Artemis7:
(19.5mV / 5V) * 255 = 1 = 0x01 in HEX. Changed 0x10 to 0x01 in my code. Problem solved. Thanks for the help with the I2C in ISR problem.

You've reversed the math that produced the 19.5mV in the first place. HEX is not an appropriate representation for this, you are actually representing the ordinal number 1, that expresses how many ADC steps are in an ADC step: ADC/ADC == 1. To imply that it is some special digital value by making it HEX is a huge "canard" or "red herring".

You're very lucky that the value wasn't mangled by the truncation of conversion to float and then back to integer.

You have

    dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )

should be

    dac_value++; // Increase voltage by one ADC step (19.5 mV / 5V) * 4095 in this case

aarg:
You've reversed the math that produced the 19.5mV in the first place. HEX is not an appropriate representation for this, you are actually representing the ordinal number 1, that expresses how many ADC steps are in an ADC step: ADC/ADC == 1. To imply that it is some special digital value by making it HEX is a huge "canard" or "red herring".

You're very lucky that the value wasn't mangled by the truncation of conversion to float and then back to integer.

You have

    dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )

should be

    dac_value++; // Increase voltage by one ADC step (19.5 mV / 5V) * 4095 in this case

Ah yes...that makes sense. Whoops. Thanks for the correction.

If you wanted to step by more than one ADC value, you would just create a step variable/constant and use:

    dac_value += dac_incrementValue; // Increase voltage by N ADC steps (19.5 mV / 5V) * 4095 in this case

Artemis7:
I added flags into my ISR functions, took the I2C code out of my functions and put it into the main loop to be executed when the flags are true. My program works now and doesn't stop functioning when I press a button. My problem now is the voltage is incrementing by 300mV instead of 19.5mV like I want.

if(up_flag == 1)
{
    dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
    Wire.beginTransmission(0x60); // Address for MCP4725
    Wire.write(0x40); // Command to update the DAC
    Wire.write(dac_value); // Send value to DAC
    Wire.write(0x00);
    Wire.endTransmission();

    delay(200);
  }

The above DAC updating codes should be re-written as follows to comply with the 4-byte command format for MCP4725 DAC (Fig-1).

if (up_flag == 1) 
{
  //Full sacle = 5V, Step = 4096, LSB = 1.22 mV when input is 0x01, for input 16, Vout = 19.52 mV
  dac_value = dac_value + 0x10; // Increase voltage by 19.5 mV ( (19.5 mV / 5V) * 4095 = 16 = 10 in HEX )
  Wire.beginTransmission(0x60); // Address for MCP4725
  Wire.write(0x40); // Command to update the DAC
  byte hByteA = highByte(dac_value)<<4;  //D11 D10 D9 D8 0   0    0    0
  byte hByteB = lowByte(dac_value)>>4;   //0   0   0   0   D7  D6  D5 D4
  byte hByte = hByteA|hByteB;            //D11 D10 D9 D8 D7 D6 D5 D4
  Wire.write(hByte); //see Fig-1 below
  Wire.write(lowByte(dac_value)<<4);     //D3 D2 D1 D0 0 0 0 0   see Fig-1 below 
  Wire.endTransmission();

  delay(200);
}

MCP4725Command.png
Figure-1: 4-byte command format for MCP4725 DAC

MCP4725Command.png

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.