Go Down

Topic: Uncertain where to post this, is this coding correct? (Read 1 time) previous topic - next topic

packocrayons

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: [Select]
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;
  }

pluggy

"Programming Questions ?"

I'm a big fan of 'if it does what you want, its right.'  :)
http://pluggy.is-a-geek.com/index.html

packocrayons

:P
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  ]:D

packocrayons

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.
Code: [Select]
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;
  }
  }
}

CrossRoads

Code: [Select]

analogWrite(colorledstate1, HIGH);

colorledstate1 is not a pin,
try writing to one of your defined pins
Code: [Select]

const int colorchange1 = 11; //red dual color pin
const int colorchange2 = 12; //green dual color pin
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.

packocrayons

You're right, I missed that. Either way, the led still goes high during the setup.
Modified code, still not looping though
Code: [Select]
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;
  }
  }
}

PeterH


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.
I only provide help via the forum - please do not contact me for private consultancy.

CrossRoads

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.
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.

packocrayons

#8
Oct 06, 2012, 05:07 am Last Edit: Oct 06, 2012, 05:47 am by packocrayons Reason: 1
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: [Select]

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;
 }
 }
}

CrossRoads

Well, that would indicate this isn't doing what you want:
Code: [Select]

  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.
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.

packocrayons

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.

Go Up