Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« on: October 04, 2012, 04:29:46 pm » |
This is my first writing of arduino code and I'd like to know how well I've done. The program is used to do two things: 1. fade one LED up and down in brightness. 2. Change the color of a multi color led from green to red. This is to be loaded onto an arduino nano and more will likely be added. Please tell me whether it is done correct or any tendencies that could compromise my coding at a more advanced level. const int fadingled = 9; //fading led pin const int brightness = 0; //brightness of the led const int fadeamount = 5 //how much the led fades const int colorchange1 = 11; //red dual color pin const int colorchange2 = 12; //green dual color pin int colorledstate1 = LOW; //red led state on/off int colorledstate2 = LOW; //green led state on/off long previousmillis = 0; //last time LED was updated long interval = 500; //time between flashes
void setup() { pinMode(fadingled,OUTPUT); //declare all LED's as outputs pinMode(colorchange1,OUTPUT); pinMode(colorchange2,OUTPUT); }
void loop() { analogWrite(led, brightness); //set the brightness to the LED brightness = brightness + fadeamount; //add or subtract to the brightness based on fadeamount if (brightness == 0 || brightness == 255) { fadeamount = -fadeamount; } unsigned long currentmillis = millis(); if currentmillis - previousmillis > interval) { //if it has been more than 500 milliseconds previousmillis = currentmillis; //turn the led off if it's on and vice versa if (colorledstate1 == LOW) { colorledstate1 = HIGH; } else { colorledstate1 = LOW; } //if the red LED is on, turn the green LED off and vice versa if (colorledstate1 == LOW) { colorledstate2 = HIGH; } else { colorledstate2 = LOW; }
|
|
|
|
|
Logged
|
|
|
|
|
Lancashire, UK
Offline
Edison Member
Karma: 8
Posts: 1988
|
 |
« Reply #1 on: October 04, 2012, 04:36:51 pm » |
"Programming Questions ?" I'm a big fan of 'if it does what you want, its right.' 
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« Reply #2 on: October 04, 2012, 04:39:34 pm » |
 Arduino hasn't showed up yet. I want to get my code done and ready to go before it gets here. Of course I bought two 
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« Reply #3 on: October 05, 2012, 02:40:53 pm » |
Well It's not working now  I put one LED to go high in the setup just so I know that it is running and it is, but it's never going through the loop. const int fadingled = 9; //fading led pin int brightness = 0; //brightness of the led int fadeamount = 5; //how much the led fades const int colorchange1 = 11; //red dual color pin const int colorchange2 = 12; //green dual color pin int colorledstate1 = LOW; //red led state on/off int colorledstate2 = LOW; //green led state on/off long previousmillis = 0; //last time LED was updated long interval = 500; //time between flashes
void setup() { pinMode(fadingled,OUTPUT); //declare all LED's as outputs pinMode(colorchange1,OUTPUT); pinMode(colorchange2,OUTPUT); analogWrite(colorledstate1, HIGH); }
void loop() { analogWrite(fadingled, brightness); //set the brightness to the LED brightness = brightness + fadeamount; //add or subtract to the brightness based on fadeamount if (brightness == 0 || brightness == 255) { fadeamount = -fadeamount; } unsigned long currentmillis = millis(); if (currentmillis - previousmillis > interval) { //if it has been more than 500 milliseconds previousmillis = currentmillis; //turn the led off if it's on and vice versa if (colorledstate1 == LOW) { colorledstate1 = HIGH; } else { colorledstate1 = LOW; } //if the red LED is on, turn the green LED off and vice versa if (colorledstate1 == LOW) { colorledstate2 = HIGH; } else { colorledstate2 = LOW; } } }
|
|
|
|
|
Logged
|
|
|
|
|
Global Moderator
Boston area, metrowest
Offline
Brattain Member
Karma: 249
Posts: 16572
Available for Design & Build services
|
 |
« Reply #4 on: October 05, 2012, 02:52:50 pm » |
analogWrite(colorledstate1, HIGH);
colorledstate1 is not a pin, try writing to one of your defined pins const int colorchange1 = 11; //red dual color pin const int colorchange2 = 12; //green dual color pin
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« Reply #5 on: October 05, 2012, 03:11:53 pm » |
You're right, I missed that. Either way, the led still goes high during the setup. Modified code, still not looping though const int fadingled = 9; //fading led pin int brightness = 0; //brightness of the led int fadeamount = 1; //how much the led fades const int colorchange1 = 11; //red dual color pin const int colorchange2 = 12; //green dual color pin int colorledstate1 = LOW; //red led state on/off int colorledstate2 = LOW; //green led state on/off long previousmillis = 0; //last time LED was updated long interval = 500; //time between flashes
void setup() { pinMode(fadingled,OUTPUT); //declare all LED's as outputs pinMode(colorchange1,OUTPUT); pinMode(colorchange2,OUTPUT); analogWrite(colorchange1, HIGH); }
void loop() { analogWrite(colorchange1,colorledstate1); //write to the LED's analogWrite(colorchange2,colorledstate2); analogWrite(fadingled, brightness); //set the brightness to the LED brightness = brightness + fadeamount; //add or subtract to the brightness based on fadeamount if (brightness == 0 || brightness == 255) { fadeamount = -fadeamount; } unsigned long currentmillis = millis(); if (currentmillis - previousmillis > interval) { //if it has been more than 500 milliseconds previousmillis = currentmillis; //turn the led off if it's on and vice versa if (colorledstate1 == LOW) { colorledstate1 = HIGH; } else { colorledstate1 = LOW; } //if the red LED is on, turn the green LED off and vice versa if (colorledstate1 == LOW) { colorledstate2 = HIGH; } else { colorledstate2 = LOW; } } }
|
|
|
|
|
Logged
|
|
|
|
|
UK
Offline
Tesla Member
Karma: 89
Posts: 6402
-
|
 |
« Reply #6 on: October 05, 2012, 03:46:15 pm » |
Modified code, still not looping though
Well, it is looping in the sense that loop() is being called, but presumably the code inside loop() isn't doing what you intended.
|
|
|
|
|
Logged
|
|
|
|
|
Global Moderator
Boston area, metrowest
Offline
Brattain Member
Karma: 249
Posts: 16572
Available for Design & Build services
|
 |
« Reply #7 on: October 05, 2012, 09:08:54 pm » |
analogWrite(pin, # from 0 to 255); I don't see you writing a number to the pins. Try changing the low, high you have there now to something either is, or represents, a number.
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« Reply #8 on: October 05, 2012, 10:07:08 pm » |
Here's the modified code, the color change is working now but the fading LED just stays solid, it stays at a certain brightness depending on the value of fadeamount. Edit: I forgot to put a delay, it's all working now. const int fadingled = 9; //fading led pin int brightness = 1; //brightness of the led int fadeamount = 1; //how much the led fades const int colorchange1 = 11; //red dual color pin const int colorchange2 = 12; //green dual color pin int colorledstate1 = LOW; //red led state on/off int colorledstate2 = LOW; //green led state on/off long previousmillis = 0; //last time LED was updated long interval = 500; //time between flashes
void setup() { pinMode(fadingled,OUTPUT); //declare all LED's as outputs pinMode(colorchange1,OUTPUT); pinMode(colorchange2,OUTPUT); }
void loop() { digitalWrite(colorchange1,colorledstate1); //write to the LED's digitalWrite(colorchange2,colorledstate2); analogWrite(fadingled, brightness); //set the brightness to the LED brightness = brightness += fadeamount; //add or subtract to the brightness based on fadeamount if (brightness == 0 || brightness == 255) { fadeamount = -fadeamount; } unsigned long currentmillis = millis(); if (currentmillis - previousmillis > interval) { //if it has been more than 500 milliseconds previousmillis = currentmillis; //turn the led off if it's on and vice versa if (colorledstate1 == LOW) { colorledstate1 = HIGH; } else { colorledstate1 = LOW; } //if the red LED is on, turn the green LED off and vice versa if (colorledstate1 == LOW) { colorledstate2 = HIGH; } else { colorledstate2 = LOW; } } }
|
|
|
|
« Last Edit: October 05, 2012, 10:47:09 pm by packocrayons »
|
Logged
|
|
|
|
|
Global Moderator
Boston area, metrowest
Offline
Brattain Member
Karma: 249
Posts: 16572
Available for Design & Build services
|
 |
« Reply #9 on: October 05, 2012, 10:54:20 pm » |
Well, that would indicate this isn't doing what you want: brightness = brightness += fadeamount; //add or subtract to the brightness based on fadeamount if (brightness == 0 || brightness == 255) { fadeamount = -fadeamount; }
Try putting a Serial.print and see what's happening to brightness & fadeamount.
|
|
|
|
|
Logged
|
|
|
|
|
Offline
Jr. Member
Karma: 1
Posts: 67
|
 |
« Reply #10 on: October 06, 2012, 07:37:08 am » |
It's doing what I want now. All I need to do is figure out how to get around using the delay. Thanks everyone for the help.
|
|
|
|
|
Logged
|
|
|
|
|
|