From f32e696fef8a2f58a572eb4d9ff3fc7da6677061 Mon Sep 17 00:00:00 2001 From: sciencewhiz Date: Mon, 8 May 2017 21:54:03 -0700 Subject: [PATCH] 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 | 28 +++++++++++------- wpilibcIntegrationTests/src/RelayTest.cpp | 24 +++++++++++++++ .../java/edu/wpi/first/wpilibj/Relay.java | 28 +++++++++++------- .../first/wpilibj/RelayCrossConnectTest.java | 29 +++++++++++++++++++ 4 files changed, 87 insertions(+), 22 deletions(-) diff --git a/wpilibc/athena/src/Relay.cpp b/wpilibc/athena/src/Relay.cpp index 33cc8769d1..8e3a7b0f5e 100644 --- a/wpilibc/athena/src/Relay.cpp +++ b/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; + } } } diff --git a/wpilibcIntegrationTests/src/RelayTest.cpp b/wpilibcIntegrationTests/src/RelayTest.cpp index 81a7c1448d..48fe5f0e74 100644 --- a/wpilibcIntegrationTests/src/RelayTest.cpp +++ b/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); } diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Relay.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Relay.java index 4dc47cdc2b..d65b4c34c7 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Relay.java +++ b/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; + } } } } diff --git a/wpilibjIntegrationTests/src/main/java/edu/wpi/first/wpilibj/RelayCrossConnectTest.java b/wpilibjIntegrationTests/src/main/java/edu/wpi/first/wpilibj/RelayCrossConnectTest.java index 7fcf1e3871..710a3a7522 100644 --- a/wpilibjIntegrationTests/src/main/java/edu/wpi/first/wpilibj/RelayCrossConnectTest.java +++ b/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);