Fixes rounding problem in DCC speed step representation. (#543)

Adds tests for exercising all 126 speed steps.

Fixes #356.

src/dcc/Loco.hxx

@@ -134,10 +134,15 @@ public:
         if (f_speed > 0)
         {
             f_speed *= ((p.get_speed_steps() * 1.0) / 126);
-            unsigned sp = f_speed;
-            sp++; // makes sure it is at least speed step 1.
+            unsigned sp = f_speed + 0.5; // round
+            if (sp == 0) // ceiling between speed step 0 and 1
+            {
+                sp = 1;
+            }
             if (sp > p.get_speed_steps())
+            {
                 sp = p.get_speed_steps();
+            }
             LOG(VERBOSE, "set speed to step %u", sp);
             p.speed_ = sp;
         }

src/dcc/Packet.cxxtest

@@ -725,4 +725,102 @@ TEST_F(Train28Test, isSmall)
     // bits would fit into the cracks.
 }
 
+class Train128Test : public PacketTest
+{
+protected:
+    Train128Test()
+        : code_(0)
+        , regExpect_(&loop_, &train_)
+        , train_(DccShortAddress(55))
+    {
+    }
+
+    ~Train128Test()
+    {
+        EXPECT_CALL(loop_, unregister_source(&train_));
+    }
+
+    /// Requests a refresh packet from the train.
+    void do_refresh()
+    {
+        new (&pkt_) Packet();
+        train_.get_next_packet(0, &pkt_);
+    }
+
+    /// Requests the packet from the train that the last code was generated
+    /// for.
+    void do_callback()
+    {
+        Mock::VerifyAndClear(&loop_);
+        new (&pkt_) Packet();
+        train_.get_next_packet(code_, &pkt_);
+        code_ = 0;
+    }
+
+    unsigned code_;
+    StrictMock<MockUpdateLoop> loop_;
+    // We use a constructor trick here to expect the registration expectation
+    // at the right time.
+    struct AddRegisterExpect
+    {
+        AddRegisterExpect(StrictMock<MockUpdateLoop> *l, Dcc128Train *t)
+        {
+            EXPECT_CALL(*l, register_source(t));
+        }
+    } regExpect_;
+    Dcc128Train train_;
+};
+
+/// Tests that all reverse speed steps are correctly generated to the track.
+TEST_F(Train128Test, SetSpeedReverse)
+{
+    for (int mph = 1; mph <= 126; mph++)
+    {
+        string ss = StringPrintf("mph=%d", mph);
+        SCOPED_TRACE(ss);
+        EXPECT_CALL(loop_, send_update(&train_, _))
+            .WillOnce(SaveArg<1>(&code_));
+        SpeedType s;
+        s.set_mph(mph);
+        s.reverse();
+        auto w = s.get_wire();
+        s.set_wire(w);
+        train_.set_speed(s);
+        do_callback();
+        EXPECT_THAT(get_packet(), ElementsAre(55, 0b00111111, mph + 1, _));
+    }
+}
+
+/// Tests that all forward speed steps are correctly generated to the track.
+TEST_F(Train128Test, SetSpeedForward)
+{
+    for (int mph = 1; mph <= 126; mph++)
+    {
+        string ss = StringPrintf("mph=%d", mph);
+        SCOPED_TRACE(ss);
+        EXPECT_CALL(loop_, send_update(&train_, _))
+            .WillOnce(SaveArg<1>(&code_));
+        SpeedType s;
+        s.set_mph(mph);
+        auto w = s.get_wire();
+        s.set_wire(w);
+        train_.set_speed(s);
+        do_callback();
+        EXPECT_THAT(
+            get_packet(), ElementsAre(55, 0b00111111, 0x80 | (mph + 1), _));
+    }
+}
+
+TEST_F(Train128Test, MaxSpeed)
+{
+    EXPECT_CALL(loop_, send_update(&train_, _)).WillOnce(SaveArg<1>(&code_));
+    SpeedType s;
+    s.set_mph(128);
+    s.reverse();
+    train_.set_speed(s);
+    do_callback();
+    EXPECT_THAT(get_packet(), ElementsAre(55, 0b00111111, 0x7F, _));
+}
+
+
 } // namespace dcc

src/openlcb/Velocity.cxx

@@ -48,7 +48,7 @@ namespace openlcb
 uint8_t Velocity::get_dcc_128()
 {
     uint8_t result;
-    uint32_t tmp = (speed() * MPH_FACTOR) + 0.5;
+    uint32_t tmp = mph() + 0.5;
 
     if (tmp == 0)
     {

src/openlcb/Velocity.cxxtest

@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 #include "openlcb/Velocity.hxx"
 #include "openlcb/TractionDefs.hxx"
+#include "utils/StringPrintf.hxx"
 
 using namespace openlcb;
 
@@ -247,18 +248,46 @@ TEST(NMRAnetVelocityTest, mph_negative)
 
 TEST(NMRAnetVelocityTest, get_dcc_128_forward)
 {
-    Velocity *velocity = new Velocity(100.5F);
+    Velocity *velocity = new Velocity(20.1168); // 45 mph
 
     EXPECT_EQ(velocity->direction(), Velocity::FORWARD);    
-    EXPECT_TRUE(velocity->get_dcc_128() == (46 | 0x80));
+    EXPECT_EQ((46 | 0x80), velocity->get_dcc_128());
 }
 
 TEST(NMRAnetVelocityTest, get_dcc_128_reverse)
 {
-    Velocity *velocity = new Velocity(-100.5F);
+    Velocity *velocity = new Velocity(-20.1168); // -45 mph
 
     EXPECT_EQ(velocity->direction(), Velocity::REVERSE);    
-    EXPECT_TRUE(velocity->get_dcc_128() == 46);
+    EXPECT_EQ(46, velocity->get_dcc_128());
+}
+
+/// Tests all 126 speed steps in DCC.
+TEST(NMRAnetVelocityTest, dcc_128_round)
+{
+    Velocity v;
+    for (int mph = 1; mph <= 126; mph++)
+    {
+        string ss = StringPrintf("mph=%d", mph);
+        SCOPED_TRACE(ss);
+        v.set_mph(mph);
+        auto wire = v.get_wire();
+        Velocity vv;
+        vv.set_wire(wire);
+        EXPECT_EQ(0x80 | (mph + 1), vv.get_dcc_128());
+    }
+
+    for (int mph = 1; mph <= 126; mph++)
+    {
+        string ss = StringPrintf("mph=-%d", mph);
+        SCOPED_TRACE(ss);
+        v.set_mph(mph);
+        v.reverse();
+        auto wire = v.get_wire();
+        Velocity vv;
+        vv.set_wire(wire);
+        EXPECT_EQ((mph + 1), vv.get_dcc_128());
+    }
 }
 
 TEST(NMRAnetVelocityTest, get_dcc_128_zero)