Pages: [1]   Go Down
Author Topic: Uncertain where to post this, is this coding correct?  (Read 1208 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Code:
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 Offline
Edison Member
*
Karma: 9
Posts: 1991
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

"Programming Questions ?"

I'm a big fan of 'if it does what you want, its right.'  smiley
Logged


Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

 smiley-razz
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  smiley-twist
Logged

Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well It's not working now smiley-sad
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.
Code:
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
Online Online
Brattain Member
*****
Karma: 538
Posts: 27097
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
analogWrite(colorledstate1, HIGH);
colorledstate1 is not a pin,
try writing to one of your defined pins
Code:
const int colorchange1 = 11; //red dual color pin
const int colorchange2 = 12; //green dual color pin
Logged

Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

You're right, I missed that. Either way, the led still goes high during the setup.
Modified code, still not looping though
Code:
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 Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

I only provide help via the forum - please do not contact me for private consultancy.

Global Moderator
Boston area, metrowest
Online Online
Brattain Member
*****
Karma: 538
Posts: 27097
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Code:
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
Online Online
Brattain Member
*****
Karma: 538
Posts: 27097
Author of "Arduino for Teens". Available for Design & Build services. Now with Unlimited Eagle board sizes!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Well, that would indicate this isn't doing what you want:
Code:
  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

Designing & building electrical circuits for over 25 years. Check out the ATMega1284P based Bobuino and other '328P & '1284P creations & offerings at  www.crossroadsfencing.com/BobuinoRev17.
Arduino for Teens available at Amazon.com.

Offline Offline
Jr. Member
**
Karma: 1
Posts: 67
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Pages: [1]   Go Up
Jump to: