Edit - Arduino Stack Exchange

You are not logged in. Your edit will be placed in a queue until it is peer reviewed.

We welcome edits that make the post easier to understand and more valuable for readers. Because community members review edits, please try to make the post substantially better than how you found it, for example, by fixing grammar or adding additional resources and hyperlinks.

Required fields*

Rev

Required fields*

Regarding running multiple functions using millis

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(","); 
}

Answer*

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][change]. 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.

[change]: https://www.arduino.cc/en/Tutorial/StateChangeDetection

Draft saved

Draft discarded

Edit Summary*

Cancel

What about a link to the state-change-detection example? arduino.cc/en/Tutorial/StateChangeDetection

Jot
– Jot

2018年12月13日 13:00:35 +00:00
Commented Dec 13, 2018 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
– Rajesh Anand10

2018年12月13日 15:28:04 +00:00
Commented Dec 13, 2018 at 15:28
@Jot: Good idea. I added the link.

Edgar Bonet
– Edgar Bonet

2018年12月13日 16:18:52 +00:00
Commented Dec 13, 2018 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
– Edgar Bonet

2018年12月13日 16:29:18 +00:00
Commented Dec 13, 2018 at 16:29

Add a comment |

How to Edit

Correct minor typos or mistakes
Clarify meaning without changing it
Add related resources or links
Always respect the author’s intent
Don’t use edits to reply to the author

How to Format

create code fences with backticks ` or tildes ~
```
like so
```
add language identifier to highlight code
```python
def function(foo):
print(foo)
```
put returns between paragraphs
for linebreak add 2 spaces at end
_italic_ or **bold**
indent code by 4 spaces
backtick escapes `like _so_`
quote by placing > at start of line
to make links (use https whenever possible)

<https://example.com>

[example](https://example.com)

<a href="https://example.com">example</a>

formatting help »
answering help »

How to Tag

A tag is a keyword or label that categorizes your question with other, similar questions. Choose one or more (up to 5) tags that will help answerers to find and interpret your question.

complete the sentence: my question is about...
use tags that describe things or concepts that are essential, not incidental to your question
favor using existing popular tags
read the descriptions that appear below the tag

If your question is primarily about a topic for which you can't find a tag:

combine multiple words into single-words with hyphens (e.g. arduino-uno), up to a maximum of 35 characters
creating new tags is a privilege; if you can't yet create a tag you need, then post this question without it, then ask the community to create it for you

popular tags »