Sequential Tail Lights Small Issues

Projects Programming
1

Hello everyone,

I'm working on code to run an Uno to control sequential tail lights for my 2012 Challenger. I'm still pretty new to coding and Arduino so I have pieced together code from other places on the internet to come up with what I am working on right now. A lot of my code is sloppy and is missing a bunch of comments because I barely know what's going on.

MY ISSUE- is that the sequence for the lights for the right side of the car run through their sequence when power is applied to the input pin AND when power is removed from the input pin. The correct operation is for the sequence to start and run only once when power is applied and to do nothing when power is removed from the input pin.

I will probably have several other issues later on but this is the only one I'm concerned about for now. You'll notice that the entire brake light section of the code has been commented out-I'm still working on this part and I haven't really tested it or put much thought into it yet.

If you see anything else that could use improving I am always open to suggestions. Thanks for looking!

(I have posted the code in the first reply because it exceeds the 9,000 character limit)

2 
                            //THIRD BRAKE LIGHT
// LEFT4 LEFT3 LEFT2 LEFT1 (REVERSE)(REVERSE) RIGHT1 RIGHT2 RIGHT3 RIGHT4


//Global Variables
const byte BrakeSwitch = 10; //BRAKE trigger

const byte RightSwitch = 11; //RIGHT trigger
const byte RIGHT1 =6;                            
const byte RIGHT2 = 7;
const byte RIGHT3 = 8;
const byte RIGHT4 = 9;



const byte LeftSwitch = 12;  //LEFT trigger
const byte LEFT1 = 5;   
const byte LEFT2 = 4; 
const byte LEFT3 = 3; 
const byte LEFT4 = 2; 


 
unsigned long buttonPushedMillis; // when button was released
unsigned long ledTurnedOnAt; // when led was turned on

unsigned long LEFTbuttonPushedMillis; // when button was released
unsigned long LEFTledTurnedOnAt; // when led was turned on


unsigned long turnOnDelay = 1; // wait to turn on LED
unsigned long turnOffDelay = 700; // turn off LED after this time
bool ledReady = false; // flag for when button is let go
bool ledState = false; // for LED is on or not.
 
 bool LEFTledReady = false; // flag for when button is let go
bool LEFTledState = false;
 
 
void setup() {

 pinMode(RIGHT1, OUTPUT);
  pinMode(RIGHT2, OUTPUT);
   pinMode(RIGHT3, OUTPUT);
    pinMode(RIGHT4, OUTPUT);    
    
 pinMode(LEFT1, OUTPUT);
  pinMode(LEFT2, OUTPUT);
   pinMode(LEFT3, OUTPUT);
    pinMode(LEFT4, OUTPUT);    
    
 pinMode(BrakeSwitch, INPUT_PULLUP);
 pinMode(RightSwitch, INPUT_PULLUP);
 pinMode(LeftSwitch, INPUT_PULLUP);


    
    
 digitalWrite(RIGHT1, LOW);
  digitalWrite(RIGHT2, LOW);
  digitalWrite(RIGHT3, LOW);
  digitalWrite(RIGHT4, LOW);
  digitalWrite(LEFT1, LOW);
  digitalWrite(LEFT2, LOW);
  digitalWrite(LEFT3, LOW);
  digitalWrite(LEFT4, LOW);
}
 
void loop() {
 // get the time at the start of this loop()
 unsigned long currentMillis = millis(); 
 
 unsigned long LEFTcurrentMillis = millis(); 
////// RIGHT TURN SIGNAL
 
 if (digitalRead(RightSwitch) == LOW) {
  // update the time when button was pushed

  
  buttonPushedMillis = currentMillis;
  ledReady = true;
 }
  
 // make sure this code isn't checked until after button has been let go
 if (ledReady) {
   //this is typical millis code here:
   if ((unsigned long)(currentMillis - buttonPushedMillis) >= turnOnDelay) {
     // okay, enough time has passed since the button was let go.
     digitalWrite(RIGHT1, HIGH);
     // setup our next "state"
     ledState = true;
     // save when the LED turned on
     ledTurnedOnAt = currentMillis;
     // wait for next button press
     ledReady = false;
   }
 }
  
 // see if we are watching for the time to turn off LED
 if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/4) {
     ledState = true;
     digitalWrite(RIGHT2, HIGH);   
   }
   
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/2) {
     ledState = true;
     digitalWrite(RIGHT3, HIGH);   
   }
    }
     
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= 3*turnOffDelay/4) {
     ledState = true;
     digitalWrite(RIGHT4, HIGH);   
   }
   }
   
if (ledState) {
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay) {
     ledState = false;
     digitalWrite(RIGHT1, LOW);
     digitalWrite(RIGHT2, LOW);
     digitalWrite(RIGHT3, LOW);
     digitalWrite(RIGHT4, LOW);
   }
 }
 }
 
 /*
 
 
 if (digitalRead(RightSwitch) == LOW) {
  // update the time when button was pushed
  buttonPushedMillis = currentMillis;
 ledReady = true;
 }
  
 // make sure this code isn't checked until after button has been let go
 if (ledReady) 
 {
   //this is typical millis code here:
   if ((unsigned long)(currentMillis - buttonPushedMillis) >= turnOnDelay) {
     // okay, enough time has passed since the button was let go.
     digitalWrite(RIGHT1, HIGH);
     // setup our next "state"
     ledState = true;
     // save when the LED turned on
     ledTurnedOnAt = currentMillis;
     // wait for next button press
     ledReady = false;
   }
 }
  
 // see if we are watching for the time to turn off LED
 if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/4) {
     ledState = true;
     digitalWrite(RIGHT2, HIGH);   
   }
   
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/2) {
     ledState = true;
     digitalWrite(RIGHT3, HIGH);   
   }
    }
     
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= 3*turnOffDelay/4) {
     ledState = true;
     digitalWrite(RIGHT4, HIGH);   
   }}
   
   
   
   
if (ledState) {
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay) {
     
     digitalWrite(RIGHT1, LOW);
     digitalWrite(RIGHT2, LOW);
     digitalWrite(RIGHT3, LOW);
     digitalWrite(RIGHT4, LOW);
     ledState = false;
   }}
 }
*/
///// LEFT TURN SIGNAL

if (digitalRead(LeftSwitch) == LOW) {
  // update the time when button was pushed

  
  LEFTbuttonPushedMillis = LEFTcurrentMillis;
  LEFTledReady = true;
 }
  
 // make sure this code isn't checked until after button has been let go
 if (LEFTledReady) {
   //this is typical millis code here:
   if ((unsigned long)(LEFTcurrentMillis - LEFTbuttonPushedMillis) >= turnOnDelay) {
     // okay, enough time has passed since the button was let go.
     digitalWrite(LEFT1, HIGH);
     // setup our next "state"
     LEFTledState = true;
     // save when the LED turned on
     LEFTledTurnedOnAt = LEFTcurrentMillis;
     // wait for next button press
     LEFTledReady = false;
   }
 }
  
 // see if we are watching for the time to turn off LED
 if (LEFTledState) {
   // okay, led on, check for now long
   if ((unsigned long)(LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/4) {
     LEFTledState = true;
     digitalWrite(LEFT2, HIGH);   
   }
   
    if (LEFTledState) {
   // okay, led on, check for now long
   if ((unsigned long)(LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/2) {
     LEFTledState = true;
     digitalWrite(LEFT3, HIGH);   
   }
    }
     
    if (LEFTledState) {
   // okay, led on, check for now long
   if ((unsigned long)(LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/1.333) {
     LEFTledState = true;
     digitalWrite(LEFT4, HIGH);   
   }
   }
   
if (LEFTledState) {
   if ((unsigned long)(LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay) {
     LEFTledState = false;
     digitalWrite(LEFT1, LOW);
     digitalWrite(LEFT2, LOW);
     digitalWrite(LEFT3, LOW);
     digitalWrite(LEFT4, LOW);
   }
   
 }
 }

///////BRAKE LIGHTS
 /* 
 if (digitalRead(BrakeSwitch) == LOW) {
  // update the time when button was pushed
  buttonPushedMillis = currentMillis;
  ledReady = true;
 }
  
 // make sure this code isn't checked until after button has been let go
 if (ledReady) {
   //this is typical millis code here:
   if ((unsigned long)(currentMillis - buttonPushedMillis) >= turnOnDelay) {
     // okay, enough time has passed since the button was let go.
     digitalWrite(LEFT1, HIGH);
     digitalWrite(RIGHT1, HIGH);
     // setup our next "state"
     ledState = true;
     // save when the LED turned on
     ledTurnedOnAt = currentMillis;
     // wait for next button press
     ledReady = false;
   }
 }
  
 // see if we are watching for the time to turn off LED
 if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/4) {
     ledState = true;
     digitalWrite(LEFT2, HIGH); 
     digitalWrite(RIGHT2, HIGH);
   }
   
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/2) {
     ledState = true;
     digitalWrite(LEFT3, HIGH);   
     digitalWrite(RIGHT3, HIGH);
   }
    }
     
    if (ledState) {
   // okay, led on, check for now long
   if ((unsigned long)(currentMillis - ledTurnedOnAt) >= turnOffDelay/1.333) {
     ledState = true;
     digitalWrite(LEFT4, HIGH);  
     digitalWrite(RIGHT4, HIGH);
   }}
   
if (ledState) {
   if (digitalRead(BrakeSwitch) == HIGH) {
     ledState = false;
     digitalWrite(LEFT1, LOW);
     digitalWrite(LEFT2, LOW);
     digitalWrite(LEFT3, LOW);
     digitalWrite(LEFT4, LOW);
     digitalWrite(RIGHT1, LOW);
     digitalWrite(RIGHT2, LOW);
     digitalWrite(RIGHT3, LOW);
     digitalWrite(RIGHT4, LOW);
   }}
 }
  */
  
}
3

That's a lot of code to blink & switch a few outputs.
If soneone else (@bluefin) doesn't jump in first - I'll have a look overnight

4

lastchancename:
If soneone else (@bluefin) doesn't jump in first

:wink:

5

Aceattack52:
Hello everyone,

I'm working on code to run an Uno to control sequential tail lights for my 2012 Challenger. I'm still pretty new to coding and Arduino so I have pieced together code from other places on the internet to come up with what I am working on right now. A lot of my code is sloppy and is missing a bunch of comments because I barely know what's going on.

Maybe if you barely know what's going on it's best not to mess with a safety component of your car. In saying that I hope that you only decide to put it out on the road after thoroughly testing it.

Aceattack52:
MY ISSUE- is that the sequence for the lights for the right side of the car run through their sequence when power is applied to the input pin AND when power is removed from the input pin. The correct operation is for the sequence to start and run only once when power is applied and to do nothing when power is removed from the input pin.

I assume you mean when the input pin goes HIGH or LOW by this.

Aceattack52:
I will probably have several other issues later on but this is the only one I'm concerned about for now. You'll notice that the entire brake light section of the code has been commented out-I'm still working on this part and I haven't really tested it or put much thought into it yet.

If you see anything else that could use improving I am always open to suggestions. Thanks for looking!

(I have posted the code in the first reply because it exceeds the 9,000 character limit)

If the brake light section isn't of concern right now then delete it from your sketch before posting it here. You also have a lot of white space in your code which you should probably get rid of to make the code easier to read. Another thing to help reading your code is to auto format it (Ctrl + T) in the arduino IDE.

OK, on to the code. There is a lot of repeated code in this sketch that could probably be incorporated into functions. Furthermore it is very difficult to tell what you are doing. Try reading this post on Planning and Implementing an Arduino Program.

To make your program easier to read I would recommend splitting your loop into several functions and calling only those functions from loop to make it easier to read and understand your program as per the example in the link above.

6

TBH - I got lost trying to figure out what you were doing...
Here's your exizting code with all the unnecessary stuff pulled out

                           //THIRD BRAKE LIGHT
// LEFT4 LEFT3 LEFT2 LEFT1 (REVERSE)(REVERSE) RIGHT1 RIGHT2 RIGHT3 RIGHT4


//Global Variables
const byte BrakeSwitch = 10; //BRAKE trigger

const byte RightSwitch = 11; //RIGHT trigger
const byte RIGHT1 =6;                            
const byte RIGHT2 = 7;
const byte RIGHT3 = 8;
const byte RIGHT4 = 9;



const byte LeftSwitch = 12;  //LEFT trigger
const byte LEFT1 = 5;   
const byte LEFT2 = 4; 
const byte LEFT3 = 3; 
const byte LEFT4 = 2; 

unsigned long currentMillis;

unsigned long buttonPushedMillis; // when button was released
unsigned long ledTurnedOnAt; // when led was turned on

unsigned long LEFTbuttonPushedMillis; // when button was released
unsigned long LEFTledTurnedOnAt; // when led was turned on


unsigned long turnOnDelay = 1; // wait to turn on LED
unsigned long turnOffDelay = 700; // turn off LED after this time
bool ledReady = false; // flag for when button is let go
bool ledState = false; // for LED is on or not.
 
bool LEFTledReady = false; // flag for when button is let go
bool LEFTledState = false;
 
// ------------------------------
void setup() {

	pinMode(RIGHT1, OUTPUT);
	pinMode(RIGHT2, OUTPUT);
	pinMode(RIGHT3, OUTPUT);
	pinMode(RIGHT4, OUTPUT);    
    
	pinMode(LEFT1, OUTPUT);
	pinMode(LEFT2, OUTPUT);
	pinMode(LEFT3, OUTPUT);
	pinMode(LEFT4, OUTPUT);    
    
	pinMode(BrakeSwitch, INPUT_PULLUP);
	pinMode(RightSwitch, INPUT_PULLUP);
	pinMode(LeftSwitch, INPUT_PULLUP);
    
	digitalWrite(RIGHT1, LOW);
	digitalWrite(RIGHT2, LOW);
	digitalWrite(RIGHT3, LOW);
	digitalWrite(RIGHT4, LOW);
	digitalWrite(LEFT1, LOW);
	digitalWrite(LEFT2, LOW);
	digitalWrite(LEFT3, LOW);
	digitalWrite(LEFT4, LOW);
}
 
// ------------------------------
void loop() {
	// get the time at the start of this loop()
	currentMillis = millis(); 
	LEFTcurrentMillis = millis(); 
 
	////// RIGHT TURN SIGNAL
	if (digitalRead(RightSwitch) == LOW) {
		// update the time when button was pushed
		buttonPushedMillis = currentMillis;
		ledReady = true;
	}
  
	// make sure this code isn't checked until after button has been let go
	if (ledReady) {
		//this is typical millis code here:
		if ((currentMillis - buttonPushedMillis) > turnOnDelay) {
		 // okay, enough time has passed since the button was let go.
		 digitalWrite(RIGHT1, HIGH);
		 // setup our next "state"
		 ledState = true;
		 // save when the LED turned on
		 ledTurnedOnAt = currentMillis;
		 // wait for next button press
		 ledReady = false;
		}
		
		// see if we are watching for the time to turn off LED
		if (ledState) {
			
			// okay, led on, check for now long
			if ((currentMillis - ledTurnedOnAt) >= turnOffDelay/4) {
			 ledState = true;
			 digitalWrite(RIGHT2, HIGH);   
			}
		 
			// okay, led on, check for now long
			if ((currentMillis - ledTurnedOnAt) >= turnOffDelay/2) {
			 ledState = true;
			 digitalWrite(RIGHT3, HIGH);   
			}
			
			// okay, led on, check for now long
			if ((currentMillis - ledTurnedOnAt) >= 3*turnOffDelay/4) {
			 ledState = true;
			 digitalWrite(RIGHT4, HIGH);   
			}
			
			// okay, led on, turn them all off
			if ((currentMillis - ledTurnedOnAt) >= turnOffDelay) {
				ledState = false;
				digitalWrite(RIGHT1, LOW);
				digitalWrite(RIGHT2, LOW);
				digitalWrite(RIGHT3, LOW);
				digitalWrite(RIGHT4, LOW);
			}
		}
	}
  
	///// LEFT TURN SIGNAL

	if (digitalRead(LeftSwitch) == LOW) {
		// update the time when button was pushed
		LEFTbuttonPushedMillis = LEFTcurrentMillis;
		LEFTledReady = true;
	}
  
	// make sure this code isn't checked until after button has been let go
	if (LEFTledReady) {
		//this is typical millis code here:
		if ((LEFTcurrentMillis - LEFTbuttonPushedMillis) >= turnOnDelay) {
			// okay, enough time has passed since the button was let go.
			digitalWrite(LEFT1, HIGH);
			// setup our next "state"
			LEFTledState = true;
			// save when the LED turned on
			LEFTledTurnedOnAt = LEFTcurrentMillis;
			// wait for next button press
			LEFTledReady = false;
		}
	}

	// see if we are watching for the time to turn off LED
	if (LEFTledState) {
		// okay, led on, check for now long
		if ((LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/4) {
			LEFTledState = true;
			digitalWrite(LEFT2, HIGH);   
		}
		// okay, led on, check for now long
		if ((LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/2) {
			LEFTledState = true;
			digitalWrite(LEFT3, HIGH);   
		}
		// okay, led on, check for now long
		if ((LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay/1.333) {
			LEFTledState = true;
			digitalWrite(LEFT4, HIGH);   
		}
		// okay, led on, turn them all off
		if ((LEFTcurrentMillis - LEFTledTurnedOnAt) >= turnOffDelay) {
			LEFTledState = false;
			digitalWrite(LEFT1, LOW);
			digitalWrite(LEFT2, LOW);
			digitalWrite(LEFT3, LOW);
			digitalWrite(LEFT4, LOW);
		}
	}
}

HOWEVER
I'll look at it again - and see if we can make it shorter, cleaner and more consistent - unless someone else takes a shot at it first!
I'm not going to mock it up on hardware - just a complete rewrite and see if it compiles.

7

Thanks lastchancename- there is a lot of good information here for me to chew on. I tried your code on the hardware I have set up for this and it works the same as my original code. This is fine because hopefully with the more concise code I can spot what's wrong a little easier.

Metallor- I'm sorry but what you posted isn't really helpful. Picking apart my original post and telling me what I need to do differently instead of how to do it isn't helpful. I know what needs to be done; I came here to see if anyone can help me do it.

8

Aceattack52:
Metallor- I'm sorry but what you posted isn't really helpful.

Metallor's advice is totally legitimate:

  • Physically removing commented out blocks makes things easier for those inclined to help to read
  • Removing white space and doing a control-T makes things easier for those inclined to help to read
  • You in any case agreed with lastchancename that your trimmed code is easier to understand: that's what metallor advocated
  • Putting repeated code in functions is an excellent practice that makes the code easier for those inclined to help to understand, not to mention eaiser for you in the long run
  • Calling functions from loop() is an excellent idea, makes the code a zillion % easier to understand
  • The link he or she gave tells you how to do the function-related stuff
9

I wanted to come back to the couple of forum posts I made to post an update in case it helps anyone else out. The system I ended up going with includes a bank of 16 transistors, one PNP and one NPN for each light since the lights (and most other electronics for automotive applications for that matter) are common ground. Then I used two Arduino Unos-one for each side. Using two boards allowed me to use delay() in my code rather than millis(). This was the right move for this application because I could get delay() to work 100% of the time whereas I found millis() to be unreliable. These Arduinos work independently of each other at the moment, I have a plan to connect them later for even more cool tricks. I wanted to go with an Arduino set up to give me some room for creativity. A system that just used a few simple electrical components could have given me the same result but now I can program any sequence to run when I hit the brake/turn signal/hazards ect.

Some people had safety concerns with this. I tested this system extensively in the garage before driving with it. I have driven it several hundred miles at this point without any sort of failure. Part of the safety aspect here was using delay() rather than millis().

Another note about the code-the car supplies a small amount of voltage to the lights at all times for some reason and I had to use an analog input rather than a digital input to account for that.

For anyone doing this project, below I have posted the code I used for just one side. The code is the same no matter which side it is running. The way it is wired allows for me to do that. I have also posted the link to a YouTube video of the final product with just one of the sequences I came up with. I hope the video shows up and continues to work into the future.

const int threshold = 615;
void setup()
{
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
}

void loop()
{
  if (analogRead(A0) > threshold)
  {
    digitalWrite(4, HIGH);
    delay(50);
    digitalWrite(3, HIGH);
    digitalWrite(4, LOW);
    delay(70);
    digitalWrite(3, LOW);

    digitalWrite(2, HIGH);
    delay(50);
    digitalWrite(3, HIGH);
    delay(50);
    digitalWrite(4, HIGH);
    delay(50);
    digitalWrite(5, HIGH);
    delay(200);

    while (analogRead(A0) > threshold)
    {
      digitalWrite(2, HIGH);
      digitalWrite(3, HIGH);
      digitalWrite(4, HIGH);
      digitalWrite(5, HIGH);
    }
  }
  if (analogRead(A0) < threshold)
  { digitalWrite(2, LOW);
    digitalWrite(3, LOW);
    digitalWrite(4, LOW);
    digitalWrite(5, LOW);
  }
}
10

Ok, well 2 uno's sounds like complete overkill regardless in fact for animated lights (i have just one for my 3rd brakelight) i would just use a bit-shifter (4015 or similar) and a 555 timer, no need for arduino's.

11

Deva_Rishi:
Ok, well 2 uno's sounds like complete overkill regardless in fact for animated lights (i have just one for my 3rd brakelight) i would just use a bit-shifter (4015 or similar) and a 555 timer, no need for arduino's.

This is a safety-related device there is no "overkill"

12

Simplicity.
Multiple points of Failure.

13

small amount of voltage to the lights at all times for some reason

Most likely so the car computer can detect a broken bulb or damaged wire. If you look at your onboard fault codes, you will probably see this logged as a fault. The car's own driver circuits are likely more complex than your NPN+PNP. Look at the datasheet for components like the BTS series of drivers from Infineon.

14

Aceattack52:
This is a safety-related device there is no "overkill"

The "overkill" does not add to safety in any way,