Sketch review Please

K so its been a long process but I think my final draft sketch is complete. I’m still very new to this and would like anyone to look it over for me. I’m still waiting for my hardware to show up so I haven’t been able to upload or run anything. Any type of feedback on better ways to write something would be much appreciated.

boolean debug = true;

#include <avr/interrupt.h>
#include <avr/power.h>
#include <avr/sleep.h>
#include <SoftwareSerial.h>

SoftwareSerial BTserial(10, 11); // RX | TX
// Connect the HC-06 TX to the Arduino RX.
// Connect the HC-06 RX to the Arduino TX

// max length of command is 20 chrs
const byte numChars = 20;
char receivedChars[numChars];
boolean newData = false;

// constants won't change.
const int downbutton = 2;       // downbutton to pin 2
const int upbutton = 3;         // upbutton to pin 3
const int relay1 =   7;         // relay1 to pin 7
const int relay2 =   6;         // relay2 to pin 6
const int relay3 =   5;         //   relay3 to pin 5
const int relay4 =   4;         // relay4 to pin 4
const int accpi =   8;           // car acc power in to pin 8

// variables will change:read for input values
int buttonState2 = 0;
int buttonState3 = 0;
int accps = 0;
int relay4s = 0;

void setup() {

  Serial.begin(9600);
  Serial.println("<Arduino is ready>");
  BTserial.begin(9600);

    pinMode(accpi, INPUT);
    pinMode(upbutton, INPUT);         // sets the downbutton pin as input
    pinMode(downbutton, INPUT);       // sets the downbutton pin as input
    pinMode(relay1, OUTPUT);       // sets the relay pin as output
    pinMode(relay2, OUTPUT);       // sets the relay pin as output
    pinMode(relay3, OUTPUT);       // sets the relay pin as output
    pinMode(relay4, OUTPUT);       // sets the relay pin as output
    pinMode(13, OUTPUT);           // set pin 13 as an output so we can use LED to monitor sleep status

  digitalWrite(relay1, LOW);     // Prevents relays from starting up engaged
    digitalWrite(relay2, LOW);     // Prevents relays from starting up engaged
    digitalWrite(relay3, LOW);     // Prevents relays from starting up engaged
    digitalWrite(relay4, LOW);     // Prevents relays from starting up engaged
}



void loop() {

  digitalWrite(13, HIGH);      // turn pin 13 LED on
    buttonState3  =  digitalRead(upbutton);         //   read the state of the upbutton
    buttonState2  =  digitalRead(downbutton);       //    read the state of the downbutton
  accps = digitalRead(accpi);                     //   read the state of the accps
  relay4s = digitalRead(relay4);

  if (accps == HIGH && buttonState2 == HIGH && relay4s == LOW) {
       ON();      
  }

    if (accps == HIGH && buttonState3 == HIGH && relay4s == HIGH) {
       OFF();   
  }

    if (accps == LOW && relay4s == HIGH) {
       AutoOFF();
  }

  if (buttonState2 == LOW || buttonState3 == LOW) {
    // sets relay 1/2 to off if no switches are pressed:
    digitalWrite(relay1, LOW);
    digitalWrite(relay2, LOW);
  }



  if (BTserial.available() > 0)     {
    recvWithStartEndMarkers();
  }

  if (newData) {
    parseData();
  }

  if (accps == LOW) {
    // Stay awake for 1 second, then sleep.
    delay(1000);
    sleepNow();
  }
}


void parseData()
{
  newData = false;
  if (debug) {
    Serial.println( receivedChars );
  }
  if (receivedChars[0] == 'O'  && receivedChars[1] == 'N' && accps == HIGH && relay4s == LOW )  {
    // turn relay 3/4 on w/ relay 1 on for 16 sec:
    ON();
  }

  if (receivedChars[0] == 'O'  && receivedChars[1] == 'F' && accps == HIGH && relay4s == HIGH )  {
    OFF();
  }
}


void ON() {
  // turn relay 3/4 on w/ relay 1 on for 16 sec:
  digitalWrite(relay3, HIGH);
  digitalWrite(relay4, HIGH);
  digitalWrite(relay1, HIGH);  
  delay(5000);
  BTserial.println("ON"); BTserial.print("##");
  if (debug) {
    Serial.println("ON");
  }
  BTserial.println("APPOPEN"); BTserial.print("##");
  if (debug) {
    Serial.println("APPOPEN");
  }
}

void OFF() {
  // turn relay 3 off w/ relay 2/4 on for 16 sec then off:
      digitalWrite(relay3, LOW);
      digitalWrite(relay2, HIGH);
      delay (5000);
      digitalWrite(relay4, LOW);
  BTserial.println("OFF"); BTserial.print("##");
  if (debug) {
    Serial.println("OFF");
  }
}

void AutoOFF() {
  // when accp is off for 3 mins turn relay 3 off w/ relay 2/4 off after 16 secs:
      delay(5000); //add0
      digitalWrite(relay3, LOW);
      digitalWrite(relay2, HIGH);
      delay(5000);
      digitalWrite(relay4, LOW);
  BTserial.println("APPCLOSE"); BTserial.print("##");
  if (debug) {
    Serial.println("APPCLOSE");
  }
}

void recvWithStartEndMarkers()
{
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char startMarker = '<';
  char endMarker = '>';
  char rc;

  if (BTserial.available() > 0)
  {
    rc = BTserial.read();
    if (recvInProgress == true)
    {
      if (rc != endMarker)
      {
        receivedChars[ndx] = rc;
        ndx++;
        if (ndx >= numChars) {
          ndx = numChars - 1;
        }
      }
      else
      {
        receivedChars[ndx] = '\0'; // terminate the string
        recvInProgress = false;
        ndx = 0;
        newData = true;
      }
    }

    else if (rc == startMarker) {
      recvInProgress = true;
    }
  }
}


void rupt(void) {
  // just wake from sleep
}

void sleepNow(void) {
    
    // Choose our preferred sleep mode
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);

  // Enable interrupt
  attachInterrupt(0, rupt, HIGH);
  // pin 2
  attachInterrupt(1, rupt, HIGH);
  // pin 3

    // Set sleep enable (SE) bit:
    sleep_enable();

    digitalWrite(13, LOW);       // turn LED off to indicate sleep

    // Put the device to sleep:
    sleep_mode();

    // Upon waking up, sketch continues from this point.
    sleep_disable();
}
  Serial.println("<Arduino is ready>");

That string literal will be copied, unnecessarily, from program memory to SRAM, at run time. Stop that by using the F() macro:

Serial.println(F(""));

    pinMode(accpi, INPUT);
    pinMode(upbutton, INPUT);         // sets the downbutton pin as input
    pinMode(downbutton, INPUT);       // sets the downbutton pin as input

The normal convention for naming variables is camelCase. The normal convention for comments is to not have useless ones. Useful ones need to match the code.

  relay4s = digitalRead(relay4);

Why do you need to read the state of an OUTPUT pin? You set it to some state. Remember that.

  // turn relay 3/4 on w/ relay 1 on for 16 sec:
  digitalWrite(relay3, HIGH);
  digitalWrite(relay4, HIGH);
  digitalWrite(relay1, HIGH);  
  delay(5000);
  BTserial.println("ON"); BTserial.print("##");
  if (debug) {
    Serial.println("ON");
  }
  BTserial.println("APPOPEN"); BTserial.print("##");
  if (debug) {
    Serial.println("APPOPEN");
  }

That is NOT what that code does.

  // Enable interrupt
  attachInterrupt(0, rupt, HIGH);
  // pin 2
  attachInterrupt(1, rupt, HIGH);
  // pin 3

This is usually done in setup().

Some explanation of that the code is supposed to do should precede the code.

In general if you have more than two variables that have the same type and the same name except for a trailing integer you should consider using an array instead. Your relay1, relay2, relay3, relay4 are an example.

const int relayPins[] = {7, 6, 5, 4};

Of course if the relays are't all doing the same thing it would make sense to name them something more meaningful than "relay":

const int IntakePumpPin =   7;
const int ExhaustPumpPin =   6;
const int ColdWaterValvePin =   5;
const int PowerRelayPin =   4;

It is generally not helpful when your comment just repeats what the code is saying. Especially when you decide to change the code and not the comment.

As far as
not Reading the state of relay4(changed to realy3 with the array)
how else can I remember this if it is changing. I did some research but nothing helpful.

Create a global variable, relay4State.
Assign it whatever initial value is appropriate.

When you need to change the state, change the value in relayState. Use that value, NOT some hardcoded value when writing to the pin.

byte relay4State = HIGH;

    relay4State = LOW;
    digitalWrite(relay4Pin, relay4State);
  BtnState3  =  digitalRead(upBtn);        
  BtnState2  =  digitalRead(downBtn);

Wouldn’t upState and downState make more sense? When you want to use the value associated with the up switch, why would you want to consult a cheat sheet to determine the name of the variable you stored the value in?

Learn now to use the smallest variable type that is reasonable.

const int downBtn = 2;      
const int upBtn = 3;      
const int relay[] = {7, 6, 5, 4};  
const int accpi = 8;   // car acc power in to pin 8

Arduinos don’t have enough pins to warrant using ints to hold pin numbers.

int BtnState2 = 0;
int BtnState3 = 0;
int accps = 0;
int relay3s = 0;

To hold HIGH or LOW?

  for (int a = 0; a < 4; a++)

To count to 4?

  if (accps == LOW)
  {
    delay(1000);
    sleepNow();

If the accessory power is lost, piss away a full second, then go to sleep. Why?

The Arduino uses very little power. Since it appears that the purpose of this device has something to do with a car, and that car presumably has a decent battery, is sleeping the Arduino really necessary?