0

I have been writing code for measuring when the digital signal comes and goes (ON and OFF respectively). I have done the part calculating a single system's downtime (difference in timings between ON and OFF from the system), but when I'm not able to scale up to multiple systems when I try to copy the same code which worked above. So it would be helpful if anyone could provide some insight into how it can be done plus explaining with the code would help.

My code:

unsigned long a=0, b=0, difference;  
void setup()   
{    
  Serial.begin(9600);  
  pinMode(2, INPUT);  
  Serial.print("start");  
  Serial.print(",");  
  Serial.print("stop");  
  Serial.print(",");  
  Serial.print("minutes");  
  Serial.print(",");  
  Serial.println("seconds");  
}

void loop()   
{  
 //delay(1000);  
 int A = digitalRead(2);  
 if(A == LOW)  
 {  
  b = millis();  
 }  
 if(A == HIGH)  
  {  
     a = millis();  
     display_output();  
   }  
 delay(3000);  
}  
void display_output()  
{  
 int minutes, seconds, sec;  
 difference = a - b;  
 seconds = int(difference/1000);  
 if(seconds >= 60)  
 {  
  //sec = seconds;  
  seconds = seconds%60;  
  minutes = minutes+1;  
 }  
 //minutes = int(seconds/60);  
 Serial.print((a/1000)+1);  
 Serial.print(",");  
 Serial.print((b/1000)+1);  
 Serial.print(",");  
// Serial.print(difference);  
 //Serial.print(",");  
 Serial.print(minutes);  
 Serial.print(",");  
 Serial.println(seconds);  
 //Serial.println(",");  
}  
  • 1
    The question is not very clear. Are you trying to measure the time between two events in time? What do you mean by "scale up to multiple systems"? Interrupts are another way to measure the change of logic state in a signal – MichaelT Dec 13 '18 at 11:00
  • Yeah, I'm measuring the time difference between the time when a signal comes to the Arduino from my machine and when the signal stops. I have multiple machines working simultaneously at different stages and I have to do a project on how often and how long the machines are down. I programmed for one machine and it was working normally, but when i try to program for two or more machines ( note that two machines may go down with some error at same time), i get some random value. – Rajesh Anand10 Dec 13 '18 at 15:36
2

First of all, you should notice there is a problem with your logic:

 int A = digitalRead(2);
 if(A == LOW)  
 {  
  b = millis();  
 }

Here you are testing the state of the pin. If it's LOW you take a record of the time. This is wrong. What you want to record is the time at which the inputs falls, not when it's LOW. This is called state change detection. In order to implement it, you have to compare the pin state that you just read with the previous state. The correct condition is “if the pin was HIGH last time I read it and is now LOW”:

// Global variable:
int pin_state = HIGH;

// Within loop():
int new_pin_state = digitalRead(2);
if (pin_state == HIGH && new_pin_state == LOW) {  // it just fell
    time_of_fall = millis();
    pin_state = new_pin_state;
}

Note that this assumes that the default state is HIGH. If the pin turns out to be LOW at power-up, then the code will assume it just fell when the program started.

Next, you have to remove any occurrence of delay() from your code. That will only make your timings wrong. At this point you have non-blocking code that can easily be adapted to handle multiple input channels.

Then you have to identify the variables that are channel-specific, and have a per-channel version of each variable (arrays are useful here). Then your code only needs to loop through the arrays.

Instead of having multiples arrays, you can make the code cleaner by grouping all the channel-specific data into a single data structure:

struct Channel {
    byte pin;
    byte state;
    unsigned long fell, rose;  // times of fall and rise
};

This way you can have a single array that holds all the data for all the channels:

const int CHANNEL_COUNT = 5;

Channel channels[CHANNEL_COUNT] = {
    { 2, HIGH, 0, 0 },
    { 3, HIGH, 0, 0 },
    { 4, HIGH, 0, 0 },
    { 5, HIGH, 0, 0 },
    { 6, HIGH, 0, 0 }
};

The code now becomes quite simple: you just loop through the array and handle one channel at a time:

void setup()
{
    for (int i = 0; i < CHANNEL_COUNT; ++i) {
        pinMode(channels[i].pin, INPUT);
    }
    Serial.begin(9600);
    Serial.println("pin,fell,rose,low time");
}

void loop()
{
    for (int i = 0; i < CHANNEL_COUNT; ++i) {
        Channel &c = channels[i];
        byte new_state = digitalRead(c.pin);
        if (c.state == HIGH && new_state == LOW) {  // input fell
            c.fell = millis();
        }
        else if (c.state == LOW && new_state == HIGH) {  // input rose
            c.rose = millis();
            display_channel(c);
        }
        c.state = new_state;
    }
}

The function display_channel() is left as an exercise to the reader ;-) (hint: it's prototype should be void display_channel(const Channel &c); in order to avoid unnecessarily copying the data structure.

Next step in code modularity would be to turn the Channel structure into a class with a proper constructor, and make display_channel() a method of that class.

  • What about a link to the state-change-detection example? arduino.cc/en/Tutorial/StateChangeDetection – Jot Dec 13 '18 at 13:00
  • This code looks neat and will definitely try with this one. I don't know much about constructors and structure things. Will learn about that. I used a delay in my program so as not to have too many data points and instead the data once in two or three seconds will be enough plus its not a time critical application, so I don't know if using millis function is overkill for my application. Thank you though :) – Rajesh Anand10 Dec 13 '18 at 15:28
  • @Jot: Good idea. I added the link. – Edgar Bonet Dec 13 '18 at 16:18
  • 2
    @RajeshAnand10: 1. Learning about object-oriented programming (classes with constructors and methods) is useful, but you can live without it for simple projects like this. 2. delay() was useful in your previous version only because you got the logic wrong. Once you properly detect state changes, it serves no useful purpose. 3. What do you mean by “overkill”? Do you believe millis() is more heavyweight than delay()? It's not. For your particular problem, it's the right tool for the job. – Edgar Bonet Dec 13 '18 at 16:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.