Fix direction not being correctly calculated (#15)

There were several issues when calculating/detecting the direction
of movement with DirectionalOdometer. Specifically, the first problem
was that the interrupt was triggered to be invoked on pin state change
instead of a rising edge which was the initial intention.
Additionally, determining the distance should only be done within the
interrupt, since the state of the direction pin is only meaningful when
the pulse pin is on a falling or rising edge.

Author: platisd

library.properties

@@ -1,5 +1,5 @@
 name=Smartcar shield
-version=5.0.0
+version=5.0.1
 author=Dimitris Platis
 maintainer=Dimitris Platis <dimitris@plat.is>
 sentence=Arduino library for controlling the Smartcar platform

src/sensors/odometer/interrupt/DirectionalOdometer.cpp

@@ -8,14 +8,13 @@ const uint8_t kInput = 0;
 using namespace smartcarlib::constants::odometer;
 
 DirectionalOdometer::DirectionalOdometer(uint8_t directionPin,
-                                         uint8_t pinStateWhenForward,
+                                         int pinStateWhenForward,
                                          unsigned long pulsesPerMeter,
                                          Runtime& runtime)
     : DirectionlessOdometer(pulsesPerMeter, runtime)
     , mDirectionPin{ directionPin }
     , mPinStateWhenForward{ pinStateWhenForward }
     , mRuntime(runtime)
-    , mNegativePulsesCounter{ 0 }
 {
 }
 
@@ -44,10 +43,12 @@ void DirectionalOdometer::update()
     }
 
     DirectionlessOdometer::update();
-    if (mRuntime.getPinState(mDirectionPin) != mPinStateWhenForward)
+    const auto directionPinState = mRuntime.getPinState(mDirectionPin);
+    if (directionPinState != mPinStateWhenForward)
     {
         mNegativePulsesCounter++;
     }
+    mDirection = directionPinState;
 }
 
 long DirectionalOdometer::getDistance()
@@ -72,11 +73,10 @@ float DirectionalOdometer::getSpeed()
     return DirectionlessOdometer::getSpeed() * getDirection();
 }
 
-int8_t DirectionalOdometer::getDirection()
+int8_t DirectionalOdometer::getDirection() const
 {
-    return mRuntime.getPinState(mDirectionPin) == mPinStateWhenForward
-               ? smartcarlib::constants::odometer::kForward
-               : smartcarlib::constants::odometer::kBackward;
+    return mDirection == mPinStateWhenForward ? smartcarlib::constants::odometer::kForward
+                                              : smartcarlib::constants::odometer::kBackward;
 }
 
 bool DirectionalOdometer::providesDirection()

src/sensors/odometer/interrupt/DirectionalOdometer.hpp

@@ -28,12 +28,12 @@ class DirectionalOdometer : public DirectionlessOdometer
      * \endcode
      */
     DirectionalOdometer(uint8_t directionPin,
-                        uint8_t pinStateWhenForward,
+                        int pinStateWhenForward,
                         unsigned long pulsesPerMeter,
                         Runtime& runtime = arduinoRuntime);
 #else
     DirectionalOdometer(uint8_t directionPin,
-                        uint8_t pinStateWhenForward,
+                        int pinStateWhenForward,
                         unsigned long pulsesPerMeter,
                         Runtime& runtime);
 #endif
@@ -66,12 +66,12 @@ class DirectionalOdometer : public DirectionlessOdometer
      * unsigned short direction = odometer.getDirection();
      * \endcode
      */
-    int8_t getDirection();
-
+    int8_t getDirection() const;
 
 private:
     const uint8_t mDirectionPin;
-    const uint8_t mPinStateWhenForward;
+    const int mPinStateWhenForward;
     Runtime& mRuntime;
-    volatile unsigned long mNegativePulsesCounter;
+    volatile unsigned long mNegativePulsesCounter{ 0 };
+    volatile int mDirection{ smartcarlib::constants::odometer::kForward };
 };

src/sensors/odometer/interrupt/DirectionlessOdometer.cpp

@@ -5,7 +5,7 @@
 namespace
 {
 const int8_t kNotAnInterrupt         = -1;
-const uint8_t kRisingEdge            = 1;
+const uint8_t kRisingEdge            = 3;
 const uint8_t kInput                 = 0;
 const unsigned long kMinimumPulseGap = 700;
 const float kMillisecondsInSecond    = 1000.0;

test/build_and_run_ut.sh

@@ -13,7 +13,7 @@ fi
 mkdir -p build
 cd build
 cmake -DCMAKE_BUILD_TYPE=Debug ..
-make
+make -j$(nproc)
 ctest
 
 # Go back to the initial directory when you are done

test/ut/DirectionalOdometer_test.cpp

@@ -136,17 +136,21 @@ TEST_F(DirectionalOdometerAttachedTest, getDistance_WhenCalled_WillReturnCorrect
     EXPECT_EQ(mDirectionalOdometer.getDistance(), 0);
 }
 
-TEST_F(DirectionalOdometerBasicTest, getDirection_WhenPinStateForward_WillReturnForward)
+TEST_F(DirectionalOdometerAttachedTest, getDirection_WhenPinStateForward_WillReturnForward)
 {
     EXPECT_CALL(mRuntime, getPinState(kDirectionPin)).WillOnce(Return(kPinStateWhenForward));
 
+    mDirectionalOdometer.update();
+
     EXPECT_EQ(mDirectionalOdometer.getDirection(), smartcarlib::constants::odometer::kForward);
 }
 
-TEST_F(DirectionalOdometerBasicTest, getDirection_WhenPinStateBackward_WillReturnBackward)
+TEST_F(DirectionalOdometerAttachedTest, getDirection_WhenPinStateBackward_WillReturnBackward)
 {
     EXPECT_CALL(mRuntime, getPinState(kDirectionPin)).WillOnce(Return(!kPinStateWhenForward));
 
+    mDirectionalOdometer.update();
+
     EXPECT_EQ(mDirectionalOdometer.getDirection(), smartcarlib::constants::odometer::kBackward);
 }
 
@@ -178,7 +182,7 @@ TEST_F(DirectionalOdometerAttachedTest, getSpeed_WhenCalled_WillReturnCorrectSpe
     unsigned long firstPulse  = 1000;
     unsigned long secondPulse = 21000;
     EXPECT_CALL(mRuntime, getPinState(kDirectionPin))
-        .Times(3)
+        .Times(2)
         .WillRepeatedly(Return(!kPinStateWhenForward));
     {
         InSequence seq;

test/ut/DirectionlessOdometer_test.cpp

@@ -1,19 +1,19 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#include "MockRuntime.hpp"
 #include "DirectionlessOdometer.hpp"
+#include "MockRuntime.hpp"
 
 using namespace ::testing;
 using namespace smartcarlib::constants::odometer;
 
 namespace
 {
-const int8_t kNotAnInterrupt               = -1;
-const int8_t kAnInterrupt                  = 1;
-const uint8_t kRisingEdge                  = 1;
-const uint8_t kInput                       = 0;
-const unsigned long kMinimumPulseGap       = 700;
+const int8_t kNotAnInterrupt         = -1;
+const int8_t kAnInterrupt            = 1;
+const uint8_t kRisingEdge            = 3;
+const uint8_t kInput                 = 0;
+const unsigned long kMinimumPulseGap = 700;
 } // namespace
 
 class DirectionlessOdometerBasicTest : public Test
@@ -56,10 +56,14 @@ class DirectionlessOdometerAttachedTest : public DirectionlessOdometerBasicTest
 class DirectionlessOdometerBadPulsesPerMeter : public DirectionlessOdometerBasicTest
 {
 public:
-    DirectionlessOdometerBadPulsesPerMeter() : DirectionlessOdometerBasicTest(0) {}
+    DirectionlessOdometerBadPulsesPerMeter()
+        : DirectionlessOdometerBasicTest(0)
+    {
+    }
 };
 
-TEST_F(DirectionlessOdometerBadPulsesPerMeter, constructor_WhenCalledWithZeroPulsesPerMeter_WillNotDivideByZero)
+TEST_F(DirectionlessOdometerBadPulsesPerMeter,
+       constructor_WhenCalledWithZeroPulsesPerMeter_WillNotDivideByZero)
 {
     // Providing `0` as the argument to pulses per meter should not cause the constructor
     // to crash due to a division by zero
@@ -191,7 +195,8 @@ TEST_F(DirectionlessOdometerAttachedTest, update_WhenPulsesTooClose_WillIgnorePu
     EXPECT_EQ(mDirectionlessOdometer.getDistance(), initialDistance);
 }
 
-TEST_F(DirectionlessOdometerAttachedTest, update_WhenFirstPulseArrives_WillCalculateDistanceButNotSpeed)
+TEST_F(DirectionlessOdometerAttachedTest,
+       update_WhenFirstPulseArrives_WillCalculateDistanceButNotSpeed)
 {
     unsigned long firstPulse = 2000;
     EXPECT_CALL(mRuntime, currentTimeMicros()).WillOnce(Return(firstPulse));
@@ -202,7 +207,8 @@ TEST_F(DirectionlessOdometerAttachedTest, update_WhenFirstPulseArrives_WillCalcu
     EXPECT_FLOAT_EQ(mDirectionlessOdometer.getSpeed(), 0);
 }
 
-TEST_F(DirectionlessOdometerAttachedTest, update_WhenFirstPulseArrivesFast_WillCalculateDistanceButNotSpeed)
+TEST_F(DirectionlessOdometerAttachedTest,
+       update_WhenFirstPulseArrivesFast_WillCalculateDistanceButNotSpeed)
 {
     unsigned long firstPulse = kMinimumPulseGap - 1;
     EXPECT_CALL(mRuntime, currentTimeMicros()).WillOnce(Return(firstPulse));