Interrupt triggers each cycle

Hello Community,

This is my first post in this forum, but it helped me already a lot by only reading threads to solve many problems by myself. Thank you all for that.
Now I run into a problem I couldnt find an existing answer to, or dont know the right words to look for.
Project Description:
The Arduino Nano is mounted on an Ethernet shield (ENC28J60) and communicating over Serial with another Arduino Nano. Both are collecting information via a DHT22 sensor. The communication is done by a dataframe of bytes, which is send and received by all three parties:
PC<-- (UDP)-->Arduino1<--(Serial)-->Arduino2
Problem:
Arduino 1 shall measure wind speed by an Anemometer with a simple reed contact.
Each revolution should trigger an interrupt on D2 and the time between the new and previous interrupt is calculated into the wind speed.
Basically the measurement works fine, but the interrupt is also triggered everytime the Arduino loops through the code. This results in a measured time of one run thorugh the program (about 700ms).
What could be the reason for that and how can I avoid it?

The complete Program would several hundret lines, so I post only the code related to the wind speed / interrupt functionality:

const int WindSens = 2;
float wind_speed;

unsigned long t_pulse0 = 0;
unsigned long t_pulse1;
int tmin = 50;    // [msec]

void measure(){
  
  unsigned long dt;

  t_pulse0 = t_pulse1;
  t_pulse1 = millis();

  dt = t_pulse1-t_pulse0;

  if(dt>0){
    if(dt>tmin){
    wind_speed = 1000/(0.5*dt);                        // 1U/s = 2m/s 
    //Serial.println((String)"\n Delta: "+dt);
    dt = 0;
    }
    else{
      return;
    }
  }
  else{
    wind_speed = 0;
  }
}

void setup(){
pinMode(WindSens, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(WindSens), measure, FALLING);
}

void loop(){
if(millis()-t_pulse1 > 5000){       // no Wind recognition
    wind_speed = 0;
  }
}

best regards
Tobias

This should be volatile.

and this: unsigned long t_pulse1;

Which board are you using? an 8 bit one ?

The same for t_pulse1

If you use a multi byte variable ( wind_speed, t_pulse1) in loop() that is also used in the ISR, you must switch the interrupts off while writing/reading this variable in loop().

Why do you think so?

Post code that actually demonstrates the problem.

Wow, that was fast, thanks for your replies.
@6v6gt @MicroBahner,
thanks for the hint "volatile".
I am pretty sure its an 8 bit board. Its a Nano clone with USB-C.
I suspected it triggering each cycle because I got a measurement about each 700 seconds, and this is approximately the runtime for one cycle. It contains a delay of 673ms, an odd number on purpose.
@jremington
I just tried to replicate the error with only the parts related to the interrupt, but without the rest of the code it actually works as intended.

I guess my question is more like: What are the usual suspects, that could trigger an interrupt, even if not related?..

Here the working partial code:

const int WindSens = 2;
volatile float wind_speed;

unsigned long t_pulse0 = 0;
volatile unsigned long t_pulse1;
int tmin = 50;    // [msec]

void measure(){
  
  unsigned long dt;

  t_pulse0 = t_pulse1;
  t_pulse1 = millis();

  dt = t_pulse1-t_pulse0;

  if(dt>0){
    if(dt>tmin){
    wind_speed = 1000/(0.5*dt);                        // 1U/s = 2m/s 
    //Serial.println((String)"\n Delta: "+dt);
    dt = 0;
    }
    else{
      return;
    }
  }
  else{
    //wind_speed = 0;
  }
}

void setup(){
  Serial.begin(9600);
  pinMode(WindSens, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(WindSens), measure, FALLING);
}

void loop(){
  if(millis()-t_pulse1 > 3000){       // no Wind recognition
    wind_speed = 0;
  }
  Serial.println((String)wind_speed+" m/s");
  delay(700);
}

If you can't reproduce the problem in your reduced sketch, then the usual things like bouncing contacts can be ruled out.
If pin 2 is reused anywhere else in your program that may trigger interrupts which are nothing to do with the rotation of the Anemometer. Does, for example, the Ethernet shield use this pin ?
Ensure that there is some indication of setup() running so you can identify a cyclical crash.

The "rest" is probably where the error is, and that is why the forum rules request you to post ALL the code.

I don't see that you've heeded the advice offered by @MicroBahner in #3 upthread.

Many ppl will grab a copy of a volatile variable when used outside the ISR context, viz:

  noInterrupts();
  float myWindSpeed = wind_seed;
  interrupts();

and then use the copy elsewhere. In your case, simpler:

  if(millis()-t_pulse1 > 3000){       // no Wind recognition
    noInterrupts();
    wind_speed = 0;
    interrupts();
  }

It can be that doing is unecessary, by shgeer luck of the circumstances. But luck is fickle, and circumstances change - these are difficult errors to find when they do crop, so best to go by the book even if you have been successfully ignoring that aspect of interrupt programming.

Don't ask me how I know this. :expressionless:

HTH

a7

With the declanation of "volatile", the complete code works as intended,
intil I plug in the Ethernet cable, then the error ocurrs.

I did not try the "noInterrupts()" function yet.
The code is not cleaned up yet and has a lot of commented debugging outputs and is not wiped of (small) unused variables.

Here is the complete code (500 lines) :

#include <Wire.h>
#include "DHT.h"
#include <EthernetENC.h>  // for  ENC28J60
#include <EthernetUdp.h>  // for UDP
#define DHTpin 7
#define DHTTYPE DHT22

DHT dht(DHTpin, DHTTYPE);
IPAddress localIP(169, 254, 0, 42);
IPAddress remoteIP(169, 254, 0, 24);

//Pin Definition
const int Fan = 9;
const int Heater = 8;
const int LNAbox = 5;
const int HPApin = 4;     //Muss final 4 sein!
const int WindSens = 2;   //Muss final 2 sein!
//Parameter
volatile float wind_speed;
int wind_speed100;
float Dew;
float a, b, c, d, e; //formel variablen
int Tmax = 30;
int Tmin = 10;
int HS = 5;   //Dew Hysteresis
unsigned long t_pulse0 = 0;
volatile unsigned long t_pulse1;
int tmin = 20;    // [msec]

//Ethernet
unsigned int localPort = 10000;   // local port to listen on
unsigned int remotePort = 10000;  // remote port to transmit too
// Enter a MAC address
byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
// An EthernetUDP instance to let us send and receive packets over UDP
EthernetUDP Udp;

//Frame Build
uint8_t frame[63];
uint8_t arr4[4], arr2[2];
uint32_t SYNC;
unsigned long sync = 0x1ACFFC1D; 
uint16_t StatusBits;
float TempLNA, HygroLNA, GyroX, GyroY, GyroZ, CurrLNA, TempBox, HygroBox;
short TmaxLNA, TminLNA, TmaxBox, TminBox, HygroStep = 5, Wind;
uint16_t ComBits;
uint8_t ComTmaxLNA, ComTminLNA, ComTmaxBox, ComTminBox, ComHygroStep;

bool PeltierPWR, PeltierSW, FanIN, FanOUT, LNA, BoxHeater, BoxFan, LNAboxPWR, HPA, CRCok;
bool comPeltierPWR, comPeltierSW, comFanIN, comFanOUT, comLNA, comBoxHeater, comBoxFan, comLNAboxPWR, comHPA, comOverride;

// print IPAdress and port
void displayIPaddress(const IPAddress address, unsigned int port) {
  Serial.print(" IP ");
  for (int i = 0; i < 4; i++) {
    Serial.print(address[i], DEC);
    if (i < 3) Serial.print(".");
  }
  Serial.print(" port ");
  Serial.println(port);
}

void displayMACaddress(byte address[]) {
  Serial.print("MAC address ");
  for (int i = 0; i < 6; i++) {
    Serial.print("0x");    
    Serial.print(address[i], HEX);
    if (i < 5) Serial.print(".");
  }
  Serial.println();
}

uint16_t checksumCalculator(uint8_t * data, uint16_t length){
   uint16_t curr_crc = 0x0000;
   uint8_t sum1 = (uint8_t) curr_crc;
   uint8_t sum2 = (uint8_t) (curr_crc >> 8);
   int index;
   for(index = 0; index < length; index = index+1)
   {
      sum1 = (sum1 + data[index]) % 255;
      sum2 = (sum2 + sum1) % 255;
   }
   return (sum2 << 8) | sum1;
}

uint32_t get32BitWord(uint8_t * data, int pos){
  uint32_t byte0 = data[pos];
  uint32_t byte1 = data[pos+1];
  uint32_t byte2 = data[pos+2];
  uint32_t byte3 = data[pos+3];
  
  uint32_t val = (byte0 << 24) + (byte1 << 16) + (byte2 << 8) + byte3;
  return val;
}

void SerialInput(){
  
  const int BUFFER_SIZE = 130; //130
  uint8_t buf[BUFFER_SIZE]; // directly casted to 1 Byte!
  uint16_t CRC;

  if (Serial.available() > 0) {
    //Serial.println("Received");
    uint8_t seek = Serial.readBytes(buf, BUFFER_SIZE);
    
    /*for(int n=0; n<63; n++){
      sframe[n] = (uint8_t)buf[n];
    }*/

    Serial.println("\nBuffer:");      // Show read bytes for Debugging
    for(int i=0; i<130; i++){
      Serial.print(buf[i], HEX);
    }

    for(int i=0; i<65; i++){
      if(buf[i]==0x1a && buf[i+1]==0xcf && buf[i+2]==0xfc && buf[i+3]==0x1d){     // seek SYNC word
        //Serial.println("Start found!");
        for(int n=0; n<63; n++){
          frame[n] = buf[n+i];    // write found frame into Frame Array
        }
      };
    }
    CRC = (frame[62] << 8) + frame[61];
    uint16_t crc = checksumCalculator(frame,61);
    SYNC = get32BitWord(frame, 0);

    if(CRC == 0){
      //Serial.println("CRC = 0"); 
      CRCok = false;
      bitWrite(StatusBits, 6, 0);
      if(SYNC == sync){  
        Serial.println("\nRx Frame CRC Broken");
      }
      else{
        Serial.println("\nRx Frame Broken");
      }
      return;
    }
    
    if((crc == CRC) && (sync == SYNC)){
      
      //Serial.println("OK");
      
      StatusBits = (frame[4] << 8) + frame[5];
      /*TempLNA = get32BitWord(frame, 6);         //Not needed on this controller
      HygroLNA = get32BitWord(frame, 10);
      GyroX = get32BitWord(frame, 14); 
      GyroY = get32BitWord(frame, 18);
      GyroZ = get32BitWord(frame, 22);
      CurrLNA = get32BitWord(frame, 26); 
      Wind = (frame[30] << 8) + frame[31];
      TempBox = get32BitWord(frame, 32);
      HygroBox = get32BitWord(frame, 36);
      TmaxLNA = (frame[40] << 8) + frame[41];
      TminLNA = (frame[42] << 8) + frame[43];
      HygroStep = (frame[48] << 8) + frame[49];

      /*Read Status Bits*/
      /*PeltierPWR = bitRead(StatusBits, 15);
      PeltierSW = bitRead(StatusBits, 14);
      FanIN = bitRead(StatusBits, 13);
      FanOUT = bitRead(StatusBits, 12);
      LNA = bitRead(StatusBits, 11);
      BoxHeater = bitRead(StatusBits, 10);
      BoxFan = bitRead(StatusBits, 9);
      LNAboxPWR = bitRead(StatusBits, 8);
      HPA = bitRead(StatusBits, 7);*/
      
      CRCok = true;
      bitWrite(StatusBits, 6, 1);
    }
    else{
      CRCok = false;
      bitWrite(StatusBits, 6, 0);
      if(SYNC == sync){  
        Serial.println("\nRx Frame CRC Broken");
      }
      else{
        Serial.println("\nRx Frame Broken");
      }
      for(int i=0; i<63; i++){
        Serial.print(frame[i], HEX);}
      
      Serial.println("No Rx Frame");
      for(int i=6; i<30; i++){        // delete Data from LNA Box
        frame[i] = 0;}
    }
  }
  else{
    Serial.println("No Rx Frame");
    for(int i=6; i<30; i++){        // delete Data from LNA Box
        frame[i] = 0;}
    
    return;
  }
}

void floatToByte(byte* arr4, float value){
  long l = *(long*) &value;

  arr4[3] = l & 0x00FF;
  arr4[2] = (l >> 8) & 0x00FF;
  arr4[1] = (l >> 16) & 0x00FF;
  arr4[0] = l >> 24;
}

void longToByte(byte* arr4, long value){    //for Sync Word
  long l = *(long*) &value;

  arr4[3] = l & 0x00FF;
  arr4[2] = (l >> 8) & 0x00FF;
  arr4[1] = (l >> 16) & 0x00FF;
  arr4[0] = l >> 24;
}

void intToByte(byte* arr2, int value){
  short s = *(short*) &value;

  arr2[1] = s & 0x00FF;
  arr2[0] = s >> 8;
}

void measure(){
  
  unsigned long dt;

  t_pulse0 = t_pulse1;
  t_pulse1 = millis();

  dt = t_pulse1-t_pulse0;

  if(dt>0){
    if(dt>tmin){
    wind_speed = 1000/(0.5*dt);                        // 1U/s = 2m/s 
    //Serial.println((String)"\n Delta: "+dt);
    dt = 0;
    }
    else{
      return;
    }
  }
  else{
    //wind_speed = 0;
  }
  
}

void UDPOutput(){
  
  longToByte(arr4, sync);
  frame[0] = arr4[0];
  frame[1] = arr4[1];
  frame[2] = arr4[2];
  frame[3] = arr4[3];
  
  intToByte(arr2, StatusBits);
  frame[4] = arr2[0];
  frame[5] = arr2[1];

  intToByte(arr2, wind_speed100);
  frame[30] = arr2[0];
  frame[31] = arr2[1];
  floatToByte(arr4, TempBox);
  frame[32] = arr4[0];
  frame[33] = arr4[1];
  frame[34] = arr4[2];
  frame[35] = arr4[3];
  floatToByte(arr4, HygroBox);
  frame[36] = arr4[0];
  frame[37] = arr4[1];
  frame[38] = arr4[2];
  frame[39] = arr4[3];
  intToByte(arr2, TmaxBox);
  frame[44] = arr2[0];
  frame[45] = arr2[1];
  intToByte(arr2, TminBox);
  frame[46] = arr2[0];
  frame[47] = arr2[1];
  intToByte(arr2, HygroStep);
  frame[48] = arr2[0];
  frame[49] = arr2[1];

  /*intToByte(arr2, ComBits);
  frame[50] = arr2[0];
  frame[51] = arr2[1];*/

  /*frame[52] = ComTmaxLNA; //ComFrame[52];
  frame[53] = ComTminLNA; //ComFrame[53];
  frame[54] = ComTmaxBox; //ComFrame[54];
  frame[55] = ComTminBox; //ComFrame[55];
  frame[56] = ComHygroStep; //ComFrame[56];*/

  frame[58] = 0;
  frame[59] = 0;
  frame[60] = 0;

  uint16_t crc = checksumCalculator(frame,61);
  intToByte(arr2, crc);
  frame[61] = arr2[1];
  frame[62] = arr2[0];

  /*Serial.println();
  for(int i=0; i<63; i++){
    Serial.print(frame[i], HEX);
  }*/

  Udp.begin(localPort);
  Udp.beginPacket(remoteIP, remotePort);  // transmit data
  for(int i=0; i<63; i++){
    Udp.write(frame[i]);
  }
  Udp.endPacket();
}

void UDPpass(){

  uint8_t ComFrame[63];

  int packetSize = Udp.parsePacket();
  if (packetSize) {
    
    Udp.read(ComFrame, packetSize);

    /*Serial.println();
    for(int i=0; i<63; i++){
      Serial.print(ComFrame[i], HEX);
    }
    Serial.println();*/
    for(int i=0; i<63; i++){
      ComFrame[i] = (uint8_t)ComFrame[i];}
      //Serial.print(ComFrame[i], HEX);}


    ComBits = (ComFrame[50] << 8) + ComFrame[51];
    ComTmaxLNA = ComFrame[52]; 
    ComTminLNA = ComFrame[53];
    ComTmaxBox = ComFrame[54];
    ComTminBox = ComFrame[55];
    ComHygroStep = ComFrame[56];

    if(ComTmaxLNA == 0){TmaxLNA = Tmax;}
    if(ComTminLNA == 0){TminLNA = Tmin;}
    if(ComTmaxBox == 0){TmaxBox = Tmax;}
    if(ComTminBox == 0){TminBox = Tmin;}
    if(ComHygroStep == 0){HygroStep = HS;}

    /*Read Command Bits*/
    /*comPeltierPWR = bitRead(ComBits, 15);
    comPeltierSW = bitRead(ComBits, 14);
    comFanIN = bitRead(ComBits, 13);
    comFanOUT = bitRead(ComBits, 12);
    comLNA = bitRead(ComBits, 11);*/
    comBoxHeater = bitRead(ComBits, 10);
    comBoxFan = bitRead(ComBits, 9);
    comLNAboxPWR = bitRead(ComBits, 8);
    comHPA = bitRead(ComBits, 7);
    comOverride = bitRead(ComBits, 6);

    uint16_t crc = checksumCalculator(ComFrame,61);
    intToByte(arr2, crc);
    ComFrame[61] = arr2[1];
    ComFrame[62] = arr2[0];
    

    for(int i=0; i<63; i++){
    Serial.write(ComFrame[i]);
    //Serial.print(frame[i], HEX);
    }

  }
}

void setup(){

  Serial.begin(9600);
  pinMode(DHTpin, INPUT);
  pinMode(Fan, OUTPUT);
  pinMode(Heater, OUTPUT);
  pinMode(LNAbox, OUTPUT);
  pinMode(HPA, OUTPUT);
  pinMode(WindSens, INPUT_PULLUP);
  dht.begin();
  attachInterrupt(digitalPinToInterrupt(WindSens), measure, FALLING);
  Ethernet.init(10);  // Most Arduino shields
  mac[5] = localIP[3];  // change default MAC address
  displayMACaddress(mac);
  Ethernet.begin(mac, localIP);
  if (Ethernet.hardwareStatus() == EthernetNoHardware) {
    Serial.println("Ethernet shield was not found.  Sorry, can't run without hardware. :(");
    while (true) delay(1);  // do nothing, no point running without Ethernet hardware
  }
  if (Ethernet.linkStatus() == LinkOFF) {                 // seems not to work, to be Deleted ???
    Serial.println("Ethernet cable is not connected.");
  }
  Udp.begin(localPort);  // start UDP
  Serial.print("Ethernet UDP started ");
  displayIPaddress(localIP, localPort);
  //Serial.println(UDP_TX_PACKET_MAX_SIZE);
}

  
void loop(){

  UDPpass();
  
  SerialInput();
  
  HygroBox = dht.readHumidity();
  TempBox = dht.readTemperature();

  if(millis()-t_pulse1 > 3000){       // no Wind recognition
    wind_speed = 0;
  }

  Serial.println((String)wind_speed+" m/s");
  /*Serial.println(t_pulse0);
  Serial.println(t_pulse1);
  Serial.println(t_pulse1-t_pulse0);*/

  wind_speed100 = wind_speed*100;

  a = 17.62*TempBox;
  b = 243.12+TempBox;
  c = 17.62*243.12;
  d = HygroBox/100;
  e = (a/b+log(d))/(c/b-log(d));
  Dew = 243.12*e;
  //Serial.println((String)Dew+" deg Dewpoint");

  if(comOverride){
    if(comLNAboxPWR){
      digitalWrite(LNAbox, HIGH);
      bitWrite(StatusBits, 8, 1);}
    else{
      digitalWrite(LNAbox, LOW);
      bitWrite(StatusBits, 8, 0);}
    if(comHPA){
      digitalWrite(HPApin, HIGH);
      bitWrite(StatusBits, 7, 1);}
    else{
      digitalWrite(HPApin, LOW);
      bitWrite(StatusBits, 7, 0);}
    if(comBoxHeater){
      digitalWrite(Heater, HIGH);
      bitWrite(StatusBits, 10, 1);}
    else{
      digitalWrite(Heater, LOW);
      bitWrite(StatusBits, 10, 0);}
    if(comBoxFan){
      digitalWrite(Fan, HIGH);
      bitWrite(StatusBits, 9, 1);}
    else{
      digitalWrite(Fan, LOW);
      bitWrite(StatusBits, 9, 0);}
    if(ComTmaxBox!=0){
      Tmax = ComTmaxBox;
      TmaxBox = ComTmaxBox;}
    if(ComTminBox!=0){
      Tmin = ComTminBox;
      TminBox = ComTminBox;}
    if(ComHygroStep!=0){
      HS = ComHygroStep;
      HygroStep = ComHygroStep;}
  }
  else{
    if(TempBox>Tmax){
    digitalWrite(Fan, HIGH);
    bitWrite(StatusBits, 9, 1);}
    else{
    digitalWrite(Fan, LOW);
    bitWrite(StatusBits, 9, 0);}

    if(TempBox<Dew+HygroStep || TempBox<TminBox){
    digitalWrite(Heater, HIGH);
    bitWrite(StatusBits, 10, 1);}
    else{
    digitalWrite(Heater, LOW);
    bitWrite(StatusBits, 10, 0);}

  }
  
  bitWrite(StatusBits, 0, 0);
  bitWrite(StatusBits, 1, 0);
  bitWrite(StatusBits, 2, 0);
  bitWrite(StatusBits, 3, 0);
  bitWrite(StatusBits, 4, 0);
  bitWrite(StatusBits, 5, 0);

  //bitWrite(StatusBits, 8, 1);   //LNA Box PWR ON ----- Dummy for Test

  //Serial.println(StatusBits, BIN);

  UDPOutput();
  delay(673);     //uneven number to avoid congruence
}  

So, you choose to ignore important advice.

I think that on some of the modules, pin2 is used as an interrupt source for various events, such as when an ethernet link comes up or a wake-on-lan event occurs.

Try moving your wind sensor interrupt to pin3.

Yes, i saw the message just before replying to give you the complete code.
There was no chance to implement and test it in between, thats why i wrote "yet".
Now I have it implemented, but unfortunately, it doesnt help.
@a7 Thanks for the advice.
@cattledog Pin 2 is not used by the Ethernet Shield based on the datasheet. But I will try to do that tomorrow. :+1:

Perhaps a long shot right here. But good to make a habit of.

At least it will keep ppl from pointing it out over and over, and may leave your feet out of the water in the future!

a7

If you have a RAM problem because of those libraries and locally defined variables, the latter incidentally don't show up in the compiler summary storage report, then consider using the F() macro for such strings to specify that these cannot exist in RAM:

Serial.println(F("Ethernet shield was not found. . .") ) ;

Ok , I got the solution.
Before trying to switch to D3, it was easier first to try desoldering the D2 Pin from the Ethernet shield and connected the Pin directly to the Sensor, since D3 is used otherwise.
This makes it work perfectly.
Apparently, the Ethernet shield was triggering the D2 Pin everytime it received a message, not every cycle as I thought.

Thanks everyone for your help!

@6v6gt
I dont understand how this is related to the issue, but I already saw this hint and will implement it in the future if running into RAM problems.
Ideally the program has no Serial message output, except the communication with the other Arduino. EDIT: Is it possible to use the F macro with Serial.write() ? So I could use it with sending the byte frame in order to lighten the RAM?

Good that it is now solved.

Low RAM can cause all sorts of strange behaviour and the false triggering of the interrupt could have been a symptom of this. The compiler storage use summary is not a reliable guide of RAM usage if there is considerable use dynamically allocated RAM as in the case of your program (big arrays defined local to a function).

The F() macro is appropriate only for constant strings which are known at compile time and resolves to a function for extracting a specific string directly from flash memory. It would not work for the variable data you build up in the frame you send out.

It is good practice to use F() when printing constant strings. In the single example I quoted from your program (post #14), you could have saved about 60 bytes.

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