Hello - I've developed the below code to do a few things depending on the positions of a push-button and a rocker switch. The code worked perfectly but am experimenting with some more advanced code - for example changed out the if else statements for switch case statements, and the for (int x = 0; x < 5; x++) { approach. I took out the delay functions that determine the LED on/off intervals, wanting to use the Millis command instead, and also took the opportunity to make it a subroutine at the end of the sketch Returns the error "previousMillis' was not declared in this scope" when attempting to compile. Any thoughts on why?
int ch1Pin = 2;// define pin 2 for chan 1 of SSR RED
int ch2Pin = 3;// define pin 3 for chan 2 of SSR YEL
int ch3Pin = 4;// define pin 4 for chan 2 of SSR FAN
const int buttonPin = 8; // the number of the pushbutton pin
int buttonState = 0; // variable for reading the pushbutton status
int triggerType = LOW;// type LOW if low trigger and HIGH if high trigger SSR. SIMON LOW level trigger means zero volts activates relay NOTE ITS WRITTEN ON THE RELAY
int ssrON, ssrOFF;// used for two different SSR trigger type. Do not change
// Definition of the rocker switch pins
int rocker_open_pin = 11; // Green switch off pin connected to digital pin 11
int rocker_closed_pin = 10; // Orange switch on pin connected to digital pin 10
int rocker_open_state = 0; // variable for rocker switch open state
int rocker_closed_state = 0; // variable for rocker switch close state
void setup() {
pinMode(ch1Pin, OUTPUT);//define ch1Pin as output
pinMode(ch2Pin, OUTPUT);//define ch2Pin as output
pinMode(ch3Pin, OUTPUT);//define ch3Pin as output
// initialize the pushbutton pin as an input:
pinMode(buttonPin, INPUT);
// rocker switch definition
pinMode(rocker_open_pin, INPUT_PULLUP); // declare open switch as input + activate internal pull-up resistor
pinMode(rocker_closed_pin, INPUT_PULLUP); // declare closed switch as input + activate internal pull-up resistor
// Code to set On or Off depending on the relay type, whether trigger type is set above at high or low
if(triggerType == HIGH)
{
ssrON = HIGH;
ssrOFF = LOW;
}else{
ssrON = LOW;
ssrOFF = HIGH;
}
digitalWrite(ch1Pin,ssrOFF);// set initial state of SSR 1: OFF
digitalWrite(ch2Pin,ssrOFF);// set initial state of SSR 2: OFF
digitalWrite(ch3Pin,ssrOFF);// set initial state of SSR 2: OFF
// Code to set up LED flash interval
unsigned long interval = 50; // the time I want LED flashing on/off
unsigned long previousMillis = 0; // millis() returns an unsigned long
}
void loop() {
// read the state of the pushbutton value:
buttonState = digitalRead(buttonPin);
switch (buttonState) {
case HIGH:
// turn LED on:
digitalWrite(ch3Pin, LOW);
break;
default:
// turn LED off:
digitalWrite(ch3Pin, HIGH);
}
// read the state of the rocker open value:
rocker_open_state = digitalRead(rocker_open_pin);
switch (rocker_open_state) {
case LOW:
// Yellow LED
for (int x = 0; x < 5; x++) {
digitalWrite(ch1Pin,ssrON);// turn relay SSR1 ON
led_flash_delay(); //calls the LED flash subroutine
digitalWrite(ch1Pin,ssrOFF);// turn relay SSR2 OFF
led_flash_delay(); //calls the LED flash subroutine
}
break;
}
// read the state of the rocker close value:
rocker_closed_state = digitalRead(rocker_closed_pin);
switch (rocker_closed_state) {
case LOW:
// RED LED
for (int x = 0; x < 5; x++) {
digitalWrite(ch2Pin,ssrON);// turn relay SSR2 ON
led_flash_delay(); //calls the LED flash subroutine
digitalWrite(ch2Pin,ssrOFF);// turn relay SSR1 OFF
led_flash_delay(); //calls the LED flash subroutine
}
break;
}
} // close void loop
void led_flash_delay() { // declares the subroutine for led flash delay
unsigned long currentMillis = millis(); // grab current time
// check if "interval" time has passed (50 milliseconds)
if (currentMillis - previousMillis >= interval) {
// do something which in this case is just wait, chew up the clock
previousMillis = millis(); // save the "current" time
}
} // close subroutine
// Code to set up LED flash interval
unsigned long interval = 50; // the time I want LED flashing on/off
unsigned long previousMillis = 0; // millis() returns an unsigned long
}
Do you see that } ?
That marks the end of the setup() function. So those 2 variables are inside of and therefore local to setup() and cease to exist when setup() ends, which is immediately after they are declared.
I think you must have intended them to be global variables, in which case declare them outside of any function, ideally at the top of your code with the other global variables.
If you click Tools->Auto Format this will fix your poor indentation. Correct indentation makes zero difference to how your code runs, but it does help us humans spot errors. If you try it on your original code, it will become clear that those 2 variables are inside a function, because they will get indented, compared to the global variables at the top of the code.
Hmm. Yes you are correct - I copied the millis commands from some examples I found. Thanks for this. Now compiler is saying 'currentMillis can't be used as a function'. It's declared in the subroutine, I'm guessing it needs to be declared with the others in the global area
Oh and thanks for the Tools > autoformat, thats great! didn't know about it
Can't say why that is without seeing the updated code.
In C/C++ subroutines are called functions. In other languages like Visual Basic, there are functions which return values and subroutines, which don't return a value. In C/C++, if a function does not return a value, it is declared as "void".
int ch1Pin = 2;// define pin 2 for chan 1 of SSR RED
int ch2Pin = 3;// define pin 3 for chan 2 of SSR YEL
int ch3Pin = 4;// define pin 4 for chan 2 of SSR FAN
const int buttonPin = 8; // the number of the pushbutton pin
int buttonState = 0; // variable for reading the pushbutton status
int triggerType = LOW;// type LOW if low trigger and HIGH if high trigger SSR. SIMON LOW level trigger means zero volts activates relay NOTE ITS WRITTEN ON THE RELAY
int ssrON, ssrOFF;// used for two different SSR trigger type. Do not change
// Definition of the rocker switch pins
int rocker_open_pin = 11; // Green switch off pin connected to digital pin 11
int rocker_closed_pin = 10; // Orange switch on pin connected to digital pin 10
int rocker_open_state = 0; // variable for rocker switch open state
int rocker_closed_state = 0; // variable for rocker switch close state
// Code to set up LED flash interval
unsigned long interval = 50; // the time I want LED flashing on/off
unsigned long previousMillis = 0; // millis() returns an unsigned long
void setup() {
pinMode(ch1Pin, OUTPUT);//define ch1Pin as output
pinMode(ch2Pin, OUTPUT);//define ch2Pin as output
pinMode(ch3Pin, OUTPUT);//define ch3Pin as output
// initialize the pushbutton pin as an input:
pinMode(buttonPin, INPUT);
// rocker switch definition
pinMode(rocker_open_pin, INPUT_PULLUP); // declare open switch as input + activate internal pull-up resistor
pinMode(rocker_closed_pin, INPUT_PULLUP); // declare closed switch as input + activate internal pull-up resistor
// Code to set On or Off depending on the relay type, whether trigger type is set above at high or low
if (triggerType == HIGH)
{
ssrON = HIGH;
ssrOFF = LOW;
} else {
ssrON = LOW;
ssrOFF = HIGH;
}
digitalWrite(ch1Pin, ssrOFF); // set initial state of SSR 1: OFF
digitalWrite(ch2Pin, ssrOFF); // set initial state of SSR 2: OFF
digitalWrite(ch3Pin, ssrOFF); // set initial state of SSR 2: OFF
}
void loop() {
// read the state of the pushbutton value:
buttonState = digitalRead(buttonPin);
switch (buttonState) {
case HIGH:
// turn LED on:
digitalWrite(ch3Pin, LOW);
break;
default:
// turn LED off:
digitalWrite(ch3Pin, HIGH);
}
// read the state of the rocker open value:
rocker_open_state = digitalRead(rocker_open_pin);
switch (rocker_open_state) {
case LOW:
// Yellow LED
for (int x = 0; x < 5; x++) {
digitalWrite(ch1Pin, ssrON); // turn relay SSR1 ON
led_flash_delay(); //calls the LED flash subroutine
digitalWrite(ch1Pin, ssrOFF); // turn relay SSR2 OFF
led_flash_delay(); //calls the LED flash subroutine
}
break;
}
// read the state of the rocker close value:
rocker_closed_state = digitalRead(rocker_closed_pin);
switch (rocker_closed_state) {
case LOW:
// RED LED
for (int x = 0; x < 5; x++) {
digitalWrite(ch2Pin, ssrON); // turn relay SSR2 ON
led_flash_delay(); //calls the LED flash subroutine
digitalWrite(ch2Pin, ssrOFF); // turn relay SSR1 OFF
led_flash_delay(); //calls the LED flash subroutine
}
break;
}
} // close void loop
void led_flash_delay() { // declares the subroutine for led flash delay
unsigned long currentMillis = millis(); // grab current time
// check if "interval" time has passed (50 milliseconds)
if (currentMillis - previousMillis >= interval) {
// do something which in this case is just wait, chew up the clock
previousMillis = currentMillis(); // save the "current" time
}
} // close subroutine
Is this a 3-position switch (up, down and centre off) ? If not, if it's only 2 position, then your code to deal with it seems way over-complicated. How is it wired?
I'm certainly no expert programmer and have cobbled together the code. It's a three position SPDT centre off rocker spring return switch. Code might be overly complicated especially since its only got one case alternative, but I'm trialling it - once it's correct the rocker will open/close a linear actuator but there will be other aspects to the case. But for now it's just being trialed on activating LEDs or not
Please post a schematic so we can see how the switches/buttons and LEDs are wired. Hand-drawn is ok.
Also please describe the intended operation of the circuit.
Then we can take another look at your code and advise what needs to change. I should warn you, probably most of it will need to change! There were many parts that made me frown and scratch my head when I read them!
Not more "advanced", and every switch statement in your code at the moment would more appropriately be an if statement, because in every case there are only 2 options.
You have not understood the principles of using millis() yet, and as a result, your attempt to replace delay() has only broken the code.
I have known it this way --
"when you put () (pair of parentheses) after a non-keyword name/identifier followed by ; (semicolon -- the termination), it is a function."
Hi PaulRB - the code may be questionable yet it worked exactly as I wanted right up until I replaced the delay (I was using that to make the LEDS flash 4 times) with the Millis function. Still I'd be interested to hear a better programmer's thoughts on how abominable my coding is! And know where/how it could be improved. But like I said it did what it was required to do. Essentially two switches, 2 LEDs, 3 relays and a fan. Push button turns fan on/off via relay. Rocker made each led flash 4 times via relay depending on which of the two SPDT positions the rocker was pushed to.
Once I get these right, the LEDs will be replaced by a linear actuator, the fan switch by a 12v signal from my amplifier so that when the amplifier is on, the fan is on
Essentially using the following after a LED on and a LED off command made it blink
delay(50);// wait for 0.05 seconds
I was attempting to replace the above with
led_flash_delay(); //calls the LED flash subroutine
And use the following subroutine
void led_flash_delay() { // declares the subroutine for led flash delay
unsigned long currentMillis = millis(); // grab current time
// check if "interval" time has passed (50 milliseconds)
if (currentMillis - previousMillis >= interval) {
// do something which in this case is just wait, chew up the clock
previousMillis = currentMillis; // save the "current" time
}
} // close subroutine
All seems to work ok except the millis isn't working so I will review that, essentially I want it to just wait 0.05 seconds instead of the delay, and using the millis means the arduino can accept the fan button input simultaneously as opposed to the delay which locks it up
Because of the way your led_flash_delay() function is written, it does not replicate delay(). In fact, it doesn't really do anything useful. But even if it did work, why "re-invent the wheel"? Why not simply use delay()?
If you have heard that using delay() is bad, well it often is, but not always. It depends what else you need your code to do, especially if you need it do something while delay() is running, because, well ... that ain't gonna happen!
But replacing delay() with your own function that replicates what delay() does would be pointless, your code still won't be able to do anything while your delay-like-function is running.
If you need your code to continue performing other tasks, while timing some tasks like flashing a led, you have to write your entire sketch in a different way, a way that doesn't need to use delay() or anything like it.
