First of all ... thank you. Reading this forum has been game changing for a new arduino user like myself. I thank you all.
So I am learning the millis() thing having recently graduated from delay(). I'm hoping to build a simple irrigation system wherein 2 (with the idea to expand) momentary push buttons activate a relay (solenoid valve) and display and indicator LED while on.
The code I am using is copied below. Each button works independently, but if I press one, wait 3 seconds and press the second, the second time dictates when they both turn off.
unsigned long elapsedTime;
unsigned long onTime;
void setup()
{
onTime = millis();
onTime = elapsedTime;
pinMode(13, OUTPUT); //Initialize pin 12 as status LED
pinMode(2, INPUT); // Our button pin
pinMode(8, OUTPUT);
pinMode(3, INPUT);
}
void loop()
{
if (digitalRead(2) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(13, HIGH); // LED comes On
onTime = millis();
}
if(onTime > 0 && millis() - onTime > 5000)
{
digitalWrite(13, LOW); // LED goes off
onTime = 5000;
}
if (digitalRead(3) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(8, HIGH); // LED comes On
onTime = millis();
}
if(onTime > 0 && millis() - onTime > 5000)
{
digitalWrite(8, LOW); // LED goes off
onTime = 5000;
}
}
Just to share... This is the code that I've been able to make work. I am sure that there are problems and issues here... but just to share and hopefully help others.
unsigned long elapsedTime;
unsigned long onTime1;
unsigned long onTime2;
void setup()
{
Serial.begin(9600);
onTime1 = millis();
onTime2 = millis();
onTime1 = elapsedTime;
onTime2 = elapsedTime;
pinMode(13, OUTPUT); //Initialize pin 12 as status LED
pinMode(2, INPUT); // Our button pin
pinMode(8, OUTPUT);
pinMode(3, INPUT);
}
void loop()
{
if (digitalRead(2) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(13, HIGH); // LED comes On
onTime1 = millis();
}
if(onTime1 > 0 && millis() - onTime1 > 5000) // This is the time for button 2
{
digitalWrite(13, LOW); // LED goes off
onTime1 = 5000;
}
if (digitalRead(3) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(8, HIGH); // LED comes On
onTime2 = millis();
}
if(onTime2 > 0 && millis() - onTime2 > 1000) // This is the time for the delay on button 3
{
digitalWrite(8, LOW); // LED goes off
onTime2 = 5000;
}
}
The millis() function returns a unsigned long variable for the time in milliseconds. It can therefor not be used as a flag as well by testing if it is not zero.
Your text layout of the code can be improved. Can you try to have the indents and spaces nicer ? If the text is a big mess, then the result of your coding might be a big mess as well. The best code is code that shows what is going on with a single glance at it.
You have some issues with your code, please look at this:
unsigned long elapsedTime; //This value is not used, remove it
unsigned long onTime1; //Initialize to zero / 0
unsigned long onTime2; //Initialize to zero / 0
void setup()
{
Serial.begin(9600);
//Why assign onTime1 + onTime2 twice for no reason?
onTime1 = millis(); //If onTime1 is initialized to 0 above, remove this
onTime2 = millis(); //If onTime2 is initialized to 0 above, remove this
onTime1 = elapsedTime; //Remove this, not used
onTime2 = elapsedTime; //Remove this, not used
//Usually you would use INPUT_PULLUP for buttons and test for LOW instead of HIGH
pinMode(13, OUTPUT); //Initialize pin 12 as status LED
pinMode(2, INPUT); // Our button pin
pinMode(8, OUTPUT);
pinMode(3, INPUT);
}
void loop()
{
if (digitalRead(2) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(13, HIGH); // LED comes On
onTime1 = millis();
}
//Add parantheses for readability
if((onTime1 > 0) && (millis() - onTime1 > 5000)) // This is the time for button 2
{
digitalWrite(13, LOW); // LED goes off
onTime1 = 5000; //Should be set to zero/0 to indicate LED-off
}
//Below as above
if (digitalRead(3) == HIGH) // Switch is closed to start LED timer
{
digitalWrite(8, HIGH); // LED comes On
onTime2 = millis();
}
if(onTime2 > 0 && millis() - onTime2 > 1000) // This is the time for the delay on button 3
{
digitalWrite(8, LOW); // LED goes off
onTime2 = 5000;
}
}
PaulMurrayCbr:
Now that you have graduated to millis(), the next thing to garduate to is using classes.
Why? The functionality of the sketch does not require coding of any classes. Arrays could make the code less repetitivive, classes are overkill for such a simple sketch.
Danois90:
Why? The functionality of the sketch does not require coding of any classes. Arrays could make the code less repetitivive, classes are overkill for such a simple sketch.
I beg to differ.
Classes would offer encapsulation of the different "onTime"s etc., and make it much easier to read.
Of course, it's not necessary, but if the OP wants to learn, it's a great opportunity.
Danois90:
Why? The functionality of the sketch does not require coding of any classes. Arrays could make the code less repetitivive, classes are overkill for such a simple sketch.
Well, functionality never requires classes. Arrays are fine, but it means all your variables need array indexes, you have to pass those indexes around. You need a set of button pins, a set of led pins, a set of timer variables, variables for state, a set of solenoid pins, etc. A class is a great way to encapsulate that.
Sure, classes aren't necessary. But for the amount of functionality the OP wants, I don't think it's overkill.
Doing it first with arrays and then discovering how much neater classes are in accomplishing the same job is a great way to proceed with learning the language.
step 1 - use separate arrays of all the variables, with functions taking an array index
step 2 - use an array of struct, with functions taking an array index
step 3 - make a constructor for initialization of pin assignments
step 4 - move functionality into member functions, get rid of the indexes
Danois90:
There is no need to build a bridge in order to pass a creek once
+1
I like the idea of classes when one needs to deal with several instances. The example that comes to mind is a simulation of a bee-farm. I would make a class for bee and for hive but not for farm.
Hi,
Thank you for all your help. I found another example and expanded that to two buttons and it seems to be working pretty well. I still don't fully understand the entire program, but I am enjoying the process of tweaking and seeing the results. I am going to post the code below just for the sake of it, if you feel so inclined I am open to feedback. Thank you.
//CONSTANTS
const int BUTTON_PIN2 = 2; // Button2
const int BUTTON_PIN3 = 3; // Button3
const int relay1 = 7;
const int relay2 = 6;
const int greenLED2 = 8;
const int redLED2 = 9;
const int greenLED3 = 10;
const int redLED3 = 11;
//VARIABLES
//int buttonPushCounter = 0; // counter for the number of button presses
int buttonState1 = 0; // current state of the button
int buttonState2 = 0; // current state of the button
int lastButtonState = 0; // previous state of the button
bool relayOn1 = false;
bool relayOn2 = false;
//MILLIS
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
const unsigned long interval = 3000;
const unsigned long redLedInterval = 1000;
//const unsigned long redLedInterval2 = 5000;
void setup()
{
pinMode(BUTTON_PIN2, INPUT);
digitalWrite(BUTTON_PIN2, HIGH); // pull-up
pinMode(BUTTON_PIN3, INPUT);
digitalWrite(BUTTON_PIN3, HIGH); // pull-up
Serial.begin(9600);
pinMode(greenLED2, OUTPUT);
pinMode(redLED2, OUTPUT);
pinMode(greenLED3, OUTPUT);
pinMode(redLED3, OUTPUT);
//SWITCH HANDLERS
pinMode(relay1, OUTPUT);
digitalWrite(relay1, HIGH);
pinMode(relay2, OUTPUT);
digitalWrite(relay2, HIGH);
}
void loop(){
// read the pushbutton input pin:
buttonState1 = digitalRead(BUTTON_PIN2);
buttonState2 = digitalRead(BUTTON_PIN3);
unsigned long currentMillis1 = millis();
unsigned long currentMillis2 = millis();
// if button is pressed, turn relay on (if it wasn't already on), and reset the timer
if( buttonState1==HIGH ) // no need to check for previous state, in this specific case
{
previousMillis = currentMillis1;
digitalWrite(relay1, LOW);
digitalWrite(greenLED2, HIGH);
digitalWrite(redLED2, LOW);
relayOn1 = true;
}
// if relay is currently on...
if( relayOn1 )
{
// turn red led on, if close to turning off the relay
if (currentMillis1 - previousMillis >= interval-redLedInterval )
digitalWrite(redLED2, (millis()/100)%2);//blink red led; 300ms on; 300ms off
// if enough time has elapsed, turn of the relay
if (currentMillis1 - previousMillis >= interval)
{
// .. turn off relay
digitalWrite(relay1, HIGH);
digitalWrite(greenLED2, LOW);
digitalWrite(redLED2, LOW);
relayOn1 = false;
}
Serial.write("Garden Bed 1 is being Watered");
Serial.println(relayOn1);
}
if( buttonState2==HIGH ) // no need to check for previous state, in this specific case
{
previousMillis2 = currentMillis2;
digitalWrite(relay2, LOW);
digitalWrite(greenLED3, HIGH);
digitalWrite(redLED3, LOW);
relayOn2 = true;
}
// if relay is currently on...
if( relayOn2 )
{
// turn red led on, if close to turning off the relay
if (currentMillis2 - previousMillis2 >= interval-redLedInterval )
digitalWrite(redLED3, (millis()/100)%2);//blink red led; 300ms on; 300ms off
// if enough time has elapsed, turn of the relay
if (currentMillis2 - previousMillis2 >= interval)
{
// .. turn off relay
digitalWrite(relay2, HIGH);
digitalWrite(greenLED3, LOW);
digitalWrite(redLED3, LOW);
relayOn2 = false;
}
Serial.write("Garden Bed 2 is being Watered");
Serial.println(relayOn2);
}
}
Yeah, I've played with different intervals. When this is eventually installed I may have different intervals set for different garden beds, but I set it the same here for ease of testing. Thanks for the feedback.