Counting the duration of a process with interrupts, Trouble

Hello everyone,

I am trying to measure how long a machine is on. The machine turns on and off througout the day (like and AC on a thermostat) I came up with this sketch that uses interrupts and millis for the count. It works flawlessly for one machine, meaning one ISR.

With 2 ISRs I get all kinds of errors. Not reading one input, not counting correctly the duration , etc.

I wonder if there's a smarter way of doing this.

By the way, "SendData (MACH_1, MACH_1_DUR );" is sending the data through a radio modem NRF24L01, to another arduino to collect, sum up, make percentages...

Should it not be in the ISR? But I need to send the data at the end of each duration count. What to do? Thanks

Here is the code. (using a mega)

in the setup I got:



  pinMode(MACH_1_PIN, INPUT_PULLUP);
  pinMode(MACH_2_PIN, INPUT_PULLUP);
  pinMode(MACH_3_PIN, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(MACH_1_PIN), TimeStamp_1, FALLING);
  attachInterrupt(digitalPinToInterrupt(MACH_2_PIN), TimeStamp_2, FALLING);

Then the functions:


void TimeStamp_1 () // interrupt falling, time stamp
{
  MACH_1_TimeStamp = millis();

  detachInterrupt(digitalPinToInterrupt(MACH_1_PIN)) ;
  attachInterrupt(digitalPinToInterrupt(MACH_1_PIN), TimeCount_1, RISING);
}

void TimeCount_1 ()// interrupt Rising
{ MACH_1_DUR = ( millis() -  MACH_1_TimeStamp );
  if (MACH_1_DUR > 100) // debouncing, if duration is smaller than 100ms .
  { //Serial.print ("MACH_1_DUR : ");
    //Serial.println (MACH_1_DUR);
    SendData (MACH_1, MACH_1_DUR );
  }
  attachInterrupt(digitalPinToInterrupt(MACH_1_PIN), TimeStamp_1, FALLING);
}

void TimeStamp_2 () // interrupt falling, time stamp
{ //Serial.print ("MACH_2_TimeStamp: ");
  MACH_2_TimeStamp = millis();
  //Serial.println (MACH_2_TimeStamp);
  detachInterrupt(digitalPinToInterrupt(MACH_2_PIN)) ;
  attachInterrupt(digitalPinToInterrupt(MACH_2_PIN), TimeCount_2, RISING);
}

void TimeCount_2 ()// interrupt Rising
{ MACH_2_DUR = ( millis() -  MACH_2_TimeStamp );
  if (MACH_2_DUR > 100)
  { SendData (MACH_2, MACH_2_DUR );
  }
  attachInterrupt(digitalPinToInterrupt(MACH_2_PIN), TimeStamp_2, FALLING);
}



Then the Send data:

void SendData (String MACH, unsigned long DUR)

{    
      //String AVG_MFG_STR = String(AVG_MFG*0.01);
     
      String PeriodMillis_STR =  String (DUR);
     
      String StrToSend = MACH+" "+ PeriodMillis_STR  +" "+ Channel;
       Serial.println (StrToSend);
      
      char text[32] ;
      StrToSend.toCharArray(text, 32);
      
      radio.write(&text, sizeof(text));
      
      delay (random (5,15));
      radio.write(&text, sizeof(text));


  
  }

It's likely that you can get accurate enough results by polling the pins and not using interrupts. In my experience, pin interrupts are not simple and Arduino doesn't make them any more simple. My understanding is that on the ATmega328P, for example, there are 5 total, including INT0 and INT1, and 3 that are divided among ports B, C, and D. I am oversimplifying that, but like I said, it's not a simple thing... It's likely easier to simply poll the pins and your results will be accurate enough, to the millisecond... with proper coding.

I don't understand why you would use delay(...) when you are using millis(), and I REALLY don't understand why your delay(...) would be a random number.

Interrupt Service Routines can be difficult to debug and usually should be avoided. Unless you need high accuracy or your machines are on for milliseconds, it is likely that you can do everything that you want in the loop() function.

What kind of accuracy are you trying to get? How long is a machine turned on?

I am not saying that this task cannot be performed with interrupts, I am just saying that interrupts may be making this harder than necessary.

I am using delay in the transmission sketch in order to send the same String twice. In case the radio had interference, I am safe that way. There are more devices talking to the receiver. If they all repeat at 10mS than they can interfere again. If they repeat at a random no, than they wont, (or is a small chance).

On the receiver end, if I wrote a little condition, that throws out the string if it is identical with the previous one. Is the problem here? I commented out the repeat transmission, it still gets erroneus readings.

I am trying to get an accuracy of 100mS,

Why use interrupts: They are super responsive and are event based. I need to get a time stamp in order to do a count. A digital read == 0 is not event based, it is continuous.

Is there a way to write a time stamp when a pin changes from HI to Low in the loop? Please help,

Also , I am trying to get good readings if the machines come on and off at the same time. The ISR will read the time stamp correctly even if it is simultaneous. (I put both inputs on one switch)

On the way out, (TimeCount_1 ()) the erroneous readings happen.

Thanks

laptophead: Here is the code. (using a mega)

Please post the complete program. If it is too long to include in your next Reply then please add your .ino file as an attachment.

I am inclined to the same view as @Perehama in Reply #1

...R

1) Strings with a capital "S" are an abomination and will make spaghetti mashed stew of your RAM heap. Replace them with c-strings. 2) You can enclose your serial data in a protocol that takes care of the addressing, to validate and discard cross transmission and erroneous data, as much as your serial overhead will allow for. This is how modern data is communicated. Consider using some of the non-printing characters, a checksum, delimiters, etc. along with timeouts in the receiving code, instead of delays and random stuff, which is not more random than the already random transmission times. 3) polling is better than interrupts for what you want to do. If it's a relay contact (typical machine interface), the way you are using interrupts, doesn't even debounce the input. When I refer to polling, digitalRead() is only a component to the total code you might need. In 100ms, an Arduino UNO (ATmega328P running at 16MHz) can perform 1600000 MIPS, which means it can likely execute through your entire code several times over.

Great advice, I need to read on C Strings, never used them.

But still the unanswered issue for me is:

How to write a digital Read based sketch that will count the mS between when my read goes LOW and when it goes HIGH again.

Anyone has an example to point to?

Thanks

laptophead: Great advice, I need to read on C Strings, never used them.

But still the unanswered issue for me is:

How to write a digital Read based sketch that will count the mS between when my read goes LOW and when it goes HIGH again.

Anyone has an example to point to?

Thanks

class PushButton {
  public:
    PushButton(byte p) : buttonPin(p) {}
    void inputPullup() const {
      pinMode(buttonPin, INPUT_PULLUP);
    }
    bool isPressed() {
      bool b = false;
      if (debounceFlag == 1) {
        if (millis() >= debounceStopTime) debounceFlag = 0;
      }
      else if (debounceFlag == 0) {
        int currentState = digitalRead(buttonPin);
        int stateChange = currentState - previousState;
        if (stateChange == fallingEdge) {
          debounceStopTime = millis() + DebounceInterval;
          b = true;
          debounceFlag = 1;
        }
        else if (stateChange == risingEdge) {
          debounceStopTime = millis() + DebounceInterval;
          debounceFlag = 1;
        }
        previousState = currentState;
      }
      return b;
    }
    const byte buttonPin;

  private:
    int previousState = HIGH;
    byte debounceFlag = 0;
    unsigned long debounceStopTime;
    const static unsigned long DebounceInterval = 20UL;//change constant value to change debounce time
    const static int risingEdge = HIGH - LOW;
    const static int fallingEdge = LOW - HIGH;
};

This is my go to pushbutton debounce. I can't take full credit for writing it, but I use it all the time. Now, if you take the class method isPressed() and call that "hasFallen()" or something like that... if you understand each line in this code, you can see i require a rising edge to register another falling edge, but I ignore entirely the rising edge.... Well, if you can understand this code, it wouldn't be too hard to write a class method for the rising edge. Alternatively, consider Bounce2.

Cool, I will get to work. Thanks