Merge pull request #27 from DCC-EX:add-delegate-loco-broadcast

Add loco broadcast with tests

.github/workflows/tests.yml

@@ -13,13 +13,9 @@ jobs:
       - uses: actions/checkout@v4.1.1
         with:
           fetch-depth: 0
-      - name: Install valgrind
-        run: sudo apt install -y valgrind
       - name: Prepare build directory
         run: cmake -Bbuild
       - name: Compile tests
         run: cmake --build build --parallel --target DCCEXProtocolTests
       - name: Run tests
         run: ./build/tests/DCCEXProtocolTests --gtest_shuffle --gtest_repeat=5 --gtest_recreate_environments_when_repeating
-      - name: Run valgrind
-        run: valgrind --leak-check=full --track-origins=yes build/tests/DCCEXProtocolTests

library.properties

@@ -1,5 +1,5 @@
 name=DCCEXProtocol
-version=1.0.0
+version=1.0.1
 author=Peter Cole, Peter Akers <akersp62@gmail.com>
 maintainer=Peter Cole, Peter Akers <akersp62@gmail.com>
 sentence=DCC-EX Native Protocol implementation

src/DCCEXProtocol.cpp

@@ -797,25 +797,21 @@ void DCCEXProtocol::_processLocoBroadcast() { //<l cab reg speedByte functMap>
   int address = DCCEXInbound::getNumber(0);
   int speedByte = DCCEXInbound::getNumber(2);
   int functMap = _getValidFunctionMap(DCCEXInbound::getNumber(3));
-  // Loco* loco=Loco::getByAddress(address);
-  // if (!loco) return;
-  // int speed=_getSpeedFromSpeedByte(speedByte);
-  // Direction dir=_getDirectionFromSpeedByte(speedByte);
-  // loco->setSpeed(speed);
-  // loco->setDirection(dir);
-  // loco->setFunctionStates(functMap);
-  // _delegate->receivedLocoUpdate(loco);
+  int speed = _getSpeedFromSpeedByte(speedByte);
+  Direction dir = _getDirectionFromSpeedByte(speedByte);
 
+  // Set a known Loco with the received info and call the delegate
   for (Loco *l = Loco::getFirst(); l; l = l->getNext()) {
     if (l->getAddress() == address) {
-      int speed = _getSpeedFromSpeedByte(speedByte);
-      Direction dir = _getDirectionFromSpeedByte(speedByte);
       l->setSpeed(speed);
       l->setDirection(dir);
       l->setFunctionStates(functMap);
       _delegate->receivedLocoUpdate(l);
     }
   }
+
+  // Send a broadcast for unknown as well in case it's a local Loco not in the roster
+  _delegate->receivedLocoBroadcast(address, speed, dir, functMap);
 }
 
 int DCCEXProtocol::_getValidFunctionMap(int functionMap) {

src/DCCEXProtocol.h

@@ -34,6 +34,7 @@
 /*
 Version information:
 
+1.0.1   - Add additional receivedLocoBroadcast() delegate method to cater for non-roster updates
 1.0.0   - First Production release
         - Add methods to clear and refresh the various lists
         - Various memory leak bugfixes
@@ -165,6 +166,13 @@ class DCCEXProtocolDelegate {
   /// @param loco Pointer to the loco object
   virtual void receivedLocoUpdate(Loco *loco) {}
 
+  /// @brief Notify when a Loco broadcast has been received - suitable for non-roster locos
+  /// @param address DCC address of the loco
+  /// @param speed Speed as derived from the speed byte
+  /// @param direction Direction as derived from the speed byte
+  /// @param functionMap Function map
+  virtual void receivedLocoBroadcast(int address, int speed, Direction direction, int functionMap) {}
+
   /// @brief Notify when the global track power state change is received
   /// @param state Power state received (PowerOff|PowerOn|PowerUnknown)
   virtual void receivedTrackPower(TrackPower state) {}

tests/CMakeLists.txt

@@ -12,8 +12,7 @@ target_sources(
 target_include_directories(DCCEXProtocolTests
                            PUBLIC ${arduinocore-api_SOURCE_DIR}/test/include)
 
-# sanitize(address,undefined)
-sanitize(undefined)
+sanitize(address,undefined)
 
 target_compile_options(DCCEXProtocolTests PUBLIC -std=c++2a -Wall -Wextra
                                                  -Wconversion -Wsign-conversion)

tests/loco/test_Loco.cpp

@@ -28,7 +28,7 @@
 
 #include "../setup/LocoTests.h"
 
-/// @brief Create a single Loco using the legacy constructor
+/// @brief Create a single Loco
 TEST_F(LocoTests, createSingleLoco) {
   // Create an individual loco
   Loco *loco1 = new Loco(1, LocoSource::LocoSourceEntry);
@@ -67,7 +67,7 @@ TEST_F(LocoTests, createSingleLoco) {
   delete loco1;
 }
 
-/// @brief Create a roster of Locos using the legacy constructor
+/// @brief Create a roster of Locos
 TEST_F(LocoTests, createRoster) {
   // Roster should start empty, don't continue if it isn't
   ASSERT_EQ(_dccexProtocol.roster->getFirst(), nullptr);

tests/loco/test_LocoUpdate.cpp

@@ -0,0 +1,104 @@
+/* -*- c++ -*-
+ *
+ * DCCEXProtocol
+ *
+ * This package implements a DCCEX native protocol connection,
+ * allow a device to communicate with a DCC-EX EX-CommandStation.
+ *
+ * Copyright © 2024 Vincent Hamp
+ * Copyright © 2024 Peter Cole
+ *
+ * This work is licensed under the Creative Commons Attribution-ShareAlike
+ * 4.0 International License. To view a copy of this license, visit
+ * http://creativecommons.org/licenses/by-sa/4.0/ or send a letter to
+ * Creative Commons, PO Box 1866, Mountain View, CA 94042, USA.
+ *
+ * Attribution — You must give appropriate credit, provide a link to the
+ * license, and indicate if changes were made. You may do so in any
+ * reasonable manner, but not in any way that suggests the licensor
+ * endorses you or your use.
+ *
+ * ShareAlike — If you remix, transform, or build upon the material, you
+ * must distribute your contributions under the same license as the
+ * original.
+ *
+ * All other rights reserved.
+ *
+ */
+
+#include "../setup/LocoTests.h"
+
+/// @brief Create a small roster and check updates are received
+TEST_F(LocoTests, receiveRosterLocoUpdate) {
+  // Setup a small roster, make sure it's created correctly
+  ASSERT_EQ(_dccexProtocol.roster->getFirst(), nullptr);
+
+  // Add two locos
+  Loco *loco42 = new Loco(42, LocoSource::LocoSourceRoster);
+  loco42->setName("Loco42");
+  Loco *loco120 = new Loco(120, LocoSource::LocoSourceRoster);
+  loco120->setName("Loco120");
+
+  // Now verify the roster, fatal error if first is nullptr
+  Loco *firstLoco = _dccexProtocol.roster->getFirst();
+  ASSERT_NE(firstLoco, nullptr);
+
+  // Set a loco update for 42 in the stream:
+  // - Speed byte = forward, speed 21
+  // - Function 0 on
+  _stream << "<l 42 0 150 1>";
+
+  // Expect to receive the delegate call at the next check()
+  EXPECT_CALL(_delegate, receivedLocoUpdate(loco42)).Times(Exactly(1));
+  // We should also expect an update with the details
+  EXPECT_CALL(_delegate, receivedLocoBroadcast(42, 21, Direction::Forward, 1)).Times(Exactly(1));
+  _dccexProtocol.check();
+
+  // Validate expected result
+  EXPECT_EQ(loco42->getSpeed(), 21);
+  EXPECT_EQ(loco42->getDirection(), Direction::Forward);
+  EXPECT_EQ(loco42->getFunctionStates(), 1);
+
+  // Set a loco update for 120 in the stream:
+  // - Speed byte = reverse, speed 11
+  // - Functions 0 and 1 on
+  _stream << "<l 120 0 12 2>";
+
+  // Expect to receive the delegate call at the next check()
+  EXPECT_CALL(_delegate, receivedLocoUpdate(loco120)).Times(Exactly(1));
+  // We should also expect an update with the details
+  EXPECT_CALL(_delegate, receivedLocoBroadcast(120, 11, Direction::Reverse, 2)).Times(Exactly(1));
+  _dccexProtocol.check();
+
+  // Validate expected result
+  EXPECT_EQ(loco120->getSpeed(), 11);
+  EXPECT_EQ(loco120->getDirection(), Direction::Reverse);
+  EXPECT_EQ(loco120->getFunctionStates(), 2);
+}
+
+/// @brief Check updates are received for non-roster locos
+TEST_F(LocoTests, receiveNonRosterLocoUpdate) {
+  // Set a loco update for an unknown loco in the stream:
+  // - Address 355
+  // - Speed byte = forward, speed 31
+  // - Functions off
+  _stream << "<l 355 0 160 0>";
+
+  // Expect to receive the delegate call at the next check()
+  EXPECT_CALL(_delegate, receivedLocoBroadcast(355, 31, Direction::Forward, 0)).Times(Exactly(1));
+  // We should not receive a Loco object delegate call
+  EXPECT_CALL(_delegate, receivedLocoUpdate(::testing::_)).Times(0);
+  _dccexProtocol.check();
+
+  // Set a loco update for an unknown loco in the stream:
+  // - Address 42
+  // - Speed byte = reverse, speed 11
+  // - Functions 0 and 1 on
+  _stream << "<l 42 0 12 2>";
+
+  // Expect to receive the delegate call at the next check()
+  EXPECT_CALL(_delegate, receivedLocoBroadcast(42, 11, Direction::Reverse, 2)).Times(Exactly(1));
+  // We should not receive a Loco object delegate call
+  EXPECT_CALL(_delegate, receivedLocoUpdate(::testing::_)).Times(0);
+  _dccexProtocol.check();
+}

tests/setup/DCCEXProtocolDelegateMock.hpp

@@ -24,6 +24,9 @@ class DCCEXProtocolDelegateMock : public DCCEXProtocolDelegate {
   // Notify when an update to a Loco object is received
   MOCK_METHOD(void, receivedLocoUpdate, (Loco *), (override));
 
+  // Notify when a Loco broadcast is received
+  MOCK_METHOD(void, receivedLocoBroadcast, (int address, int speed, Direction direction, int functionMap), (override));
+
   // Notify when a track power state change is received
   MOCK_METHOD(void, receivedTrackPower, (TrackPower), (override));