operating 2 outputs (relays) at same time from 1 input, is my if/else wrong?

Hi guys, I am a relative beginner but not completely green. I have been messing about with different variations of this code for a while now and I cant get it to perform the actions correctly.

the background is I have a 3 axis analogue joystick, running through an Arduino Uno board, operating 7 relays

relay 1-6 operate hydraulic directional valves
relay 7 operates a hydraulic dump valve and needs to operate when ever one or more of the 1-6 relays operates.

I have relays 1-6 operating properly from the joystick, this part is fine,
however relay 7 is ONLY operating when the z axis moves in one direction - this is the last line of code - which makes me think its a coding problem and I have the ‘if - else’ function wrong

so in a nutshell; when ever pin 11, 10, 9, 8, 7, or 6 go HIGH, I need pin 4 to also go HIGH

#define joyX A0;
#define joyY A2;
#define joyZ A5;

void setup() {

  Serial.begin(9600);
  
  pinMode (11, OUTPUT); // hydraulic arm out
  pinMode (10, OUTPUT); // hydraulic arm in
  
  pinMode (9, OUTPUT); // hydraulic arm swing left
  pinMode (8, OUTPUT); // hydraulic arm swing right
  
  pinMode (7, OUTPUT); // hydraulic arm up
  pinMode (6, OUTPUT); // hydraulic arm down

  pinMode (4, OUTPUT); // hydraulic oil dump valve - must be open when any other function is in use

} 
void loop() {
 int xValue = analogRead(A0);
 int yValue = analogRead(A2);
 int zValue = analogRead(A5);
   
  if ( xValue > 490)
  {digitalWrite(10, HIGH);
  digitalWrite(4, HIGH);}
    else{digitalWrite(10, LOW);
    digitalWrite(4, LOW);}
  
  if ( xValue < 530)
  {digitalWrite(11, HIGH);
  digitalWrite(4, HIGH);}
    else{digitalWrite(11, LOW);
    digitalWrite(4, LOW);}
  
  if ( yValue > 490)
  {digitalWrite(8, HIGH);
  digitalWrite(4, HIGH);}
    else{digitalWrite(8, LOW);
    digitalWrite(4, LOW);}
  
  if ( yValue < 530)
  {digitalWrite(9, HIGH);
  digitalWrite(4, HIGH);}
    else{digitalWrite(9, LOW);
    digitalWrite(4, LOW);}
 
  if ( zValue > 490)
  {digitalWrite(6, HIGH);
  digitalWrite(4, HIGH);}
    else{digitalWrite(6, LOW);
    digitalWrite(4, LOW);}
  
  if ( zValue < 530)
  {digitalWrite(7, HIGH); 
  digitalWrite(4, HIGH);} // this is currently the only function that operates pin 4 to open the dump valve
    else{digitalWrite(7, LOW);
    digitalWrite(4, LOW);}

  delay(50);
}

thank you in advance for the help
Hedley

Hello
Welcome
++Karma; // For positing your code correctly on your first post.

So, if I have understood correctly you want output 4 to be high if any of the other outputs is high, is that coeect?

In which case:

if (digitalRead(6) || digitalRead(7) || digitalRead(8) || digitalRead(9) || digitalRead(10) || digitalRead(11)) {
  digitalWrite(4, HIGH);
} else {
  digitalWrite(4, LOW);
}

It would make your code easier to read if you gave your pins meaningful names such as:

const uint8_t armOut = 11;

Then use those names instead of the pin numbers.

Your logic is flawed. You have the dump valve being controlled by each set of if/elses.

So, if xValue is greater than 490, the dump valve is activated. Unfortunately, microseconds later, you run a similar section for xValue < 530.

So if x is 550, the first if activates the dump valve and the next one immediately turns it off.

You could use a boolean variable and set if false at the start of loop. In each if, set the boolean true if the valve is active.

At the end of loop, if any tool was active, the boolean will be true. Use that to decide whether to open or close the dump valve.

You are having problems with your ranges overlapping. You have the arm moving one way when > 490 and the other way when < 530. That means for values between 490 and 530 IT IS MOVING BOTH WAYS?!?

The two directions are conflicting over the use of the Oil Dump output. When the arm is moving in the first direction it turns on the oil dump, but since it is not moving in the OTHER direction it turns OFF the oil dump, even though the arm is moving.

Here is a re-write to fix some of those problems:

// Analog Input (AI) Pins
const byte JoyX_AIPin = A0;
const byte JoyY_AIPin = A2;
const byte JoyZ_AIPin = A5;


// Digital Output (DO) Pins
const byte OilDump_DOPin = 4;
const byte ArmDown_DOPin = 6;
const byte ArmUp_DOPin = 7;
const byte ArmRight_DOPin = 8;
const byte ArmLeft_DOPin = 9;
const byte ArmIn_DOPin = 10;
const byte ArmOut_DOPin = 11;


const int DeadZoneLow = 490;
const int DeadZoneHigh = 530;


void setup()
{
  Serial.begin(9600);


  pinMode (ArmOut_DOPin, OUTPUT); // hydraulic arm out
  pinMode (ArmIn_DOPin, OUTPUT); // hydraulic arm in


  pinMode (ArmLeft_DOPin, OUTPUT); // hydraulic arm swing left
  pinMode (ArmRight_DOPin, OUTPUT); // hydraulic arm swing right


  pinMode (ArmUp_DOPin, OUTPUT); // hydraulic arm up
  pinMode (ArmDown_DOPin, OUTPUT); // hydraulic arm down


  pinMode (OilDump_DOPin, OUTPUT); // hydraulic oil dump valve - must be open when any other function is in use
}


void loop()
{
  int xValue = analogRead(JoyX_AIPin);
  int yValue = analogRead(JoyY_AIPin);
  int zValue = analogRead(JoyZ_AIPin);


  if (xValue < DeadZoneLow)
  {
    digitalWrite(ArmIn_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else if (xValue > DeadZoneHigh)
  {
    digitalWrite(ArmOut_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else
  {
    // X Dead Zone
    digitalWrite(ArmIn_DOPin, LOW);
    digitalWrite(ArmOut_DOPin, LOW);
    digitalWrite(OilDump_DOPin, LOW);
  }


  if ( yValue < DeadZoneLow)
  {
    digitalWrite(ArmRight_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else if (yValue > DeadZoneHigh)
  {
    digitalWrite(ArmLeft_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else
  {
    // Y Dead Zone
    digitalWrite(ArmLeft_DOPin, LOW);
    digitalWrite(ArmRight_DOPin, LOW);
    digitalWrite(OilDump_DOPin, LOW);
  }


  if ( zValue < DeadZoneLow)
  {
    digitalWrite(ArmDown_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else if ( zValue > DeadZoneHigh)
  {
    digitalWrite(ArmUp_DOPin, HIGH);
    digitalWrite(OilDump_DOPin, HIGH);
  }
  else
  {
    // Z Dead Zone
    digitalWrite(ArmUp_DOPin, LOW);
    digitalWrite(ArmDown_DOPin, LOW);
    digitalWrite(OilDump_DOPin, LOW);
  }
}

thanks for your replies guys, I knew it was wrong but I just couldn't see the wood for the trees with sorting it out

PerryBebbington:
++Karma; // For positing your code correctly on your first post.

thanks - I did read the pinned "read this before posting" thread

johnwasser:
That means for values between 490 and 530 IT IS MOVING BOTH WAYS?!?

between 490 and 530 the arm should be stationary, and with the dump valve LOW (closed), the arm is essentially locked in position since the oil cannot flow out of the arm control ram/valve without the dump being opened

I will go and have another play with it and see if I can sort it.

thanks again

hedley:
Thanks - I did read the pinned "read this before posting" thread

When you've been here a while you'll discover that's a rare pleasure :slight_smile:

johnwasser:
Here is a re-write to fix some of those problems:

Been playing about with this code;
firstly, thank you for taking the time to write it,
however, it puts all relays on at the start, and switches them off when the joystick is moved, and pin 4 relay is still only operated with the z axis

At the moment I am struggling to see the way forward with it.

PerryBebbington:
So, if I have understood correctly you want output 4 to be high if any of the other outputs is high, is that coeect?

In which case:

if (digitalRead(6) || digitalRead(7) || digitalRead(8) || digitalRead(9) || digitalRead(10) || digitalRead(11)) {

digitalWrite(4, HIGH);
} else {
 digitalWrite(4, LOW);}

with this code, I understand the concept of the boolean OR logic, however none of the outputs turn on pin 4,
if I use the digitalRead for a single pin then it works, but I am not able to implement the || function?

this is how I have written it into the rest of the code, perhaps I have done it wrong?

#define joyX A0;
#define joyY A2;
#define joyZ A5;

void setup() {

  Serial.begin(9600);
  
  pinMode (11, OUTPUT); // hydraulic arm out
  pinMode (10, OUTPUT); // hydraulic arm in
  
  pinMode (9, OUTPUT); // hydraulic arm swing left
  pinMode (8, OUTPUT); // hydraulic arm swing right
  
  pinMode (7, OUTPUT); // hydraulic arm up
  pinMode (6, OUTPUT); // hydraulic arm down

  pinMode (4, OUTPUT); // hydraulic oil dump valve - must be open when any other function is in use

} 
void loop() {
 int xValue = analogRead(A0);
 int yValue = analogRead(A2);
 int zValue = analogRead(A5);
   
  if ( xValue > 490)
  {digitalWrite(10, HIGH);}
    else{digitalWrite(10, LOW);}
  
  if ( xValue < 530)
  {digitalWrite(11, HIGH);}
    else{digitalWrite(11, LOW);}
  
  if ( yValue > 490)
  {digitalWrite(8, HIGH);}
    else{digitalWrite(8, LOW);}
  
  if ( yValue < 530)
  {digitalWrite(9, HIGH);}
    else{digitalWrite(9, LOW);}
 
  if ( zValue > 490)
  {digitalWrite(6, HIGH);}
    else{digitalWrite(6, LOW);}
  
  if ( zValue < 530)
  {digitalWrite(7, HIGH);} 
    else{digitalWrite(7, LOW);}
  
  if (digitalRead(6) || digitalRead(7) || digitalRead(8) || digitalRead(9) || digitalRead(10) || digitalRead(11)) {
  digitalWrite(4, HIGH);} 
    else {
  digitalWrite(4, LOW);}

  delay(50);

thanks
Hedley

What you have looks correct.

I tried a modified version of your code on a Mega as follows:

#define joyX A0;
#define joyY A2;
#define joyZ A5;

void setup() {

  Serial.begin(9600);
 
  pinMode (11, OUTPUT); // hydraulic arm out
  pinMode (10, OUTPUT); // hydraulic arm in
 
  pinMode (9, OUTPUT); // hydraulic arm swing left
  pinMode (8, OUTPUT); // hydraulic arm swing right
 
  pinMode (7, OUTPUT); // hydraulic arm up
  pinMode (6, OUTPUT); // hydraulic arm down

  pinMode (4, OUTPUT); // hydraulic oil dump valve - must be open when any other function is in use

  pinMode(LED_BUILTIN, OUTPUT);
}

void loop() {
 int xValue = analogRead(A0);
 int yValue = analogRead(A2);
 int zValue = analogRead(A5);
  /*
  if ( xValue > 490)
  {digitalWrite(10, HIGH);}
    else{digitalWrite(10, LOW);}
 
  if ( xValue < 530)
  {digitalWrite(11, HIGH);}
    else{digitalWrite(11, LOW);}
 
  if ( yValue > 490)
  {digitalWrite(8, HIGH);}
    else{digitalWrite(8, LOW);}
 
  if ( yValue < 530)
  {digitalWrite(9, HIGH);}
    else{digitalWrite(9, LOW);}
  */
  if ( zValue > 530)
  {digitalWrite(6, HIGH);}
    else{digitalWrite(6, LOW);}
 
  if ( zValue < 490)
  {digitalWrite(7, HIGH);}
    else{digitalWrite(7, LOW);}
 
  if (digitalRead(6) || digitalRead(7) || digitalRead(8) || digitalRead(9) || digitalRead(10) || digitalRead(11)) {
  digitalWrite(LED_BUILTIN, HIGH);}
    else {
  digitalWrite(LED_BUILTIN, LOW);}

  delay(50);
}

Note:
I have commented out the if statements for pins 8, 9, 10, 11 for simplicity.
I have swapped the limits for the if statements so there is no overlap where both 6 and 7 are on at the same time.
I have changed the output to light the built in LED for testing purposes.
The code behaves as expected, which is that when either 6 or 7 is high then the built in LED lights, when neither 6 or 7 is high then the built in LED is dark.

Are your relays LOW true (energized when output pin is LOW)?

PerryBebbington:
What you have looks correct.

Note:

I have swapped the limits for the if statements so there is no overlap where both 6 and 7 are on at the same time.

:confused:

How did I miss my BIG mistake?

working well now with a few tweaks!

just need to add a push button to operate a hydraulic diverter valve and its ready to fit to the tractor mounted hedge cutter!
hedge cutting season starts tomorrow - so just in time...

thank you everyone for your inputs, it is much appreciated!

hedley:
firstly, thank you for taking the time to write it,
however, it puts all relays on at the start, and switches them off when the joystick is moved, and pin 4 relay is still only operated with the z axis

At the moment I am struggling to see the way forward with it.

Oops. Let me fix those…

// Analog Input (AI) Pins
const byte JoyX_AIPin = A0;
const byte JoyY_AIPin = A2;
const byte JoyZ_AIPin = A5;


// Digital Output (DO) Pins
const byte OilDump_DOPin = 4;
const byte ArmDown_DOPin = 6;
const byte ArmUp_DOPin = 7;
const byte ArmRight_DOPin = 8;
const byte ArmLeft_DOPin = 9;
const byte ArmIn_DOPin = 10;
const byte ArmOut_DOPin = 11;


// Other useful constants
const int DeadZoneLow = 490;
const int DeadZoneHigh = 530;


const byte RelayOn = LOW;
const byte RelayOff = HIGH;


void setup()
{
  Serial.begin(9600);


  pinMode (ArmOut_DOPin, OUTPUT); // hydraulic arm out
  digitalWrite(ArmOut_DOPin, RelayOff);
  pinMode (ArmIn_DOPin, OUTPUT); // hydraulic arm in
  digitalWrite(ArmIn_DOPin, RelayOff);


  pinMode (ArmLeft_DOPin, OUTPUT); // hydraulic arm swing left
  digitalWrite(ArmLeft_DOPin, RelayOff);
  pinMode (ArmRight_DOPin, OUTPUT); // hydraulic arm swing right
  digitalWrite(ArmRight_DOPin, RelayOff);


  pinMode (ArmUp_DOPin, OUTPUT); // hydraulic arm up
  digitalWrite(ArmUp_DOPin, RelayOff);
  pinMode (ArmDown_DOPin, OUTPUT); // hydraulic arm down
  digitalWrite(ArmDown_DOPin, RelayOff);


  pinMode (OilDump_DOPin, OUTPUT); // hydraulic oil dump valve - must be open when any other function is in use
  digitalWrite(OilDump_DOPin, RelayOff);
}


void loop()
{
  boolean moving = false;


  int xValue = analogRead(JoyX_AIPin);
  int yValue = analogRead(JoyY_AIPin);
  int zValue = analogRead(JoyZ_AIPin);


  if (xValue < DeadZoneLow)
  {
    digitalWrite(ArmIn_DOPin, RelayOn);
    moving = true;
  }
  else if (xValue > DeadZoneHigh)
  {
    digitalWrite(ArmOut_DOPin, RelayOn);
    moving = true;
  }
  else
  {
    // X Dead Zone
    digitalWrite(ArmIn_DOPin, RelayOff);
    digitalWrite(ArmOut_DOPin, RelayOff);
  }


  if ( yValue < DeadZoneLow)
  {
    digitalWrite(ArmRight_DOPin, RelayOn);
    moving = true;
  }
  else if (yValue > DeadZoneHigh)
  {
    digitalWrite(ArmLeft_DOPin, RelayOn);
    moving = true;
  }
  else
  {
    // Y Dead Zone
    digitalWrite(ArmLeft_DOPin, RelayOff);
    digitalWrite(ArmRight_DOPin, RelayOff);
  }


  if ( zValue < DeadZoneLow)
  {
    digitalWrite(ArmDown_DOPin, RelayOn);
    moving = true;
  }
  else if ( zValue > DeadZoneHigh)
  {
    digitalWrite(ArmUp_DOPin, RelayOn);
    moving = true;
  }
  else
  {
    // Z Dead Zone
    digitalWrite(ArmUp_DOPin, RelayOff);
    digitalWrite(ArmDown_DOPin, RelayOff);
  }


  digitalWrite(OilDump_DOPin, moving ? RelayOn : RelayOff);
  // Shortcut for:
  // if (moving)
  //   digitalWrite(OilDump_DOPin, RelayOn);
  // else
  //   digitalWrite(OilDump_DOPin, RelayOff);
}

How did I miss my BIG mistake?

I've had my share of staring at 'obviously' correct code, unable to see why it doesn't work as expected, given up, gone to bed then next day spotted my mistake immediately. Don't beat yourself up over it, just realise it's not just you. Make sure you learn from the mistake.