Interrupts not counting while using millis()

Hey,

i'm building a weather station which includes:

  • windspeed (interrupt counting)

  • winddirection (Analog input voltage measurement)

  • temp and humidity (dht11)

All of the data is written to my thingspeak channel to plot it.

The general setup is working fine but i noticed that the winddirection measurements are jumping up and down, so i included smoothing (12 measurements in a minute) for the analog input. To get this to work i had to change all my delays to millis() to get an analog reading every 5 seconds and to update all the other readings every minute to the thingspeak channel. Now the function to count my interrupts is not working anymore. I can't count any interrupts. I tried to get information from several older threads in this forum bit i couldn't work out a solution for my problem and i don't understand why using interrupts and millis seems to be a problem.

Maybe someone can explain what my problem is and how to fix it, thanks in advance.

#include "DHT.h"
#include "ThingSpeak.h"
#include <WiFiNINA.h>

#define WIFI_SSID "XXXXXX"
#define WIFI_PASSWORD "XXXXXX"

WiFiClient client;

unsigned long aufnahmeStartMillis;
unsigned long fahneStartMillis;
unsigned long currentMillis;

const unsigned long aufnahmePeriod = 60000;
const unsigned long fahneAveragePeriod = 5000;


const int numReadings = 12;
int readings[numReadings];     
int readIndex = 0;             
int total = 0;               
int average = 0;                
int windDirectionVoltage = 0;

int inputPin = A0;


unsigned long seconds = 1000L;
unsigned long minutes = seconds * 60;

unsigned long aufnahmezeit = minutes;
const byte interruptPin = 2;
int InterruptCounter;


float WindSpeed;
char* windDirection;
float humid;
float temp;


char thingSpeakAddress = "api.thingspeak.com";
unsigned long myChannelNumber = "XXXXX";
const char * myWriteAPIKey = "XXXXXXXXX";


#define DHTPIN 4
#define DHTTYPE DHT11

DHT dht(DHTPIN, DHTTYPE);
String myStatus = "";
int number = 0;
int windDegrees = 0;


void setup() {
  Serial.begin(9600);
  dht.begin();
  WiFi.begin(WIFI_SSID, WIFI_PASSWORD);
  Serial.print("connecting...");
  while (WiFi.status() != WL_CONNECTED) {
    Serial.print(".");
    delay(500);
  }
  Serial.println();
  Serial.print("connected: ");
  Serial.println(WiFi.localIP());

  ThingSpeak.begin(client);

  aufnahmeStartMillis = millis();
  fahneStartMillis = millis();

  for (int thisReading = 0; thisReading < numReadings; thisReading++) {
    readings[thisReading] = 0;
  }

}


void loop() {

  currentMillis = millis();
  
  messung();
  windrichtung();
 

}

void windrichtung() {

  if (currentMillis - fahneStartMillis >= fahneAveragePeriod) 
  {
    
  total = total - readings[readIndex];
  readings[readIndex] = analogRead(inputPin);
  total = total + readings[readIndex];
  readIndex = readIndex + 1;

  if (readIndex >= numReadings) {
    readIndex = 0;
  }

  windDirectionVoltage = total / numReadings;
  
  Serial.print("WindDirectionVoltage: ");
  Serial.println(windDirectionVoltage);

    fahneStartMillis = currentMillis;

  }

  
}

void messung() {
          
  InterruptCounter = 0;

  attachInterrupt(digitalPinToInterrupt(interruptPin), count, RISING);

  if (currentMillis - aufnahmeStartMillis >= aufnahmePeriod) {

  detachInterrupt(digitalPinToInterrupt(interruptPin));
    
  if (windDirectionVoltage >= 212 && windDirectionVoltage < 273)    {
    windDirection = "N";
  }
  else if (windDirectionVoltage >= 577 && windDirectionVoltage < 665) {
    windDirection = "NNE";
  }
  else if (windDirectionVoltage >= 483 && windDirectionVoltage < 577) {
    windDirection = "NE";
  }
  else if (windDirectionVoltage >= 929 && windDirectionVoltage < 943) {
    windDirection = "ENE";
  }
  else if (windDirectionVoltage >= 906 && windDirectionVoltage < 929) {
    windDirection = "E";
  }
  else if (windDirectionVoltage >= 943 && windDirectionVoltage < 1023) {
    windDirection = "ESE";
  }
  else if (windDirectionVoltage >= 795 && windDirectionVoltage < 858) {
    windDirection = "SE";
  }
  else if (windDirectionVoltage >= 858 && windDirectionVoltage < 906) {
    windDirection = "SSE";
  }
  else if (windDirectionVoltage >= 665 && windDirectionVoltage < 748) {
    windDirection = "S";
  }
  else if (windDirectionVoltage >= 748 && windDirectionVoltage < 795) {
    windDirection = "SSW";
  }
  else if (windDirectionVoltage >= 348 && windDirectionVoltage < 399) {
    windDirection = "SW";
  }
  else if (windDirectionVoltage >= 399 && windDirectionVoltage < 483) {
    windDirection = "WSW";
  }
  else if (windDirectionVoltage >= 0 && windDirectionVoltage < 106)   {
    windDirection = "W";
  }
  else if (windDirectionVoltage >= 163 && windDirectionVoltage < 212) {
    windDirection = "WNW";
  }
  else if (windDirectionVoltage >= 106 && windDirectionVoltage < 163) {
    windDirection = "NW";
  }
  else if (windDirectionVoltage >= 273 && windDirectionVoltage < 348) {
    windDirection = "NNW";
  }

  if (windDirection == "N") {
    windDegrees = 360;
  }
  else if (windDirection == "NNE") {
    windDegrees = 22;
  }
  else if (windDirection == "NE") {
    windDegrees = 45;
  }
  else if (windDirection == "ENE") {
    windDegrees = 68;
  }
  else if (windDirection == "E") {
    windDegrees = 90;
  }
  else if (windDirection == "ESE") {
    windDegrees = 112;
  }
  else if (windDirection == "SE") {
    windDegrees = 135;
  }
  else if (windDirection == "SSE") {
    windDegrees = 158;
  }
  else if (windDirection == "S") {
    windDegrees = 180;
  }
  else if (windDirection == "SSW") {
    windDegrees = 202;
  }
  else if (windDirection == "SW") {
    windDegrees = 225;
  }
  else if (windDirection == "WSW") {
    windDegrees = 248;
  }
  else if (windDirection == "W") {
    windDegrees = 270;
  }
  else if (windDirection == "WNW") {
    windDegrees = 292;
  }
  else if (windDirection == "NW") {
    windDegrees = 315;
  }
  else if (windDirection == "NNW") {
    windDegrees = 338;
  }


  humid = dht.readHumidity();
  temp = dht.readTemperature();

  WindSpeed = (float(InterruptCounter) / (float(60)) * 2.4);

  Serial.print("Umdrehungen: ");
  Serial.println(InterruptCounter);
  Serial.print("WindSpeed: ");
  Serial.print(WindSpeed);
  Serial.print("km/h / ");
  Serial.print(WindSpeed / 3.6);
  Serial.println("m/s" );
  Serial.print("Windrichtung: ");
  Serial.println(windDirection);
  Serial.print("WindDirectionVoltage: ");
  Serial.println(windDirectionVoltage);
  Serial.print("Humidity: ");
  Serial.println(humid);
  Serial.print("Temperature: ");
  Serial.println(temp);

  // set the fields with the values
  ThingSpeak.setField(1, WindSpeed);
  ThingSpeak.setField(2, windDirection);
  ThingSpeak.setField(3, humid);
  ThingSpeak.setField(4, temp);
  ThingSpeak.setField(5, windDegrees);

  // set the status
  ThingSpeak.setStatus(myStatus);

  // write to the ThingSpeak channel
  int x = ThingSpeak.writeFields(myChannelNumber, myWriteAPIKey);


  if (x == 200) {
    Serial.println("Channel update successful.");
  }
  else {
    Serial.println("Problem updating channel. HTTP error code " + String(x));
  }

    
   aufnahmeStartMillis = currentMillis;
   }

    
}
  

void count() {
  InterruptCounter++;

}

Your isr variable

Should be declared volatile
volatile unsigned int InterruptCounter;

(Probably better as an unsigned too)

1 Like

Thank you for the response but this didn't solve my problem. I still can't count interrupts.

You should not be doing this in loop(). It should be once in setup().

  detachInterrupt(digitalPinToInterrupt(interruptPin));

Why this? Why are you attaching and detaching an interrupt constantly?

you're right, it was just the way that i saw other examples do it. I changed this but still no luck with the interrupt counting.

This read of an ISR variable has no critical section to protect it. There are probably other interrupt related mistakes, often the first one happens by guessing, and so also the rest do.

Read:
https://gammon.com.au/interrupts

If, generally, you need to obtain a count from an interrupt, it's safer to let it run, and just zero the counter whenever you want to initiate a count, than to enable/disable them.

It’s ok to attach and detach
The idea was to monitor how many interrupts you get during a given time that you measure

I’m reading this on my phone and it’s not well indented code but you probably need some flags that say you have reached the timeout and it’s time to publish. You don’t want to reset the count to zero at each call either

This should be done in setup and reset when the time is out and you have use the count

If the program logic doesn't create side effects. It also might have problems on platforms other than AVR, don't forget that those are rapidly increasing in number. A lot of cores are wrappers of existing APIs. There is plenty of potential for trouble there.

The idea was to monitor how many interrupts you get during a given time that you measure

Means to an end. The basic idea is to monitor how many input transitions there are during a time interval. There are so many better ways to do that. Another is to read the ISR value at the start of the interval, and again at the end of the interval, and subtract.

A good reason for a detach() would be to release an ISR that has a heavy overhead. This one definitely doesn't. Both, because of the extreme simplicity of the ISR, and the extremely low input data rate.

I don’t dispute that.

Care to highlight what you have in mind? What kind of trouble?

that's OK because those interrupts are disabled in this approach, so no-one is messing with the bytes behind your back.

to go back to your question, best would probably be to re-architect your code

  • use a millis() based approach as you did to assess an average winddirection
  • during that time let the interruption tick in
  • when the time allocated for the wind direction is done, calculate everything needed (either detaching the interrupt as you did or make a critical section where you duplicate the count and reset it) and publish

if you run two counters, you are making your life more difficult because the average winddirection you have might have been calculated at a different time than the speed

Your sketch is not compiled on UNO. Please, see below the error message. Which Board are you using? There is error besides warnings.

Arduino: 1.8.19 (Windows 10), Board: "Arduino Uno"

C:\Users\GM\Documents\Arduino\libraries\WiFiNINA-master\src/utility/wifi_drv.h:293:12: error: 'PinStatus' does not name a type

     static PinStatus digitalRead(uint8_t pin);

            ^~~~~~~~~

C:\Users\GM\Documents\Arduino\sketch_jul16c\sketch_jul16c.ino:42:26: warning: invalid conversion from 'const char*' to 'char' [-fpermissive]

 char thingSpeakAddress = "api.thingspeak.com";

                          ^~~~~~~~~~~~~~~~~~~~

C:\Users\GM\Documents\Arduino\sketch_jul16c\sketch_jul16c.ino:43:33: warning: invalid conversion from 'const char*' to 'long unsigned int' [-fpermissive]

 unsigned long myChannelNumber = "XXXXX";

                                 ^~~~~~~

exit status 1

Error compiling for board Arduino Uno.


That is not how you compare char arrays in C++. Use strcmp()

1 Like

Thank you so much for all these responses. To be honest i found a quite simple to solution to my earlier problem. I'm now attaching the interrupt in setup() but i'm reseting it back to 0 at the end of messung() which seems to work fine. I also added a "critical section" section for the interrupt counting like @anon57585045 suggested. Currently i'm also never detaching.

Regarding the solution @J-M-L described here: I will try this in the next few days. You are right, my average winddirection will be calculated at a slightly different time than the speed which does not really matter for my usage right now.

I'm running on UNO WiFi REV 2 and compiling without any issues

Yes! This could be that the UNO is an issue.

One potential pitfall, no implementation that I know of, guarantees any kind of time frame for enabling/disabling. So rapid alternation there could cause a collision, or at least not the results that you expect in your program flow (trivially, nearly instantaneous response). I do admit it's hypothetical, but this kind of use might fall outside what the designers expected it to be used for, and so might not have that level of assurance.

I’m not sure I get what you mean, when the Function call returns you have attached or not the callback. And in this specific case, it’s not rapid, it was collecting data for 60 seconds (so if you miss à few microseconds it’s not significant)

That would be the assurance I want exactly, that attachment has fully occurred before the return. Under some circumstances it could be a problem. Not here though, you're right.

What makes me wonder too, I don't think I've ever written any ISR software that tested this. When you "kick off" an interrupt you are basically sitting there waiting for a response from it, so it could be instant or it could be a few microseconds, who would notice? Or, from a sensor that it won't see for several milliseconds...

The way it typically works is registering the callback pointer in a table and enabling the interrupt

Given the API level, I believe it’s guaranteed that when the function comes back everything is setup

I’ve seen code working both ways - and it makes some sense to not record interrupts if you don’t care about them at that point in time.

I agree though that just letting it run and take into account time elapsed when you access the count in a protected section is probably a simpler design

I ran into another issue with my code. After 2-3 hours working fine, it just stops sending data. It doesn‘t loose connection but it just stops working. Any idea what the issue could be?