Fix exception when getting a relay in kReverse Direction. Fixes #458 (#525)

Add additional tests that would have caught this previously.

wpilibc/athena/src/Relay.cpp

@@ -185,25 +185,31 @@ void Relay::Set(Relay::Value value) {
 Relay::Value Relay::Get() const {
   int32_t status;
 
-  if (HAL_GetRelay(m_forwardHandle, &status)) {
+  if (m_direction == kForwardOnly) {
+    if (HAL_GetRelay(m_forwardHandle, &status)) {
+      return kOn;
+    } else {
+      return kOff;
+    }
+  } else if (m_direction == kReverseOnly) {
     if (HAL_GetRelay(m_reverseHandle, &status)) {
       return kOn;
     } else {
-      if (m_direction == kForwardOnly) {
+      return kOff;
+    }
+  } else {
+    if (HAL_GetRelay(m_forwardHandle, &status)) {
+      if (HAL_GetRelay(m_reverseHandle, &status)) {
         return kOn;
       } else {
         return kForward;
       }
-    }
-  } else {
-    if (HAL_GetRelay(m_reverseHandle, &status)) {
-      if (m_direction == kReverseOnly) {
-        return kOn;
-      } else {
-        return kReverse;
-      }
     } else {
-      return kOff;
+      if (HAL_GetRelay(m_reverseHandle, &status)) {
+        return kReverse;
+      } else {
+        return kOff;
+      }
     }
   }
 

wpilibcIntegrationTests/src/RelayTest.cpp

@@ -47,22 +47,46 @@ TEST_F(RelayTest, Relay) {
   Wait(kDelayTime);
   EXPECT_TRUE(m_forward->Get()) << "Relay did not set forward";
   EXPECT_FALSE(m_reverse->Get()) << "Relay did not set forward";
+  EXPECT_EQ(m_relay->Get(), Relay::kForward);
 
   // set the relay to reverse
   m_relay->Set(Relay::kReverse);
   Wait(kDelayTime);
   EXPECT_TRUE(m_reverse->Get()) << "Relay did not set reverse";
   EXPECT_FALSE(m_forward->Get()) << "Relay did not set reverse";
+  EXPECT_EQ(m_relay->Get(), Relay::kReverse);
 
   // set the relay to off
   m_relay->Set(Relay::kOff);
   Wait(kDelayTime);
   EXPECT_FALSE(m_forward->Get()) << "Relay did not set off";
   EXPECT_FALSE(m_reverse->Get()) << "Relay did not set off";
+  EXPECT_EQ(m_relay->Get(), Relay::kOff);
 
   // set the relay to on
   m_relay->Set(Relay::kOn);
   Wait(kDelayTime);
   EXPECT_TRUE(m_forward->Get()) << "Relay did not set on";
   EXPECT_TRUE(m_reverse->Get()) << "Relay did not set on";
+  EXPECT_EQ(m_relay->Get(), Relay::kOn);
+
+  // test forward direction
+  delete m_relay;
+  m_relay = new Relay(TestBench::kRelayChannel, Relay::kForwardOnly);
+
+  m_relay->Set(Relay::kOn);
+  Wait(kDelayTime);
+  EXPECT_TRUE(m_forward->Get()) << "Relay did not set forward";
+  EXPECT_FALSE(m_reverse->Get()) << "Relay did not set forward";
+  EXPECT_EQ(m_relay->Get(), Relay::kOn);
+
+  // test reverse direction
+  delete m_relay;
+  m_relay = new Relay(TestBench::kRelayChannel, Relay::kReverseOnly);
+
+  m_relay->Set(Relay::kOn);
+  Wait(kDelayTime);
+  EXPECT_FALSE(m_forward->Get()) << "Relay did not set reverse";
+  EXPECT_TRUE(m_reverse->Get()) << "Relay did not set reverse";
+  EXPECT_EQ(m_relay->Get(), Relay::kOn);
 }

wpilibj/src/athena/java/edu/wpi/first/wpilibj/Relay.java

@@ -235,25 +235,31 @@ public class Relay extends SensorBase implements MotorSafety, LiveWindowSendable
    * @return The current state of the relay as a Relay::Value
    */
   public Value get() {
-    if (RelayJNI.getRelay(m_forwardHandle)) {
+    if (m_direction == Direction.kForward) {
+      if (RelayJNI.getRelay(m_forwardHandle)) {
+        return Value.kOn;
+      } else {
+        return Value.kOff;
+      }
+    } else if (m_direction == Direction.kReverse) {
       if (RelayJNI.getRelay(m_reverseHandle)) {
         return Value.kOn;
       } else {
-        if (m_direction == Direction.kForward) {
+        return Value.kOff;
+      }
+    } else {
+      if (RelayJNI.getRelay(m_forwardHandle)) {
+        if (RelayJNI.getRelay(m_reverseHandle)) {
           return Value.kOn;
         } else {
           return Value.kForward;
         }
-      }
-    } else {
-      if (RelayJNI.getRelay(m_reverseHandle)) {
-        if (m_direction == Direction.kReverse) {
-          return Value.kOn;
-        } else {
-          return Value.kReverse;
-        }
       } else {
-        return Value.kOff;
+        if (RelayJNI.getRelay(m_reverseHandle)) {
+          return Value.kReverse;
+        } else {
+          return Value.kOff;
+        }
       }
     }
   }

wpilibjIntegrationTests/src/main/java/edu/wpi/first/wpilibj/RelayCrossConnectTest.java

@@ -56,6 +56,7 @@ public class RelayCrossConnectTest extends AbstractComsSetup {
         .get());
     assertTrue("Input two was not high when relay set both high", m_relayFixture.getInputTwo()
         .get());
+    assertEquals(Value.kOn, m_relayFixture.getRelay().get());
     assertEquals("On", table.getString("Value"));
   }
 
@@ -69,6 +70,7 @@ public class RelayCrossConnectTest extends AbstractComsSetup {
     assertTrue("Input two was not high when relay set Value.kForward", m_relayFixture
         .getInputTwo()
         .get());
+    assertEquals(Value.kForward, m_relayFixture.getRelay().get());
     assertEquals("Forward", table.getString("Value"));
   }
 
@@ -82,9 +84,36 @@ public class RelayCrossConnectTest extends AbstractComsSetup {
     assertFalse("Input two was not low when relay set Value.kReverse", m_relayFixture
         .getInputTwo()
         .get());
+    assertEquals(Value.kReverse, m_relayFixture.getRelay().get());
     assertEquals("Reverse", table.getString("Value"));
   }
 
+  @Test
+  public void testForwardDirection() {
+    m_relayFixture.getRelay().setDirection(Direction.kForward);
+    m_relayFixture.getRelay().set(Value.kOn);
+    m_relayFixture.getRelay().updateTable();
+    assertFalse("Input one was not low when relay set Value.kOn in kForward Direction",
+        m_relayFixture.getInputOne().get());
+    assertTrue("Input two was not high when relay set Value.kOn in kForward Direction",
+        m_relayFixture.getInputTwo().get());
+    assertEquals(Value.kOn, m_relayFixture.getRelay().get());
+    assertEquals("On", table.getString("Value"));
+  }
+
+  @Test
+  public void testReverseDirection() {
+    m_relayFixture.getRelay().setDirection(Direction.kReverse);
+    m_relayFixture.getRelay().set(Value.kOn);
+    m_relayFixture.getRelay().updateTable();
+    assertTrue("Input one was not high when relay set Value.kOn in kReverse Direction",
+        m_relayFixture.getInputOne().get());
+    assertFalse("Input two was not low when relay set Value.kOn in kReverse Direction",
+        m_relayFixture.getInputTwo().get());
+    assertEquals(Value.kOn, m_relayFixture.getRelay().get());
+    assertEquals("On", table.getString("Value"));
+  }
+
   @Test(expected = InvalidValueException.class)
   public void testSetValueForwardWithDirectionReverseThrowingException() {
     m_relayFixture.getRelay().setDirection(Direction.kForward);