Need Help w/ End of My Code

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
I've been playing around with my Arduino kit and finally pieced together the code to create a delayed "on" after 5 seconds when the switch I have is held (it's the cheap one that comes with the kit). It works perfectly for the first press (and hold) of the switch, holds a green led high and a red led low for 5 seconds, then flips them. However, I have to hit the Uno's reset button to get the sequence to run again, or else, when I hold the switch down subsequent times the red led that is meant to delay immediately comes on.

I'm using millis instead of the delay function (or command, whatever you call it) and I'm needing to know how to tidy up the end of the code so that once the switch is released, I can press it again and start the original, delayed sequence again. I've seen the examples for the "if" function where "currentTime - previousTime...blah blah blah," but I don't need the code to create a blink or oscillation. I would like the circuit to eventually turn off at a predetermined time interval (20-30 minutes) if it is not interrupted by another switch press. I'm using millis because I do want to add other operations that need to run no matter what, so the delay function won't help.

Any help would be greatly appreciated, I think I am just missing something simple to add on the end but my Google-fu didn't turn up anything.
 

djsfantasi

Joined Apr 11, 2010
9,131
I've been playing around with my Arduino kit and finally pieced together the code to create a delayed "on" after 5 seconds when the switch I have is held (it's the cheap one that comes with the kit). It works perfectly for the first press (and hold) of the switch, holds a green led high and a red led low for 5 seconds, then flips them. However, I have to hit the Uno's reset button to get the sequence to run again, or else, when I hold the switch down subsequent times the red led that is meant to delay immediately comes on.

I'm using millis instead of the delay function (or command, whatever you call it) and I'm needing to know how to tidy up the end of the code so that once the switch is released, I can press it again and start the original, delayed sequence again. I've seen the examples for the "if" function where "currentTime - previousTime...blah blah blah," but I don't need the code to create a blink or oscillation. I would like the circuit to eventually turn off at a predetermined time interval (20-30 minutes) if it is not interrupted by another switch press. I'm using millis because I do want to add other operations that need to run no matter what, so the delay function won't help.

Any help would be greatly appreciated, I think I am just missing something simple to add on the end but my Google-fu didn't turn up anything.
It’s kind of difficult to help with your code since you haven’t posted it.

On the line above your reply, there is an icon that looks like three dots. Click on it, and a menu will appear with several options. Click on the Code option and follow the prompts. You will cut and paste your code into the appropriate dialog box. Most of the other options aren’t critical the first time you use this. Due to the “walk before you run” philosophy.

You do that and I’ll be glad to help :)
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
It’s kind of difficult to help with your code since you haven’t posted it.

On the line above your reply, there is an icon that looks like three dots. Click on it, and a menu will appear with several options. Click on the Code option and follow the prompts. You will cut and paste your code into the appropriate dialog box. Most of the other options aren’t critical the first time you use this. Due to the “walk before you run” philosophy.

You do that and I’ll be glad to help :)
I can see the three dots, but I only have two options (edit thread & create poll). I've copied the code below:


int switchState = 0;
const unsigned long event_1 = 5000;
unsigned long previousTime = 0;
void setup(){
pinMode(2, INPUT);
pinMode(3, OUTPUT);
pinMode(4, OUTPUT);

}
void loop(){
unsigned long currentTime = millis();
switchState = digitalRead(2);
if (switchState == LOW) {
digitalWrite(3, HIGH); // green led
digitalWrite(4, LOW); // red led
}
else {
if (currentTime - previousTime >= event_1){
digitalWrite(3, LOW);
digitalWrite(4, HIGH);
}
}
}

It seems to only be missing the line numbers from the IDE, but everything looks correct. If there's another way to share it better, please let me know.
 

djsfantasi

Joined Apr 11, 2010
9,131
Wrong three dots… You didn’t see these
IMG_5776.jpeg
I’ll be able to help tomorrow. It’s too late for me tonight. Time for bed.

Now that they have the code, perhaps someone can help before I can.
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
Code:
int switchState = 0;
const unsigned long event_1 = 5000;
unsigned long previousTime = 0;
void setup(){
  pinMode(2, INPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

}

void loop(){
  unsigned long currentTime = millis();
  switchState = digitalRead(2);
  if (switchState == LOW) {
    digitalWrite(3, HIGH); // green led
    digitalWrite(4, LOW); // red led
  }
  else {
    if (currentTime - previousTime >= event_1){
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
    }
  }
}
Found it, thanks for the screenshot.
 

strantor

Joined Oct 3, 2010
6,743
I think the problem is that currentTime gets updated each scan but previousTime is only ever set once (to 0, at the start where it is declared). So the result of currentTime - previousTime will only ever be equal to currentTime - 0. In other words currentTime - previousTime will always be equal to the same thing as millis().

PreviousTime needs to be updated continuously whenever the button is not pressed. Then when the button IS pressed, it needs to stop updating while currentTime continues to update, so that the result of currentTime - previousTime starts to grow from the instant the button was pressed, instead of from the instant the arduino was powered on.


Try this:
(I am typing on my phone and have not tested this)
C-like:
int switchState = 0;
const unsigned long event_1 = 5000;
unsigned long previousTime = 0;
unsigned long currentTime = 0; // declaration removed from loop and relocated to here
void setup(){
  pinMode(2, INPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

}

void loop(){
  currentTime = millis(); // no longer a declaration ("unsigned long") since variables need only be declared once and not re-declared on every scan of the loop.
  switchState = digitalRead(2);
  if (switchState == LOW) {
    digitalWrite(3, HIGH); // green led
    digitalWrite(4, LOW); // red led
    previousTime = millis(); // added this
  }
  else {
    if (currentTime - previousTime >= event_1){
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
    }
  }
}
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
I think the problem is that currentTime gets updated each scan but previousTime is only ever set once (to 0, at the start where it is declared). So the result of currentTime - previousTime will only ever be equal to currentTime - 0. In other words currentTime - previousTime will always be equal to the same thing as millis().

PreviousTime needs to be updated continuously whenever the button is not pressed. Then when the button IS pressed, it needs to stop updating while currentTime continues to update, so that the result of currentTime - previousTime starts to grow from the instant the button was pressed, instead of from the instant the arduino was powered on.


Try this:
(I am typing on my phone and have not tested this)
C-like:
int switchState = 0;
const unsigned long event_1 = 5000;
unsigned long previousTime = 0;
unsigned long currentTime = 0; // declaration removed from loop and relocated to here
void setup(){
  pinMode(2, INPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

}

void loop(){
  currentTime = millis(); // no longer a declaration ("unsigned long") since variables need only be declared once and not re-declared on every scan of the loop.
  switchState = digitalRead(2);
  if (switchState == LOW) {
    digitalWrite(3, HIGH); // green led
    digitalWrite(4, LOW); // red led
    previousTime = millis(); // added this
  }
  else {
    if (currentTime - previousTime >= event_1){
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
    }
  }
}
That works perfectly, thank you very much for the help. I'm going to review how you explained that since I don't quite understand what's going on (I'm completely new to all of this).
 

Ya’akov

Joined Jan 27, 2019
8,567
Welcome to AAC.

The general idea has to do with the function of the variable. You are using it to store state information. If you think through what state you want to know about, it will help you use such variables for whatever you might need.

Storing state information is an essential part of many programs. It provides the context the logical operations occur in.
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
I think the problem is that currentTime gets updated each scan but previousTime is only ever set once (to 0, at the start where it is declared). So the result of currentTime - previousTime will only ever be equal to currentTime - 0. In other words currentTime - previousTime will always be equal to the same thing as millis().

PreviousTime needs to be updated continuously whenever the button is not pressed. Then when the button IS pressed, it needs to stop updating while currentTime continues to update, so that the result of currentTime - previousTime starts to grow from the instant the button was pressed, instead of from the instant the arduino was powered on.


Try this:
(I am typing on my phone and have not tested this)
C-like:
int switchState = 0;
const unsigned long event_1 = 5000;
unsigned long previousTime = 0;
unsigned long currentTime = 0; // declaration removed from loop and relocated to here
void setup(){
  pinMode(2, INPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

}

void loop(){
  currentTime = millis(); // no longer a declaration ("unsigned long") since variables need only be declared once and not re-declared on every scan of the loop.
  switchState = digitalRead(2);
  if (switchState == LOW) {
    digitalWrite(3, HIGH); // green led
    digitalWrite(4, LOW); // red led
    previousTime = millis(); // added this
  }
  else {
    if (currentTime - previousTime >= event_1){
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
    }
  }
}
Would it be correct to say that the previousTime = millis(); causes the command to constantly refresh until the switch is closed and the timing portion begins to count? Also, is it the unsigned long portion that facilitates the longer timing duration versus using "int" instead of "const"?
 
Last edited:

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
If I wanted the led to turn itself off after being activated at a longer duration, would I add another "else" function with a second event variable at the top with a different duration or is there a better way?

I just tried it, but was getting an error with the "}" at the end of the code (last line).
 

Ya’akov

Joined Jan 27, 2019
8,567
If I wanted the led to turn itself off after being activated at a longer duration, would I add another "else" function with a second event variable at the top with a different duration or is there a better way?

I just tried it, but was getting an error with the "}" at the end of the code (last line).
Always post the code you are asking about. It is very hard to help by imagining what your code looks like. Often the problem is as simple as a syntax error or bracket mismatch—and since you can't tell us that you did those things you have to show the actual code.
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
Always post the code you are asking about. It is very hard to help by imagining what your code looks like. Often the problem is as simple as a syntax error or bracket mismatch—and since you can't tell us that you did those things you have to show the actual code.
Ooops, sorry.
Code:
int switchState = 0;
const unsigned long event_1 = 3000;
const unsigned long event_2 = 7000;
unsigned long previousTime = 0;
unsigned long currentTime = 0;
void setup(){
  pinMode(2, INPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
 
}

void loop(){
  currentTime = millis();
  switchState = digitalRead(2);
  if (switchState == LOW) {
    digitalWrite(3, HIGH); // green led
    digitalWrite(4, LOW); // red led
    previousTime = millis();
  }
  else {
    if (currentTime - previousTime >= event_1){
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
    }
  else {
    if (currentTime - previousTime >= event_2){
      digitalWrite(3, HIGH);
      digitalWrite(4, LOW);
    }
  }
}
This is the error I receive when I attempt to upload it: " Compilation error: expected primary-expression before ')' token "
 

MrChips

Joined Oct 2, 2009
29,874
Your problem is likely caused by unpaired braces { }.
There are a number of things that I do to avoid having problems.

1 ) I place braces on separate lines and indent code meticulously.
2) I type braces before filling in the code between braces.
3) I enter comments on opening and closing braces so that I know which are paired.
For example:
C:
void loop()
{ // start of loop
   if (running)
      { // running
      } // end of running
   else
      { // not running
      } // end of not running
} // end of loop
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
Your problem is likely caused by unpaired braces { }.
There are a number of things that I do to avoid having problems.

1 ) I place braces on separate lines and indent code meticulously.
2) I type braces before filling in the code between braces.
3) I enter comments on opening and closing braces so that I know which are paired.
For example:
C:
void loop()
{ // start of loop
   if (running)
      { // running
      } // end of running
   else
      { // not running
      } // end of not running
} // end of loop
Whatever version of the IDE I'm using automatically adds most of them unless I do it manually. When I do, I've intentionally moved the brace to the next line. I can't tell from looking at the code where one is missing. With every primary function, the IDE shows a line dropping down the margin to the closed/final brace with each command. All of them are matched so I'm not sure whether one is missing or one needs to be removed.
 

Ya’akov

Joined Jan 27, 2019
8,567
Whatever version of the IDE I'm using automatically adds most of them unless I do it manually. When I do, I've intentionally moved the brace to the next line. I can't tell from looking at the code where one is missing.
The editor also does brace matching. If you place the cursor on one brace it will highlight the opening or closing brace that matches it.
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
The editor also does brace matching. If you place the cursor on one brace it will highlight the opening or closing brace that matches it.
Interesting. My braces on line 13 and 21 do not match anything. The one on line 26 matches the final one on line 31, which should probably match the one tied to the void loop () on 13. How do I get them to match? I've tried indenting and spacing them different ways, but the error always pops citing the final one on line 31. Do I need to retype it completely?
 

Ya’akov

Joined Jan 27, 2019
8,567
If you count brackets there has to be an even number. This is one way of checking for the problem if you haven’t really messed it up. I usually start by counting open braces, then subtracting one when I encounter a close. If I have a remainder I know that something was added or missed.
 

Thread Starter

Alln3w2m3

Joined Jun 20, 2023
57
The editor also does brace matching. If you place the cursor on one brace it will highlight the opening or closing brace that matches it.
Disregard my last, I believe I fixed the bracing. Sheesh, that was a subtle issue. I do have a new error that I will post in a separate reply.
 

Ya’akov

Joined Jan 27, 2019
8,567
Disregard my last, I believe I fixed the bracing. Sheesh, that was a subtle issue. I do have a new error that I will post in a separate reply.
This is why @MrChips advice about formatting is so important. If you rigorously indent for each open brace you will much more easily find the problem when they aren’t matched. It’s also easier to read. As an aside I make my tabs 2 spaces because more can quickly cause the code to overrun the right margin and make it much harder to read.
.
 
Top