cli() and sei() delay needed?

I'm using interrupts and I ran into a weird problem after sei(). The code won't work correctly if I don't include at least a two second delay after sei()...is there a reason why you would need a delay after sei()? This is a basic outline of the setup and loop:

void setup(void) {

cli(); //temporarily disable interrupts before configuring

//code to configure inputs/outputs and interrupt settings }

void loop(void) {

sei(); //enable global interrupts delay(2000); //WITHOUT THIS DELAY, THE CODE DOESN'T WORK //code to read analog sensor values and take average

//more code...

}

whatever9: is there a reason why you would need a delay after sei()?

Yup. Because you have a bug somewhere in here...

   //code to configure inputs/outputs and interrupt settings

Or, because you failed to account for the fact that enabling interrupts has two side effects, there could be a bug here...

sei(); //enable global interrupts
  //code to read analog sensor values and take average

What two side effects are you talking about? What bug would cause a delay to make it work correctly?

These are my interrupt settings:

//enable pin change interrupt 1 (for PCINT[11:8])
sbi(GIMSK,PCIE1);
sbi(PCMSK1,PCINT8);

//enable pin change interrupt 0 (for PCINT[7:0])
sbi(GIMSK,PCIE0);
sbi(PCMSK0,PCINT7);
sbi(PCMSK0,PCINT0);
sbi(PCMSK0,PCINT1);
sbi(PCMSK0,PCINT2);

//set analog reference to VCC
cbi(ADMUX,REFS1);
cbi(ADMUX,REFS0);

Apparently my sarcasm was too weak. Maybe a more on-the-nose approach will have the desired effect.

http://snippets-r-us.com/

Please post all your code. In code tags.

How to use this forum

Ok, I have a large program so I reduced it to what wasn’t working, and this simpler code still doesn’t work unless there is the two second delay (on the third line of Loop), so it appears to not be an interrupt issue.

#include <Interrupt.h> 
#include <TinyWireM.h>
#include <Adafruit_MCP4725.h>
#include <avr/sleep.h>

Adafruit_MCP4725 dac;

int value1;
int power_on = 1;
int i;
unsigned long sample;
unsigned long startMillis;
unsigned long signalMax[20] = {0};
unsigned long sum = 0, average;
int sensor_in = A5; 

void setup(void)
{
  
   dac.begin(0x62); //set DAC address 
   pinMode(sensor_in,INPUT); 
   
}





void loop(void)
{
  
  if (power_on)
  {
    power_on = 0;
    
  delay(2000); //THIS IS NEEDED OR ELSE IT DOESN'T WORK
  
      

    for (i =0; i < 20; i++) 
      {
        startMillis= millis(); 
        while (millis() - startMillis < 50) //find maximum peak in 50 ms window
        {
        
           sample = analogRead(sensor_in); //read sensor input
         
          if (sample < 1024) // toss out spurious readings
          {
            if (sample > signalMax[i])
            {
              signalMax[i] = sample; // save just the max levels
            }
          }
        }
     
      }
   
          for(i = 0; i < 20; i++)
          {
            sum = signalMax[i] + sum;  
            
          }
          
          average = sum/20; //average over 1 second
          
          value1 = average*4+350; 
        
     
        sum = 0; //reset sum
        
         //reset peak values
         for(i = 0; i < 20; i++)
         {
            signalMax[i] = 0;
         }
    

    
    dac.setVoltage(value1, false); //output sensor average
    
     
   
    sleep_function(); //go to sleep
  }

}  
 
  
  void sleep_function(void)
{
  
  
    set_sleep_mode(SLEEP_MODE_PWR_DOWN); 

    sleep_mode(); 

}

This is from the DAC library:

/**************************************************************************/
/*! 
    @file     Adafruit_MCP4725.cpp
    @author   K.Townsend (Adafruit Industries)
	@license  BSD (see license.txt)
	
	I2C Driver for Microchip's MCP4725 I2C DAC

	This is a library for the Adafruit MCP4725 breakout
	----> http://www.adafruit.com/products/935
		
	Adafruit invests time and resources providing this open source code, 
	please support Adafruit and open-source hardware by purchasing 
	products from Adafruit!

	@section  HISTORY

    v1.0 - First release
*/
/**************************************************************************/
#if ARDUINO >= 100
 #include "Arduino.h"
#else
 #include "WProgram.h"
#endif

#include <Wire.h>

#include "Adafruit_MCP4725.h"

/**************************************************************************/
/*! 
    @brief  Instantiates a new MCP4725 class
*/
/**************************************************************************/
Adafruit_MCP4725::Adafruit_MCP4725() {
}

/**************************************************************************/
/*! 
    @brief  Setups the HW
*/
/**************************************************************************/
void Adafruit_MCP4725::begin(uint8_t addr) {
  _i2caddr = addr;
  Wire.begin();

}
 
/**************************************************************************/
/*! 
    @brief  Sets the output voltage to a fraction of source vref.  (Value
            can be 0..4095)

    @param[in]  output
                The 12-bit value representing the relationship between
                the DAC's input voltage and its output voltage.
    @param[in]  writeEEPROM
                If this value is true, 'output' will also be written
                to the MCP4725's internal non-volatile memory, meaning
                that the DAC will retain the current voltage output
                after power-down or reset.
*/
/**************************************************************************/
void Adafruit_MCP4725::setVoltage( uint16_t output, bool writeEEPROM )
{
  uint8_t twbrback = TWBR;
  TWBR = ((F_CPU / 400000L) - 16) / 2; // Set I2C frequency to 400kHz
  Wire.beginTransmission(_i2caddr);
  if (writeEEPROM)
  {
    Wire.write(MCP4726_CMD_WRITEDACEEPROM);
  }
  else
  {
    Wire.write(MCP4726_CMD_WRITEDAC);
  }
  Wire.write(output / 16);                   // Upper data bits          (D11.D10.D9.D8.D7.D6.D5.D4)
  Wire.write((output % 16) << 4);            // Lower data bits          (D3.D2.D1.D0.x.x.x.x)
  Wire.endTransmission();
  TWBR = twbrback;
}
/**************************************************************************/
/*! 
    @file     Adafruit_MCP4725.h
    @author   K. Townsend (Adafruit Industries)
	@license  BSD (see license.txt)
	
	This is a library for the Adafruit MCP4725 breakout board
	----> http://www.adafruit.com/products/935
	
	Adafruit invests time and resources providing this open source code, 
	please support Adafruit and open-source hardware by purchasing 
	products from Adafruit!

	@section  HISTORY

    v1.0  - First release
*/
/**************************************************************************/

#if ARDUINO >= 100
 #include "Arduino.h"
#else
 #include "WProgram.h"
#endif

#include <Wire.h>

#define MCP4726_CMD_WRITEDAC            (0x40)  // Writes data to the DAC
#define MCP4726_CMD_WRITEDACEEPROM      (0x60)  // Writes data to the DAC and the EEPROM (persisting the assigned value after reset)

class Adafruit_MCP4725{
 public:
  Adafruit_MCP4725();
  void begin(uint8_t a);  
  void setVoltage( uint16_t output, bool writeEEPROM );

 private:
  uint8_t _i2caddr;
};

if (sample < 1024) // toss out spurious readings

This will always be true, analogs are 0-1023.

.

Can you clarify what "doesn't work" means? What do you expect to happen? What actually happens?

LarryD, yeah I know, I'm not sure why that was included in sample code I got, and I just left it there for some reason :)

I still have no idea why I need that 2 second delay! Nick, with the 2 second delay, I get a DAC output of about 2V as expected, but if I don't have the delay, it's close to 0V.

The code snippet in your Original Post suggests there is a huge gap - both in time and in logic - between the CLI and SEI.

I would expect CLI to be followed closely by SEI with just a few lines of code in between - usually to allow a multi-byte value to be read without being altered by an ISR.

Don't use them to control when a particular interrupt is introduced. Use the specific code for switching the interrupt on or off for that.

If you want to use interrupts in a way that is not accommodated by the Arduino attachInterrupt() etc then there is no alternative to RTFM.

...R

Thanks for the suggestion, on that topic just wondering if you individually switch all interrupts off, what's the point of using cli() to disable all interrupts? Sorry the actual problem turned out to be unrelated to this...I'm still trying to figure out where in the code it's making it necessary to have the 2 second delay...could analogRead need this long of a delay?

You normally turn all interrupts off when you need to access a variable which an interrupt routine might change, and you don't want any interrupt to corrupt it. This is normally for a very brief period.

As for analogRead, that doesn't need any delay. In itself it takes 104 µs to run.

whatever9:
just wondering if you individually switch all interrupts off, what’s the point of using cli() to disable all interrupts?

I think you are confusing the business of stopping an interrupt completely with stopping it temporarily (and briefly). When you switch an interrupt off (by detachInterrupt() for example) no more of those interrupts happen. But when you stop interrupts with CLI an interrupt that happens after CLI and before SEI will be processed immediately after SEI.

…R

Why are you even doing this? You shouldn't have to.

Ok, I think I understand the interrupt turning off topic now... I'm still trying to figure out the need for this 2 second delay :o Maybe using analogRead requires a delay after the DAC code?

whatever9: Maybe using analogRead requires a delay after the DAC code?

I guess you did not read Reply #11. Pity Nick was wasting his time.

And it is an ADC, not a DAC.

...R

What is connected to A5?

Robin2, I am using both the ADC in an Attiny84, and a separate DAC chip. I thought maybe there was some interaction that was causing a problem and requiring a greater delay. Coding Badly, an analog distance sensor is connected to A5.

Indeed, if I remove the DAC code and use an LED to indicate whether the average sensor value is correct, I get a good result without needing the 2 second delay at the beginning of Loop...so I think it must be an issue with the Wire library or DAC...

whatever9: Robin2, I am using both the ADC in an Attiny84, and a separate DAC chip.

I did not realize that, and you did say "Maybe using analogRead requires a delay after the DAC code?"

Post the latest version of your code.

...R

The original code I posted doesn’t work without the delay (analogRead returns 0), and here it is again:

#include <Interrupt.h>
#include <TinyWireM.h>
#include <Adafruit_MCP4725.h>
#include <avr/sleep.h>

Adafruit_MCP4725 dac;

int value1;
int power_on = 1;
int i;
unsigned long sample;
unsigned long startMillis;
unsigned long signalMax[20] = {0};
unsigned long sum = 0, average;
int sensor_in = A5;

void setup(void)
{
 
   dac.begin(0x62); //set DAC address
   pinMode(sensor_in,INPUT);
   
}





void loop(void)
{
 
  if (power_on)
  {
    power_on = 0; //don't run this routine again
   
  delay(2000); //THIS IS NEEDED OR ELSE IT DOESN'T WORK
 
     

    for (i =0; i < 20; i++)
      {
        startMillis= millis();
        while (millis() - startMillis < 50) //find maximum peak in 50 ms window
        {
       
           sample = analogRead(sensor_in); //read sensor input
         
          if (sample < 1024) // toss out spurious readings
          {
            if (sample > signalMax[i])
            {
              signalMax[i] = sample; // save just the max levels
            }
          }
        }
     
      }
   
          for(i = 0; i < 20; i++)
          {
            sum = signalMax[i] + sum; 
           
          }
         
          average = sum/20; //average over 1 second
         
          value1 = average*4+350;
       
     
        sum = 0; //reset sum
       
         //reset peak values
         for(i = 0; i < 20; i++)
         {
            signalMax[i] = 0;
         }
   

   
    dac.setVoltage(value1, false); //output sensor average
   
     
   
    sleep_function(); //go to sleep
  }

} 
 
 
  void sleep_function(void)
{
 
 
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);

    sleep_mode();

}