Trailer Light Tester - Need advice to simplify

[spartacus_84]

This is my first program, so be gentle
I have search, copied and augmented a lot of code from here, so first off, Thank You!!

This program works. But as a newbie, I'm sure there is a lot of stuff that could have been done more efficiently.
I'm looking for experts out there for input. Just a note, I have never programmed before. My history includes 1 year of electronics vocational training 18 years ago in High School. I perform technical support of an agriculture implement manufacturing company.
This has been one of the most frustrating and then rewarding last couple days I've had in a while.
There was a great tester (Wolf) on the marker we used in the past, but they have since went under. Our replacement testers worked, but production didn't like them.
The program works in this simplified fashion.
Cycle through all the light outputs of a trailer continuously one at a time.
If the user wants to check a individual function, latching buttons will allow them to single out that function or multiple.
I have seen this requested before on the forum and no one had a complete program.
So here you go!

/*
  Trailer Light Tester
  Mitchell Wood 2/22/21

  Used to test the light functions of a trailer for production in our manufacturing facility.
  This works for trailers with brake output as a combination of left and right as well as an independent brake output. 
  Ag vehicles require an independent brake light as well as left and right hazards
//Outputs that control a relay for 12V Source Switching
int leftOut = 7;    // Left
int rightOut = 6;  // Right
int brakeOut = 5; // Brake Lights
int markerOut = 4; // Marker

//Inputs for 4 latching push buttons
// All inputs need to be low. Add 10K ohm resistor to pull down
int leftIn = 8; //Left Lamp Button
int rightIn = 9; //Right Lamp Button
int brakeIn = 10; //Brake Lamp Button
int markerIn = 11; //Marker Lamp Button

//Button States Current state values
int leftState = 0;
int rightState = 0;
int brakeState = 0;
int markerState = 0;

//Settings
int bt = 333; // Blink Time Delay - 1/3 sec or 90hz
int timerState = 0; //Current Loop Time
int lastTimerState = 0; //Last Loop Time
int timerLED = 13; //BuiltIn LED for troubleshooting
int timerCounter = 0; //Incremental counter for cycling through Auto Sequence
long previousMillis = 0;


void setup()
{
  pinMode(leftOut, OUTPUT);
  pinMode(rightOut, OUTPUT);
  pinMode(brakeOut, OUTPUT);
  pinMode(markerOut, OUTPUT);
  pinMode(leftIn, INPUT);
  pinMode(rightIn, INPUT);
  pinMode(brakeIn, INPUT);
  pinMode(markerIn, INPUT);
  pinMode(timerLED, OUTPUT);
  Serial.begin(9600); //Debugging
}

void loop()
{
  //READ Inputs - Program reads current state of all inputs and writes them to appropriate State value
  leftState = digitalRead(leftIn);
  rightState = digitalRead(rightIn);
  brakeState = digitalRead(brakeIn);
  markerState = digitalRead(markerIn);
  //TIMER
  unsigned long currentMillis = millis(); //get current time and save to currentMillis Value
  if (currentMillis - previousMillis > bt) // if time is after Blink cycle, Reset to 0
  {
    previousMillis = currentMillis;
    if (timerState == LOW)  //Toggle Timer
      timerState = HIGH;
    else
      timerState = LOW;  //Continue until time reached
    digitalWrite(timerLED, timerState); //write timer state to builtin led
  }

  //TIMER Counter
  timerState = digitalRead(timerLED);  //
  if ((timerState != lastTimerState))
    if (timerState == HIGH) {
      timerCounter++;                   //Increment count by 1
      Serial.println(timerCounter);     //Serial Output - Show current count
    }
  lastTimerState = timerState;          //Save timer state
                                        //RESET COUNTER
  if (timerCounter >= 21) {             //Count to 21 and then
    timerCounter = 0;                   //Reset
  }
 //MANUAL OVERRIDE MODE
  // Is Brake ON,LEFT ON,RIGHT ON  
  if ((brakeState == HIGH) && (leftState == HIGH) && (rightState == HIGH))    //Brake is HIGH and L/R flash in unison Simulates Pressing Brake and Hazards
  {
    digitalWrite(brakeOut, HIGH);
    digitalWrite(leftOut, timerState);
    digitalWrite(rightOut, timerState);
    timerCounter = 0;
  }
  // Is Brake ON,LEFT OFF,RIGHT OFF
  else if ((brakeState == HIGH) && (leftState == LOW) && (rightState == LOW))  //Brake is HIGH and L/R are HIGH - Simulates pressing Break pedal
    digitalWrite(brakeOut, HIGH);
    digitalWrite(leftOut, HIGH);
    digitalWrite(rightOut, HIGH);
    timerCounter = 0;
  }
  // Is Brake ON,LEFT ON,RIGHT OFF
  else if ((brakeState == HIGH) && (leftState == HIGH) && (rightState == LOW)) //Simulates Braking while turning left
  {
    digitalWrite(brakeOut, HIGH);
    digitalWrite(leftOut, timerState);
    digitalWrite(rightOut, HIGH);
    timerCounter = 0;
  }
  // Is Brake ON,LEFT OFF,RIGHT ON
  else if ((brakeState == HIGH) && (leftState == LOW) && (rightState == HIGH))  //Simulates Braking while turning right
  {
    digitalWrite(brakeOut, HIGH);
    digitalWrite(leftOut, HIGH);
    digitalWrite(rightOut, timerState);
    timerCounter = 0;
  }
  // Is BRAKE OFF,LEFT ON,RIGHT ON
  else if ((brakeState == LOW) && (leftState == HIGH) && (rightState == HIGH))  //For Diagnosing wiring issues - Toggles between Left and Right HIGH
  {
    digitalWrite (brakeOut, LOW);
    digitalWrite (leftOut, timerState);
    digitalWrite (rightOut, !timerState);
    timerCounter = 0;
  }
  // Is BRAKE OFF,LEFT ON,RIGHT OFF
  else if ((brakeState == LOW) && (leftState == HIGH) && (rightState == LOW))   //Simulates turning left, No Brakes
  {
    digitalWrite (brakeOut, LOW);
    digitalWrite (leftOut, timerState);
    digitalWrite (rightOut, LOW);
    timerCounter = 0;
  }
  // Is BRAKE OFF,LEFT OFF,RIGHT ON
  else if ((brakeState == LOW) && (leftState == LOW) && (rightState == HIGH))  //Simulates turning Right, No Brakes
  {
    digitalWrite (brakeOut, LOW);
    digitalWrite (leftOut, LOW);
    digitalWrite (rightOut, timerState);
    timerCounter = 0;
  }
  // Is All Off
  else if ((brakeState == LOW) && (leftState == LOW) && (rightState == LOW)) //No inputs HIGH, Continue to Auto Cycle
  {
    digitalWrite (brakeOut, LOW); // Sets BLR output to LOW
    digitalWrite (leftOut, LOW);
    digitalWrite (rightOut, LOW);
  }
  if (markerState == HIGH)            //Marker Light Test, Marker lights are an independent function so no need to mix with out functions
  {
    digitalWrite(markerOut, HIGH);   
    timerCounter = 0;
  }
  else {
    digitalWrite(markerOut, LOW);  //Sets Marker to LOW 
  }
 
 
 //AUTO CYCLE MODE
 //Cycle thru all 4 outputs for 5 flashes of the timerLED Each Function takes 3.34s to complete

  if ((brakeState == LOW) && (leftState == LOW) && (rightState == LOW) && (markerState == LOW)) 
  {
    if (timerCounter >= 0 && timerCounter <= 5) //Left Cycle
    {
      digitalWrite (leftOut, timerState);
      digitalWrite (rightOut, LOW);
      digitalWrite (brakeOut, LOW);
      digitalWrite (markerOut, LOW);
    }
    else if (timerCounter >= 6 && timerCounter <= 10) //Right Cycle
    {
      digitalWrite (leftOut, LOW);
      digitalWrite (rightOut, timerState);
      digitalWrite (brakeOut, LOW);
      digitalWrite (markerOut, LOW);
    }
    else if (timerCounter >= 11 && timerCounter <= 15) //Brake Cycle - this is for RV and Ag style outputs, so a independent brakeoutput is needed
    {
      digitalWrite (brakeOut, HIGH);
      digitalWrite (leftOut, HIGH);
      digitalWrite (rightOut, HIGH);
      digitalWrite (markerOut, LOW);
    }
    else if (timerCounter >= 16 && timerCounter <= 21)  //Marker Cycle
    {
      digitalWrite (leftOut, LOW);
      digitalWrite (rightOut, LOW);
      digitalWrite (brakeOut, LOW);
      digitalWrite (markerOut, HIGH);
    }
  }
}

[spartacus_84]

Login | Tinkercad

Tinkercad has been clutch to help me out. But it runs a little slow when my program grew.

I'm going to shove this all in a 50 ammo can and if the production guys like it, I'll make 3 more of them.

[Blackfin]

Nicely done. If it works and does what you want, excellent. Kudos for posting your code in "code tags."

You may have been missing a comment terminator at the top:

.
.
.
  This works for trailers with brake output as a combination of left and right as well as an independent brake output.
  Ag vehicles require an independent brake light as well as left and right hazards

 */  // <-- here

You may need to add some braces in your timer counter:

   //TIMER Counter
    timerState = digitalRead(timerLED);  //
    if ((timerState != lastTimerState))
    {// <-- here
        if (timerState == HIGH) 
        {
            timerCounter++;                   //Increment count by 1
            Serial.println(timerCounter);     //Serial Output - Show current count
        }
        lastTimerState = timerState;          //Save timer state
                                //RESET COUNTER
        if (timerCounter >= 21) 
        {             //Count to 21 and then
            timerCounter = 0;                   //Reset
        }

    }// <-- here

Are you missing a brace:

// Is Brake ON,LEFT OFF,RIGHT OFF
    else if ((brakeState == HIGH) && (leftState == LOW) && (rightState == LOW))  //Brake is HIGH and L/R are HIGH - Simulates pressing Break pedal
    {//  <-- here?
        digitalWrite(brakeOut, HIGH);

Your pin definitions could be 'byte' or 'uint8_t' instead of 'int' to save some RAM. They should also be 'const' to avoid code inadvertently changing it. E.g.

const uint8_t markerOut = 4; // Marker

For reading inputs like switches is generally preferred to use INPUT_PULLUP on the pinMode(...) and then use the switch to ground the pin. You'd then look for LOW to know if the switch is pressed.

[spartacus_84]

Thanks Blackfin.

The comment terminator was due to the 9000 char limit. It got me.

I have troubles with brackets... a lot

Thanks for the const uint8_t info. I'll read up on it. <--- This is why I posted, I didn't even knew that existed.

And I just read up on the INPUT_PULLUP. That would have solved about 2 hrs of "what the heck" Is this typical of what most do to avoid a pulldown resistor? just for circuit simplicity or is there an other advantage?

[CrossRoads]

Pull up resistor provides electrical separation between 5V and Gnd.
With a pull down and a miswired switch, a closed switch shorts 5V to Gnd.
With the internal pullup. a switch just connects a pin to Gnd, and 5V is isolated.

[SteveMann]

"I have troubles with brackets... a lot"

We'll make fun of you in a year...(vbg)

Use CTRL-T to format your code and any mismatched braces will be obvious.

Everything inside a pair of braces is a single statement. For example:

if(a==b) doSomething;

If doSomething is a series of code, then simply put it in braces:

if(a==b) {doSomething;
doSomethingElse;}

This is why setup() and loop() and really, any function opens and closes with matching braces:

void setup(){
  setupStatements;
}

If you have too many braces, and you do a CTRL-T, you will see closing braces at the same indent level, and probably a function begin indented.

Hope this helps.

[spartacus_84]

The completed project!

We have a large format latex printer that I made the decals on. There is a lot I want to do to simplify the hardware internally. I'm using an off the shelf 4 ch relay board and a couple of 40 amp relays for the high power outputs. Its powered by a 12ahr SLA battery. And there is enough hot glue inside to fund a small hobby store owner's kid college.

Ideally I would like to get to the level of designing my own PCB with a SLA charging circuit, 13.8VDC to 5VDC, four 10 amp relays and two 40 amp relays for the Electric Brake and Accessory outputs. with the common connected to the battery.

But it works and its solid. Hopefully our production team won't beat it up too bad.

Thanks for all the input guys!

[spartacus_84]

More Pics

[Blackfin]

Looks robust. Nicely done.

[TomGeorge]

Hi,
Very impressive...
I like the low profile button/switch on top, to protect them.
I have also used those LED illuminated press buttons on the side, they really help to polish a project off.

Tom...