If you want to know if a pin is high or low you have to READ the pin. It's no good just comparing the NUMBER of the pin with HIGH. You'll find digitalRead() is useful for this purpose.
Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?
How have you got your switch wired?
Your code looks as though you have a button from the Arduino input to 5V, do you have a 10k pull down resistor between the Arduino input and gnd to make sure the input goes to gnd when the button is not pressed?
OP,
I'll go out on a limb, and you may hate me for this - but you have missed the basic language tutorials entirely.
Go back to the Forum Home Page, and study HOW to use the Arduino language (C/C++) - and it's specifics to the Arduino platform.
To read a pin's state - as noted above, you use digitalRead()
To set a pin's state - as noted above, you use digitalWrite()
To loop - ignore the loop() function - that't the main 'container' function of Arduino-land.
It does loop - but outside the scope of anything you are likely to be doing at this stage.
Looping withinyour functions may achieved be with for(), do-while() or other constructs.
While you're learning, see if you can abandon delay() once you have the basics working.
Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?
How have you got your switch wired?
Your code looks as though you have a button from the Arduino input to 5V, do you have a 10k pull down resistor between the Arduino input and gnd to make sure the input goes to gnd when the button is not pressed?
What model Arduino are you using?
Thanks.. Tom...
I use an Arduino Nano
And btw none of the lights go on now.
lastchancename:
OP,
I'll go out on a limb, and you may hate me for this - but you have missed the basic language tutorials entirely.
Go back to the Forum Home Page, and study HOW to use the Arduino language (C/C++) - and it's specifics to the Arduino platform.
To read a pin's state - as noted above, you use digitalRead()
To set a pin's state - as noted above, you use digitalWrite()
To loop - ignore the loop() function - that't the main 'container' function of Arduino-land.
It does loop - but outside the scope of anything you are likely to be doing at this stage.
Looping withinyour functions may achieved be with for(), do-while() or other constructs.
While you're learning, see if you can abandon delay() once you have the basics working.
Is this a better code then not using the loop within functions?
if ((LED1 == HIGH) && (currentMillis - previousMillis >= OnTime))
2 lines previously you read the state of LED1 (or at least try to) and throw away what is returned. Then in the line above you test the value of the LED1 pin not the state of the pin.
Have you tried compiling the program ?
Several errors are reported
if ((LED1 == HIGH) && (currentMillis - previousMillis >= OnTime))
2 lines previously you read the state of LED1 (or at least try to) and throw away what is returned. Then in the line above you test the value of the LED1 pin not the state of the pin.
Have you tried compiling the program ?
Several errors are reported
That’s a huge step forward from your first code!
Congratulations in paying attention to the tutorials.
The flow of your code will become more obvious if you press Ctrl+T in the IDE, which will realign and balance up your indenting.
Also, the extra empty/blank lines aren’t necessary unless they emphasise or assist with the readability of the code blocks.
I reckon you’re only a few hours from a working sketch - and you’ve included millis() as well! That will make future functionality much easier to add.
One hint, put your switch ‘upside down’ between the pin and ground, then you can eliminate the external resistor, and use INPUT_PULLUP. Less parts & wiring, but you need to ‘invert’your sensing of the input states.
Hi,
I think this will work, without all the nesting in the if statements.
const int GreenLEDPin = 2;
const int RedLEDPin = 4;
const int KNOPPin = 7;
unsigned long currentMillis;
unsigned long previousMillis ;
long interval;
long OnTime = 500;
long OffTime = 750;
bool GreenledState = LOW;
bool RedledState = LOW;
bool Status ;
void setup ()
{
pinMode (GreenLEDPin, OUTPUT);
pinMode (RedLEDPin, OUTPUT);
pinMode (KNOPPin, INPUT);
}
void loop()
{
currentMillis = millis();
Status = digitalRead(KNOPPin);
if (Status == HIGH)
{
digitalWrite(RedLEDPin, LOW);
BlinkGreen();
}
else
{
digitalWrite(GreenLEDPin, LOW);
BlinkRed();
}
}
void BlinkGreen()
{
if (GreenledState == HIGH) // Check LED state to load relevant delay time
{
interval = OnTime;
}
else
{
interval = OffTime;
}
if (currentMillis - previousMillis >= interval)
{
// save the last time you blinked the LED
previousMillis = currentMillis;
// if the LED is off turn it on and vice-versa:
if (GreenledState == LOW)
{
GreenledState = HIGH;
}
else
{
GreenledState = LOW;
}
// set the LED with the ledState of the variable:
digitalWrite(GreenLEDPin, GreenledState);
}
}
void BlinkRed()
{
if (RedledState == HIGH) // Check LED state to load relevant delay time
{
interval = OnTime;
}
else
{
interval = OffTime;
}
if (currentMillis - previousMillis >= interval) {
// save the last time you blinked the LED
previousMillis = currentMillis;
// if the LED is off turn it on and vice-versa:
if (RedledState == LOW)
{
RedledState = HIGH;
}
else
{
RedledState = LOW;
}
// set the LED with the ledState of the variable:
digitalWrite(RedLEDPin, RedledState);
}
}
I have like you, used blink without delay, but I used the ledstate value to determine the delay interval.
I have also renamed your pin and variables to describe what they represent.
