Give unit tests full control over timing

.github/workflows/linting.yml

@@ -8,13 +8,13 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
       - name: Run clang-format style check for C/C++/Protobuf programs.
-        uses: jidicula/clang-format-action@v4.11.0
+        uses: jidicula/clang-format-action@v4.18.0
         with:
-          clang-format-version: 16
+          clang-format-version: 19
           # Some files can be excluded from the clang-format style check, like 3rd party driver files,
           # as their licenses may not allow us to modify them, such as if they are LGPL licensed.
           # The exclude-regex option can be used to exclude these files from the style check:
-          exclude-regex: ^.*(PCANBasic\.h|libusb\.h|InnoMakerUsb2CanLib\.h|arduino_example\.ino)$
+          exclude-regex: ^.*(PCANBasic\.h|libusb\.h|InnoMakerUsb2CanLib\.h|arduino_example\.ino|imxrt_flexcan\.hpp|kinetis_flexcan\.hpp)$
   cmake-format:
     name: cmake-format style
     runs-on: ubuntu-latest

hardware_integration/include/isobus/hardware_integration/can_hardware_interface.hpp

@@ -69,8 +69,12 @@ namespace isobus
 		static std::shared_ptr<CANHardwarePlugin> get_assigned_can_channel_frame_handler(std::uint8_t channelIndex);
 
 		/// @brief Starts the threads for managing the CAN stack and CAN drivers
+		/// @param[in] start_thread If building with threading enabled, this parameter controls whether
+		/// the worker thread for the hardware layer is started. Typically you'll want to leave this as true.
+		/// However, if you want to drive the hardware layer yourself by calling update manually, you can
+		/// pass this in as false to not spawn the worker thread.
 		/// @returns `true` if the threads were started, otherwise false (perhaps they are already running)
-		static bool start();
+		static bool start(bool start_thread = true);
 
 		/// @brief Stops all CAN management threads and discards all remaining messages in the Tx and Rx queues.
 		/// @returns `true` if the threads were stopped, otherwise `false`

hardware_integration/include/isobus/hardware_integration/spi_hardware_plugin.hpp

@@ -29,7 +29,7 @@ namespace isobus
 		/// @brief Begin a transaction on the SPI bus. This should be called before any of the read/write operations.
 		/// @details Here the SPI bus can be acquired and prepared for a new transaction.
 		/// @note If any error occurs, end_transaction() should return false to mark a failed transaction
-		virtual void begin_transaction(){};
+		virtual void begin_transaction() {};
 
 		/// @brief Write a frame to the SPI bus. This should only be called after begin_transaction().
 		/// The result should only be read after end_transaction().

hardware_integration/src/can_hardware_interface.cpp

@@ -250,13 +250,18 @@ namespace isobus
 		return retVal;
 	}
 
-	bool CANHardwareInterface::start()
+	bool CANHardwareInterface::start(bool start_thread)
 	{
 		LOCK_GUARD(Mutex, hardwareChannelsMutex);
 
+		if (start_thread)
+		{
 #if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
-		start_threads();
+			start_threads();
+#else
+			// Ignored
 #endif
+		}
 		std::for_each(hardwareChannels.begin(), hardwareChannels.end(), [](const std::unique_ptr<CANHardware> &channel) {
 			channel->start();
 		});

hardware_integration/src/virtual_can_plugin.cpp

@@ -83,7 +83,7 @@ namespace isobus
 
 	bool VirtualCANPlugin::read_frame(isobus::CANMessageFrame &canFrame)
 	{
-		return read_frame(canFrame, 1000);
+		return read_frame(canFrame, 100);
 	}
 
 	bool VirtualCANPlugin::read_frame(isobus::CANMessageFrame &canFrame, std::uint32_t timeout) const

isobus/src/isobus_heartbeat.cpp

@@ -231,7 +231,8 @@ namespace isobus
 
 				if (managedHeartbeat == interface->trackedHeartbeats.end())
 				{
-					interface->trackedHeartbeats.emplace_back(targetControlFunction); // Heartbeat will be sent on next update
+					interface->trackedHeartbeats.emplace_back(targetControlFunction);
+					interface->trackedHeartbeats.back().timestamp_ms = 0; // Heartbeat will be sent on next update
 				}
 			}
 		}

test/CMakeLists.txt

@@ -61,6 +61,7 @@ set(TEST_SRC
     can_message_tests.cpp
     heartbeat_tests.cpp
     tc_server_tests.cpp
+    helpers/test_time_source.cpp
     helpers/control_function_helpers.cpp
     helpers/messaging_helpers.cpp)
 

test/address_claim_tests.cpp

@@ -8,21 +8,28 @@
 #include "isobus/isobus/can_network_manager.hpp"
 #include "isobus/isobus/can_partnered_control_function.hpp"
 
+#include "helpers/test_fixture.hpp"
+
 #include <chrono>
 #include <thread>
 
 using namespace isobus;
 
-TEST(ADDRESS_CLAIM_TESTS, PartneredClaim)
+class AddressClaimTest : public AgIsoStackTestFixture
+{
+	// Wrapper to give tests a more meaningful name - no content.
+};
+
+TEST_F(AddressClaimTest, AddressClaim_PartneredClaim)
 {
 	auto firstDevice = std::make_shared<VirtualCANPlugin>();
 	auto secondDevice = std::make_shared<VirtualCANPlugin>();
 	CANHardwareInterface::set_number_of_can_channels(2);
 	CANHardwareInterface::assign_can_channel_frame_handler(0, firstDevice);
 	CANHardwareInterface::assign_can_channel_frame_handler(1, secondDevice);
-	CANHardwareInterface::start();
+	CANHardwareInterface::start(false);
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(250));
+	time_source.update_for_ms(250);
 
 	NAME firstName(0);
 	firstName.set_arbitrary_address_capable(true);
@@ -53,7 +60,7 @@ TEST(ADDRESS_CLAIM_TESTS, PartneredClaim)
 	const isobus::NAMEFilter filterFirst(NAME::NAMEParameters::FunctionCode, static_cast<std::uint8_t>(NAME::Function::CabClimateControl));
 	auto secondPartneredFirstEcu = CANNetworkManager::CANNetwork.create_partnered_control_function(1, { filterFirst });
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(500));
+	time_source.update_for_ms(500);
 	EXPECT_TRUE(firstInternalECU->get_address_valid());
 	EXPECT_TRUE(secondInternalECU2->get_address_valid());
 	EXPECT_TRUE(firstPartneredSecondECU->get_address_valid());
@@ -69,16 +76,16 @@ TEST(ADDRESS_CLAIM_TESTS, PartneredClaim)
 	CANNetworkManager::CANNetwork.deactivate_control_function(secondInternalECU2);
 }
 
-TEST(ADDRESS_CLAIM_TESTS, CannotClaim)
+TEST_F(AddressClaimTest, CannotClaim)
 {
 	VirtualCANPlugin plugin;
 	plugin.open();
 
 	CANHardwareInterface::set_number_of_can_channels(1);
 	CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared<VirtualCANPlugin>());
-	CANHardwareInterface::start();
+	CANHardwareInterface::start(false);
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(250));
+	time_source.update_for_ms(250);
 
 	// Claim a very low name on every address
 	NAME firstName(0);
@@ -138,7 +145,7 @@ TEST(ADDRESS_CLAIM_TESTS, CannotClaim)
 
 	auto secondInternalECU2 = CANNetworkManager::CANNetwork.create_internal_control_function(secondName, 0);
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(1500));
+	time_source.update_for_ms(1500);
 
 	bool cannot_claim_message_seen = false;
 	while (!plugin.get_queue_empty())

test/cf_functionalities_tests.cpp

@@ -8,6 +8,7 @@
 
 #include "helpers/control_function_helpers.hpp"
 #include "helpers/messaging_helpers.hpp"
+#include "helpers/test_fixture.hpp"
 
 using namespace isobus;
 
@@ -25,21 +26,26 @@ class TestControlFunctionFunctionalities : public ControlFunctionFunctionalities
 	}
 };
 
-TEST(CONTROL_FUNCTION_FUNCTIONALITIES_TESTS, CFFunctionalitiesTest)
+class ControlFunctionFunctionalitiesTest : public AgIsoStackTestFixture
+{
+	// Wrapper to give tests a more meaningful name - no content.
+};
+
+TEST_F(ControlFunctionFunctionalitiesTest, CFFunctionalitiesTest)
 {
 	VirtualCANPlugin requesterPlugin;
 	requesterPlugin.open();
 
 	CANHardwareInterface::set_number_of_can_channels(1);
 	CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared<VirtualCANPlugin>());
-	CANHardwareInterface::start();
+	CANHardwareInterface::start(false);
 
-	auto internalECU = test_helpers::claim_internal_control_function(0x01, 0);
+	auto internalECU = test_helpers::claim_internal_control_function(0x01, 0, time_source);
 	auto otherECU = test_helpers::force_claim_partnered_control_function(0x12, 0);
 
 	TestControlFunctionFunctionalities cfFunctionalitiesUnderTest(internalECU);
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(50));
+	time_source.update_for_ms(50);
 
 	EXPECT_EQ(true, cfFunctionalitiesUnderTest.get_functionality_is_supported(ControlFunctionFunctionalities::Functionalities::MinimumControlFunction));
 	EXPECT_EQ(false, cfFunctionalitiesUnderTest.get_functionality_is_supported(ControlFunctionFunctionalities::Functionalities::UniversalTerminalServer));
@@ -516,6 +522,7 @@ TEST(CONTROL_FUNCTION_FUNCTIONALITIES_TESTS, CFFunctionalitiesTest)
 	CANNetworkManager::CANNetwork.update();
 
 	cfFunctionalitiesUnderTest.update(); // Updating manually since we're not integrated with the diagnostic protocol inside this test
+	time_source.update_for_ms(5);
 
 	ASSERT_TRUE(requesterPlugin.read_frame(testFrame));
 	ASSERT_TRUE(testFrame.isExtendedFrame);

test/core_network_management_tests.cpp

@@ -13,6 +13,7 @@
 
 #include "helpers/control_function_helpers.hpp"
 #include "helpers/messaging_helpers.hpp"
+#include "helpers/test_fixture.hpp"
 
 using namespace isobus;
 
@@ -26,7 +27,12 @@ void test_control_function_state_callback(std::shared_ptr<ControlFunction> contr
 	wasTestStateCallbackHit = true;
 }
 
-TEST(CORE_TESTS, TestCreateAndDestroyPartners)
+class CoreTest : public AgIsoStackTestFixture
+{
+	// Wrapper to give tests a more meaningful name - no content.
+};
+
+TEST_F(CoreTest, TestCreateAndDestroyPartners)
 {
 	std::vector<isobus::NAMEFilter> vtNameFilters;
 	const isobus::NAMEFilter testFilter(isobus::NAME::NAMEParameters::FunctionCode, static_cast<std::uint8_t>(isobus::NAME::Function::VirtualTerminal));
@@ -40,7 +46,7 @@ TEST(CORE_TESTS, TestCreateAndDestroyPartners)
 	CANNetworkManager::CANNetwork.deactivate_control_function(TestPartner3);
 }
 
-TEST(CORE_TESTS, TestCreateAndDestroyICFs)
+TEST_F(CoreTest, TestCreateAndDestroyICFs)
 {
 	isobus::NAME TestDeviceNAME(0);
 	TestDeviceNAME.set_arbitrary_address_capable(true);
@@ -66,7 +72,7 @@ TEST(CORE_TESTS, TestCreateAndDestroyICFs)
 	CANNetworkManager::CANNetwork.deactivate_control_function(testICF3);
 }
 
-TEST(CORE_TESTS, BusloadTest)
+TEST_F(CoreTest, BusloadTest)
 {
 	EXPECT_EQ(0.0f, CANNetworkManager::CANNetwork.get_estimated_busload(200)); // Invalid channel should return zero load
 
@@ -90,7 +96,7 @@ TEST(CORE_TESTS, BusloadTest)
 	{
 		CANNetworkManager::CANNetwork.process_receive_can_message_frame(testFrame); // Send a bunch of junk messages
 	}
-	std::this_thread::sleep_for(std::chrono::milliseconds(101));
+	time_source.simulate_delay_ms(101);
 	CANNetworkManager::CANNetwork.update();
 
 	// Bus load should be non zero, and less than 100%
@@ -102,13 +108,13 @@ TEST(CORE_TESTS, BusloadTest)
 #endif
 }
 
-TEST(CORE_TESTS, CommandedAddress)
+TEST_F(CoreTest, CommandedAddress)
 {
 	CANHardwareInterface::set_number_of_can_channels(1);
 	CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared<VirtualCANPlugin>());
-	CANHardwareInterface::start();
+	CANHardwareInterface::start(false);
 
-	auto internalECU = test_helpers::claim_internal_control_function(0x43, 0);
+	auto internalECU = test_helpers::claim_internal_control_function(0x43, 0, time_source);
 	auto externalECU = test_helpers::force_claim_partnered_control_function(0xF8, 0);
 
 	// Let's construct a short BAM session for commanded address
@@ -165,19 +171,19 @@ TEST(CORE_TESTS, CommandedAddress)
 	  }));
 	CANNetworkManager::CANNetwork.update();
 
-	std::this_thread::sleep_for(std::chrono::milliseconds(500));
+	time_source.update_for_ms(500);
 	EXPECT_EQ(0x04, internalECU->get_address());
 
 	CANNetworkManager::CANNetwork.deactivate_control_function(internalECU);
 	CANNetworkManager::CANNetwork.deactivate_control_function(externalECU);
 	CANHardwareInterface::stop();
 }
 
-TEST(CORE_TESTS, InvalidatingControlFunctions)
+TEST_F(CoreTest, InvalidatingControlFunctions)
 {
 	CANHardwareInterface::set_number_of_can_channels(1);
 	CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared<VirtualCANPlugin>());
-	CANHardwareInterface::start();
+	CANHardwareInterface::start(false);
 
 	// Request the address claim PGN to simulate a control function starting to claim an address
 	CANNetworkManager::CANNetwork.process_receive_can_message_frame(test_helpers::create_message_frame_pgn_request(
@@ -187,7 +193,7 @@ TEST(CORE_TESTS, InvalidatingControlFunctions)
 	CANNetworkManager::CANNetwork.update();
 
 	// Simulate waiting for some contention
-	std::this_thread::sleep_for(std::chrono::milliseconds(15));
+	time_source.update_for_ms(15);
 	CANNetworkManager::CANNetwork.update();
 
 	CANNetworkManager::CANNetwork.add_control_function_status_change_callback(test_control_function_state_callback);
@@ -210,7 +216,7 @@ TEST(CORE_TESTS, InvalidatingControlFunctions)
 	CANNetworkManager::CANNetwork.update();
 
 	// Now, if we wait a while, that partner didn't claim again, so it should be invalid.
-	std::this_thread::sleep_for(std::chrono::seconds(2));
+	time_source.update_for_ms(2000);
 	CANNetworkManager::CANNetwork.update();
 
 	EXPECT_FALSE(testPartner->get_address_valid());
@@ -224,7 +230,7 @@ TEST(CORE_TESTS, InvalidatingControlFunctions)
 	CANHardwareInterface::stop();
 }
 
-TEST(CORE_TESTS, NewExternalControlFunctionTriggersStateCallback)
+TEST_F(CoreTest, NewExternalControlFunctionTriggersStateCallback)
 {
 	wasTestStateCallbackHit = false;
 	testControlFunction.reset();
@@ -260,7 +266,7 @@ TEST(CORE_TESTS, NewExternalControlFunctionTriggersStateCallback)
 	wasTestStateCallbackHit = false;
 }
 
-TEST(CORE_TESTS, ControlFunctionAddressChangeTriggersStateCallback)
+TEST_F(CoreTest, ControlFunctionAddressChangeTriggersStateCallback)
 {
 	wasTestStateCallbackHit = false;
 	testControlFunction.reset();
@@ -301,7 +307,7 @@ TEST(CORE_TESTS, ControlFunctionAddressChangeTriggersStateCallback)
 	wasTestStateCallbackHit = false;
 }
 
-TEST(CORE_TESTS, PartnerCreatedAfterExternalClaimHasValidAddress)
+TEST_F(CoreTest, PartnerCreatedAfterExternalClaimHasValidAddress)
 {
 	constexpr std::uint8_t TEST_CHANNEL = 3;
 	constexpr std::uint8_t CLAIMED_ADDRESS = 0x96;
@@ -344,7 +350,7 @@ TEST(CORE_TESTS, PartnerCreatedAfterExternalClaimHasValidAddress)
 	isobus::CANNetworkManager::CANNetwork.deactivate_control_function(partner);
 }
 
-TEST(CORE_TESTS, SimilarControlFunctions)
+TEST_F(CoreTest, SimilarControlFunctions)
 {
 	CANNetworkManager::CANNetwork.update();