newbie sketch help

in the void loop(), I don't know if it's right. could anyone help point the way?

//Relay Control
const int buttonPin[4] = {A7,A6,A5,A4};    //Input switches
const int ledPin[4] =  {2,3,4,5};          //Output relays
int buttonStates = LOW;                    //Switches at LOW


void setup() {
  for(int i =A7; i>3; i++) pinMode(i, INPUT); //swith input pin 
  for(int i =2; i<6; i++) pinMode(i, OUTPUT); //relay output pin
}

void loop() {
  for(byte i =A7; i>3; i++,buttonPin)           //switch pin
  for(byte i =2; i<6; i++,ledPin){              //relay pin
  int reading = digitalRead(buttonPin[i]);      //read switch
  if (buttonStates == HIGH) {                   //if switch is HIGH
    digitalWrite(ledPin[i], HIGH);              //turn on relay
  } else {
    digitalWrite(ledPin[i], LOW);               //turn off relay

  }
}
}

Always use {} with 'for' it makes things easier to read and debug.
Don't take shortcuts, one line per command.

Example: The first 'for' is missing {}

What are you trying to do?

.

How should we know what "right" is? You haven't told us what you want it to do.

But - I'll venture a guess it's not doing what you want it to, since buttonStates will always be LOW (you never set it to anything else), so the conditional will never be true, so it will always write all the ledpins low.

You are making invalid array accesses - arrays are zero-indexed - they start from zero. But in your for loops, you're starting the counter with the value of the first element. You should be starting from 0.

You also have two loops inside eachother, which is fine and normal, but:

  • You call the variable from both for loops i. Pick a different variable name for the second one, so you can refer to each of them.
  • You don't have curly braces for the first for loop. Always use curly braces for loops and any non-trivial if/then, if you omit them, the behavior is non-intuitive.
  • Your second loop looks like you're trying to index the ledpins array - but you're going all the way up to 5, when the last element is element 3 (the fourth one).
  • It looks like you don't actually want two loops, and instead just want one loop that counts from 0 through 3, and reads the corresponding pin in the buttonPins array, and writes the corresponding pin in the ledPins array.
  • the ,ledPin after the i++ is wrong.

I think you want something more like:

void loop() {
  for(byte i =0; i<4; i++){              //relay pin
  int reading = digitalRead(buttonPin[i]);      //read switch
  if (reading== HIGH) {                   //if switch is HIGH
    digitalWrite(ledPin[i], HIGH);              //turn on relay
  } else {
    digitalWrite(ledPin[i], LOW);               //turn off relay

  }
}
}

Or even:

void loop() {
  for(byte i =0; i<4; i++){
  digitalWrite(ledPin[i], digitalRead(buttonPin[i]));
}
}

I'm trying to make 4 relay turn "on" with 4 switch corresponding to the relay. So my sketch should be;

//Relay Control
const int buttonPin[4] = {A7,A6,A5,A4};    //Input switches
const int ledPin[4] =  {2,3,4,5};          //Output relays
int buttonStates = 0;                      //switch at LOW
int ledStates = 0;                         //relay at OFF



void setup() {
  for(int i =A7; i>3; i++) pinMode(i, INPUT); //swith input pin 
  for(int i =2; i<6; i++) pinMode(i, OUTPUT); //relay output pin
}

void loop() {
  for(byte i =A7; i>3; i++,buttonPin)           //switch pin
  for(byte i =2; i<6; i++,ledPin){              //relay pin
  int reading = digitalRead(buttonPin[i]);      //read switch
  if (buttonStates == HIGH) {                   //if switch is HIGH
    digitalWrite(ledPin[i], HIGH);              //turn on relay
  } else {
    digitalWrite(ledPin[i], LOW);               //turn off relay

  }
}
}
  for(int i =A7; i>3; i++) pinMode(i, INPUT); //swith input pin

Starting with i equal to A7, while i is greater than 3, set the mode of the ith pin, and then increment i. Just how many times will that iterate? Just which pins are you setting to INPUT mode?

To follow from @PaulS's comment ...
The second element of an incrementing FOR statement should be the upper limit.

They way you have written it there is no upper limit until the value of i reaches 32767 and rolls over to -32768 and then increments back until it reaches the value of A7.

...R

They way you have written it there is no upper limit until the value of i reaches 32767, rolls over to -32768 and then increments back until it reaches the value of A7.

Actually, as soon as it rolls over, i will be less than 3, so the loop will terminate at that point.

But, of course, that seems unlikely to be what OP wants.

PaulS:
Actually, as soon as it rolls over, i will be less than 3, so the loop will terminate at that point.

You are quite correct. I was making things too complicated. I will edit my earlier Post.

...R