Interrupt crashes code when implemented

Question

I've got a mini weather-station with a Davis Anemometer Vantage Pro that uses a reed switch to detect rotations. Using the rotations it then can convert to MPH. When using it alone, I have no problem measuring the wind speed. It measures the rotations over a 3 second period, and finds the average wind speed over those 3 seconds; however, when implemented into a group of code that also writes Temperature, Humidity, and Accelerometer data to an SD card, it crashes my code. For the sake of keeping this comment shorter, I am using code from http://cactus.io/hookups/weather/anemometer/davis/hookup-arduino-to-davis-anemometer-wind-speed, and am implementing into the code below. It's a big chunk, but the important stuff should be the ISR_rotation and enabling interrupts at the start of the loop.

// going to include all the libraries we need and definitions
#include "SD.h" //sd card
#include"SPI.h" //sd card
#include "SHT1x.h" //temp/humid
#include <Wire.h> //accelerometer
#include <Adafruit_MMA8451.h> //accelerometer
#include <Adafruit_Sensor.h> //accelerometer
#include <math.h> //for wind speed
//define pins and functions
#define dataPin 6 //data pin for temp/humid
#define sckPin 7 //serial clock for temp/humid
#define WindSensorPin (2) //pin for wind sensor
SHT1x th_sensor(dataPin, sckPin); //caling sht1x.h
Adafruit_MMA8451 mma = Adafruit_MMA8451(); //calling the accelerometer library

//WindSpeed stuff
volatile unsigned long Rotations; // cup rotation counter used in interrupt routine 
volatile unsigned long ContactBounceTime=0; // Timer to avoid contact bounce in interrupt routine 
float WindSpeed; // speed miles per hour 

//SD card stuff
const int chipSelect = 10;
String dataString =""; // holds the data to be written to the SD card
File sensorData;

//Temp/Humid stuff
float humid; // humidity value
float temp_c; // temperature value

void setup(){
  // Open serial communications
  Serial.begin(9600);
  Serial.print("Initializing SD card...");
  pinMode(chipSelect, OUTPUT); //sd writing
  pinMode(WindSensorPin, INPUT); //wind data
  attachInterrupt(digitalPinToInterrupt(WindSensorPin), isr_rotation, FALLING);//""wind interrupt""

  // see if the card is present and can be initialized:
  if (!SD.begin(chipSelect)) {
    Serial.println("Card failed, or not present");
  // don't do anything more:
  return;
  }
  //accelerometer setup
  if (! mma.begin()) {
    Serial.println("Couldnt start");
    while (1);
  }
  Serial.println("MMA8451 found!");
  mma.setRange(MMA8451_RANGE_2_G);
  Serial.print("Range = "); Serial.print(2 << mma.getRange());  
  Serial.println("G");
}

//
void loop(){
  //This interrupt portion is for incrementing Rotations
  Rotations = -22; // Set Rotations count to 0 ready for calculations 
  //interrupts(); // Enables interrupts 
  //delay (3000); // Wait 3 seconds to average 
  //noInterrupts(); // Disable interrupts 
  //convert to mp/h using the formula V=P(2.25/T) where T is the delay in seconds
  //V = P(2.25/3) = P * 0.75 
  //WindSpeed = Rotations * 0.75; 

  //read the temperature/humidity sensor
  humid=th_sensor.readHumidity();
  temp_c=th_sensor.retrieveTemperatureC();

  //read the 'raw' data in 14-bit counts for accelerometer
  //mma.x for x direction, mma.y for y direction, mma.z for z direction
  mma.read();
  //Convert 14 bit to SI units (m/s^2)
  sensors_event_t event; 
  mma.getEvent(&event);
  delay(3000);
  WindSpeed = Rotations * 0.75; 
  //read the wind speed data

  //build the data string
  dataString = String(temp_c) + "," + String(humid) + "," + String(event.acceleration.x) + "," + String(event.acceleration.y) + "," + String(event.acceleration.z);// + "," + String(WindSpeed); //to CSV
  saveData(); // save to SD card
  //for testing with serial monitor to see if we are obtaining correct values
  Serial.print("Temperature: ");
  Serial.print(temp_c);
  Serial.print(", Humidity: ");
  Serial.println(humid);
  Serial.print(" X: ");
  Serial.print(event.acceleration.x);
  Serial.print(" Y: ");
  Serial.print(event.acceleration.y);
  Serial.print(" Z: ");
  Serial.println(event.acceleration.z);
  Serial.print("Rotations: ");
  Serial.println(Rotations);
  delay(3000);
}

//
void saveData(){
if(SD.exists("data.csv")){ // check the card is still there
  // now append new data file
  sensorData = SD.open("data.csv", FILE_WRITE);
  Serial.println("Data written");
    if (sensorData){
    sensorData.println(dataString);
    sensorData.close(); // close the file
    }
  }
else{
  Serial.println("Error writing to file/Card removed");
  }
}

// This is the function that the interrupt calls to increment the rotation count 
void isr_rotation () { 
if ((millis() - ContactBounceTime) > 15 ) { // debounce the switch contact. 
  Rotations++; 
  ContactBounceTime = millis(); 
  } 
}

When any wind information and interrupts are commented out, I have no problem at all with writing to SD card and getting the correct data from the accelerometer and sht10. I must be doing something wrong with the interrupt, but I'm not sure. I've read to not include any serial prints, delays, keep em short, etc. and it works alone. It crashes right after running the setup (before it can serial print any information), which makes me believe it's something with the interrupt. I've tried commenting out noInterrupts() and it returns trash data for rotations, so that's not usable. In case it's important, the memory usage is in the 60% range, so that shouldn't be relevant.

Thanks for any help or advice you have.

EDIT: After totally removing the interrupt() and noInterrupts() portion, I get trash data for rotations that changes depending on the debounce timing (higher debounce limit=fewer triggered rotations). I seem to get a constant Rotations range of 20-23 that does pretty accurately depict how many rotations I add (for example spinning 8 times gets me around 30). I also prettied it up the code a little bit.

Answer 1

Your basic flaw is that you turn interrupts off (prior to commenting those lines out), and then attempt to use the SD card which almost certainly will want to use interrupts.

Your technique of turning interrupts on, and then doing a big delay, is not one I like. I comment on that sort of thing in my page about interrupts, in particular in reply #10. The essence of that reply is:

---

Timing an interval

I have seen example code suggesting you count things for a second by turning interrupts off for a second and then on again. I don't recommend this, because for one thing, you can't do serial prints if interrupts are off.

Example (not recommended):

volatile unsigned long events;
const unsigned long INTERVAL = 1000;  // 1 second

void eventISR ()     
  { 
  events++;  
  }   // end of eventISR

void setup () 
  { 
  Serial.begin(115200); 
  attachInterrupt (digitalPinToInterrupt (2), eventISR, FALLING); 
  }   // end of setup

void loop ()    
  {
  events = 0;       // reset counter
  interrupts ();    // allow interrupts    
  delay (INTERVAL); // wait desired time
  noInterrupts();   // stop interrupts

  Serial.print ("I counted ");
  Serial.println (events);
  }  // end of loop

(This is effectively what your code does).

That may work for simple situations, but turning interrupts off means that the Serial prints won't work until they are turned on again (which they may not be in a more complex sketch).

It is better to use the millis() result and just detect when the time limit is up.

Improved sketch:

volatile bool counting;
volatile unsigned long events;

unsigned long startTime;
const unsigned long INTERVAL = 1000;  // 1 second

void eventISR ()
  {
  if (counting)
    events++;    
  }  // end of eventISR

void setup ()
  {
  Serial.begin (115200);
  Serial.println ();
  attachInterrupt (digitalPinToInterrupt (2), eventISR, FALLING);
  }  // end of setup

void showResults ()
  {
  Serial.print ("I counted ");
  Serial.println (events);
  }  // end of showResults

void loop ()
  {
  if (counting)
    {
    // is time up?
    if (millis () - startTime < INTERVAL)
      return;  
    counting = false;
    showResults ();
    }  // end of if

  noInterrupts ();
  events = 0;
  startTime = millis ();
  EIFR = bit (INTF0);  // clear flag for interrupt 0
  counting = true;
  interrupts ();
  }  // end of loop

In loop() here we wait for the interval (1 second in this case) to be up, otherwise we return, effectively doing nothing. You could of course do other things instead of just returning.

If the time is up we display the count.

If counting is not currently active (and presuming we want it to be active) we remember the start time, reset the counter to zero, and let it start counting up.

That code seemed to work OK up to 100 kHz, although the counts were getting a bit inaccurate. You can do more precise timings by using the hardware counters / timers. Details at my Timers and counters page.