diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f824fbd..3bc77e3 100644 --- a/.github/workflows/tests.yml +++ b/.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 diff --git a/library.properties b/library.properties index d270b75..acd2e0d 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=DCCEXProtocol -version=1.0.0 +version=1.0.1 author=Peter Cole, Peter Akers maintainer=Peter Cole, Peter Akers sentence=DCC-EX Native Protocol implementation diff --git a/src/DCCEXProtocol.cpp b/src/DCCEXProtocol.cpp index 31c9eb9..ee56f18 100644 --- a/src/DCCEXProtocol.cpp +++ b/src/DCCEXProtocol.cpp @@ -797,25 +797,21 @@ void DCCEXProtocol::_processLocoBroadcast() { // 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) { diff --git a/src/DCCEXProtocol.h b/src/DCCEXProtocol.h index 4fbdbdd..91f439d 100644 --- a/src/DCCEXProtocol.h +++ b/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) {} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ff1e669..9ed2dc1 100644 --- a/tests/CMakeLists.txt +++ b/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) diff --git a/tests/loco/test_Loco.cpp b/tests/loco/test_Loco.cpp index f1c8714..c57f896 100644 --- a/tests/loco/test_Loco.cpp +++ b/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); diff --git a/tests/loco/test_LocoUpdate.cpp b/tests/loco/test_LocoUpdate.cpp new file mode 100644 index 0000000..9d12cbb --- /dev/null +++ b/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 << ""; + + // 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 << ""; + + // 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 << ""; + + // 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 << ""; + + // 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(); +} diff --git a/tests/setup/DCCEXProtocolDelegateMock.hpp b/tests/setup/DCCEXProtocolDelegateMock.hpp index d3634e8..d170b78 100644 --- a/tests/setup/DCCEXProtocolDelegateMock.hpp +++ b/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));