Re-write the HallEffectAndPositionZeroingEstimator tests and make them pass.

They were just wrong.  This is a great oppertunity to split the tests
up into smaller, independent, unit tests.

Thanks Phil and Sabina for sorting out what was wrong.

Change-Id: Iddb5c47158cc57c59b09d10cf0b8a13bd7f0217f
diff --git a/frc971/zeroing/zeroing_test.cc b/frc971/zeroing/zeroing_test.cc
index 819fad0..27ae179 100644
--- a/frc971/zeroing/zeroing_test.cc
+++ b/frc971/zeroing/zeroing_test.cc
@@ -499,41 +499,60 @@
   }
 }
 
+// Test fixture for HallEffectAndPositionZeroingEstimator.
+class HallEffectAndPositionZeroingEstimatorTest : public ZeroingTest {
+ public:
+  // The starting position of the system.
+  static constexpr double kStartPosition = 2.0;
+
+  // Returns a reasonable set of test constants.
+  static constants::HallEffectZeroingConstants MakeConstants() {
+    constants::HallEffectZeroingConstants constants;
+    constants.lower_hall_position = 0.25;
+    constants.upper_hall_position = 0.75;
+    constants.index_difference = 1.0;
+    constants.hall_trigger_zeroing_length = 2;
+    constants.zeroing_move_direction = false;
+    return constants;
+  }
+
+  HallEffectAndPositionZeroingEstimatorTest()
+      : constants_(MakeConstants()), sim_(constants_.index_difference) {
+    // Start the system out at the starting position.
+    sim_.InitializeHallEffectAndPosition(kStartPosition,
+                                         constants_.lower_hall_position,
+                                         constants_.upper_hall_position);
+  }
+
+ protected:
+  // Constants, and the simulation using them.
+  const constants::HallEffectZeroingConstants constants_;
+  PositionSensorSimulator sim_;
+};
+
 // Tests that an error is detected when the starting position changes too much.
-TEST_F(ZeroingTest, TestHallEffectZeroing) {
-  constants::HallEffectZeroingConstants constants;
-  constants.lower_hall_position = 0.25;
-  constants.upper_hall_position = 0.75;
-  constants.index_difference = 1.0;
-  constants.hall_trigger_zeroing_length = 2;
-  constants.zeroing_move_direction = false;
-
-  PositionSensorSimulator sim(constants.index_difference);
-
-  const double start_pos = 1.0;
-
-  sim.InitializeHallEffectAndPosition(start_pos, constants.lower_hall_position,
-                                      constants.upper_hall_position);
-
-  HallEffectAndPositionZeroingEstimator estimator(constants);
+TEST_F(HallEffectAndPositionZeroingEstimatorTest, TestHallEffectZeroing) {
+  HallEffectAndPositionZeroingEstimator estimator(constants_);
 
   // Should not be zeroed when we stand still.
   for (int i = 0; i < 300; ++i) {
-    MoveTo(&sim, &estimator, start_pos);
+    MoveTo(&sim_, &estimator, kStartPosition);
     ASSERT_FALSE(estimator.zeroed());
   }
 
-  MoveTo(&sim, &estimator, 0.9);
+  MoveTo(&sim_, &estimator, 1.9);
   ASSERT_FALSE(estimator.zeroed());
 
   // Move to where the hall effect is triggered and make sure it becomes zeroed.
-  MoveTo(&sim, &estimator, 0.5);
+  MoveTo(&sim_, &estimator, 1.5);
   EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.5);
+  MoveTo(&sim_, &estimator, 1.5);
   ASSERT_TRUE(estimator.zeroed());
 
-  // Check that the offset is calculated correctly.
-  EXPECT_DOUBLE_EQ(-0.25, estimator.offset());
+  // Check that the offset is calculated correctly.  We should expect to read
+  // 0.5.  Since the encoder is reading -0.5 right now, the offset needs to be
+  // 1.
+  EXPECT_DOUBLE_EQ(1.0, estimator.offset());
 
   // Make sure triggering errors works.
   estimator.TriggerError();
@@ -543,44 +562,57 @@
   estimator.Reset();
   ASSERT_FALSE(estimator.zeroed());
   ASSERT_FALSE(estimator.error());
+}
 
-  // Make sure we don't become zeroed if the hall effect doesn't trigger for
-  // long enough.
-  MoveTo(&sim, &estimator, 0.9);
-  EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.5);
-  EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.9);
-  EXPECT_FALSE(estimator.zeroed());
+// Tests that we don't zero on a too short pulse.
+TEST_F(HallEffectAndPositionZeroingEstimatorTest, TestTooShortPulse) {
+  HallEffectAndPositionZeroingEstimator estimator(constants_);
 
-  // Make sure we can zero moving in the opposite direction as before and stay
-  // zeroed once the hall effect is no longer triggered.
-
-  MoveTo(&sim, &estimator, 0.0);
-  ASSERT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.4);
+  // Trigger for 1 cycle.
+  MoveTo(&sim_, &estimator, 0.9);
   EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.6);
+  MoveTo(&sim_, &estimator, 0.5);
   EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.9);
-  EXPECT_FALSE(estimator.zeroed());
-
-  // Check that the offset is calculated correctly.
-  EXPECT_DOUBLE_EQ(-0.75, estimator.offset());
-
-  // Make sure we don't zero if we start in the hall effect's range, before we
-  // reset, we also check that there were no errors.
-  MoveTo(&sim, &estimator, 0.5);
-  ASSERT_TRUE(estimator.zeroed());
-  ASSERT_FALSE(estimator.error());
-  estimator.Reset();
-  EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.5);
-  EXPECT_FALSE(estimator.zeroed());
-  MoveTo(&sim, &estimator, 0.5);
+  MoveTo(&sim_, &estimator, 0.9);
   EXPECT_FALSE(estimator.zeroed());
 }
 
+// Tests that we don't zero when we go the wrong direction.
+TEST_F(HallEffectAndPositionZeroingEstimatorTest, TestWrongDirectionNoZero) {
+  HallEffectAndPositionZeroingEstimator estimator(constants_);
+
+  // Pass through the sensor, lingering long enough that we should zero.
+  MoveTo(&sim_, &estimator, 0.0);
+  ASSERT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.4);
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.6);
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.7);
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.9);
+  EXPECT_FALSE(estimator.zeroed());
+}
+
+// Make sure we don't zero if we start in the hall effect's range.
+TEST_F(HallEffectAndPositionZeroingEstimatorTest, TestStartingOnNoZero) {
+  HallEffectAndPositionZeroingEstimator estimator(constants_);
+  MoveTo(&sim_, &estimator, 0.5);
+  estimator.Reset();
+
+  // Stay on the hall effect.  We shouldn't zero.
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.5);
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.5);
+  EXPECT_FALSE(estimator.zeroed());
+
+  // Verify moving off the hall still doesn't zero us.
+  MoveTo(&sim_, &estimator, 0.0);
+  EXPECT_FALSE(estimator.zeroed());
+  MoveTo(&sim_, &estimator, 0.0);
+  EXPECT_FALSE(estimator.zeroed());
+}
 
 }  // namespace zeroing
 }  // namespace frc971