feat(core): Make heartbeat interface use the new messaging handler

Author: GwnDaan

isobus/include/isobus/isobus/can_message_handling.hpp

@@ -108,7 +108,7 @@ namespace isobus
 		                      DataChunkCallback frameChunkCallback = nullptr) const;
 
 		friend class CANMessageHandler; ///< Allow the CANMessageHandler to modify the messaging provider
-		std::shared_ptr<CANMessagingProvider> messagingProvider; ///< The messaging provider to use for sending messages
+		std::weak_ptr<CANMessagingProvider> messagingProvider; ///< The messaging provider to use for sending messages
 	};
 
 	/// @brief A class for managing the routing of incoming and outgoing CAN messages

isobus/include/isobus/isobus/can_network_manager.hpp

@@ -395,7 +395,7 @@ namespace isobus
 		std::array<std::unique_ptr<TransportProtocolManager>, CAN_PORT_MAXIMUM> transportProtocols; ///< One instance of the transport protocol manager for each channel
 		std::array<std::unique_ptr<ExtendedTransportProtocolManager>, CAN_PORT_MAXIMUM> extendedTransportProtocols; ///< One instance of the extended transport protocol manager for each channel
 		std::array<std::unique_ptr<FastPacketProtocol>, CAN_PORT_MAXIMUM> fastPacketProtocol; ///< One instance of the fast packet protocol for each channel
-		std::array<std::unique_ptr<HeartbeatInterface>, CAN_PORT_MAXIMUM> heartBeatInterfaces; ///< Manages ISOBUS heartbeat requests, one per channel
+		std::array<std::shared_ptr<HeartbeatInterface>, CAN_PORT_MAXIMUM> heartBeatInterfaces; ///< Manages ISOBUS heartbeat requests, one per channel
 
 		std::array<std::deque<std::uint32_t>, CAN_PORT_MAXIMUM> busloadMessageBitsHistory; ///< Stores the approximate number of bits processed on each channel over multiple previous time windows
 		std::array<std::uint32_t, CAN_PORT_MAXIMUM> currentBusloadBitAccumulator; ///< Accumulates the approximate number of bits processed on each channel during the current time window
@@ -413,6 +413,7 @@ namespace isobus
 		std::vector<ParameterGroupNumberCallbackData> globalParameterGroupNumberCallbacks; ///< A list of all global PGN callbacks
 		std::vector<ParameterGroupNumberCallbackData> anyControlFunctionParameterGroupNumberCallbacks; ///< A list of all global PGN callbacks
 		CANMessageHandler messageHandler; ///< The message handler driven by the network manager
+		std::shared_ptr<CANMessagingProvider> messagingProvider; ///< The messaging provider for the network manager, currently set to itself
 		EventDispatcher<CANMessage> messageTransmittedEventDispatcher; ///< An event dispatcher for notifying consumers about transmitted messages by our application
 		EventDispatcher<std::shared_ptr<InternalControlFunction>> addressViolationEventDispatcher; ///< An event dispatcher for notifying consumers about address violations
 		Mutex receivedMessageQueueMutex; ///< A mutex for receive messages thread safety

isobus/include/isobus/isobus/isobus_heartbeat.hpp

@@ -20,14 +20,15 @@
 #include "isobus/isobus/can_callbacks.hpp"
 #include "isobus/isobus/can_internal_control_function.hpp"
 #include "isobus/isobus/can_message.hpp"
+#include "isobus/isobus/can_message_handling.hpp"
 #include "isobus/utility/event_dispatcher.hpp"
 
 #include <list>
 
 namespace isobus
 {
 	/// @brief This class is used to send and receive ISOBUS heartbeats.
-	class HeartbeatInterface
+	class HeartbeatInterface : public CANMessagingConsumer
 	{
 	public:
 		/// @brief This enum is used to define the possible errors that can occur when receiving a heartbeat.
@@ -37,10 +38,6 @@ namespace isobus
 			TimedOut ///< The heartbeat message has not been received within the repetition rate
 		};
 
-		/// @brief Constructor for a HeartbeatInterface
-		/// @param[in] sendCANFrameCallback A callback used to send CAN frames
-		HeartbeatInterface(const CANMessageFrameCallback &sendCANFrameCallback);
-
 		/// @brief This can be used to disable or enable this heartbeat functionality.
 		/// It's probably best to leave it enabled for most applications, but it's not
 		/// strictly needed.
@@ -91,7 +88,7 @@ namespace isobus
 
 		/// @brief Processes a CAN message, called by the network manager.
 		/// @param[in] message The CAN message being received
-		void process_rx_message(const CANMessage &message);
+		void process_rx_message(const CANMessage &message) override;
 
 		/// @brief Updates the interface. Called by the network manager,
 		/// so there is no need for you to call it in your application.
@@ -143,7 +140,6 @@ namespace isobus
 		                                          std::uint32_t repetitionRate,
 		                                          void *parentPointer);
 
-		const CANMessageFrameCallback sendCANFrameCallback; ///< A callback for sending a CAN frame
 		EventDispatcher<HeartBeatError, std::shared_ptr<ControlFunction>> heartbeatErrorEventDispatcher; ///< Event dispatcher for heartbeat errors
 		EventDispatcher<std::shared_ptr<ControlFunction>> newTrackedHeartbeatEventDispatcher; ///< Event dispatcher for when a heartbeat message from another control function becomes tracked by this interface
 		std::list<Heartbeat> trackedHeartbeats; ///< Store tracked heartbeat data, per CF

isobus/src/can_message_handling.cpp

@@ -22,17 +22,17 @@ namespace isobus
 	                                            void *parentPointer,
 	                                            DataChunkCallback frameChunkCallback) const
 	{
-		if (nullptr != messagingProvider)
+		if (!messagingProvider.expired())
 		{
-			return messagingProvider->send_can_message(parameterGroupNumber,
-			                                           dataBuffer,
-			                                           dataLength,
-			                                           sourceControlFunction,
-			                                           destinationControlFunction,
-			                                           priority,
-			                                           txCompleteCallback,
-			                                           parentPointer,
-			                                           frameChunkCallback);
+			return messagingProvider.lock()->send_can_message(parameterGroupNumber,
+			                                                  dataBuffer,
+			                                                  dataLength,
+			                                                  sourceControlFunction,
+			                                                  destinationControlFunction,
+			                                                  priority,
+			                                                  txCompleteCallback,
+			                                                  parentPointer,
+			                                                  frameChunkCallback);
 		}
 		return false;
 	}
@@ -73,10 +73,13 @@ namespace isobus
 
 	void CANMessageHandler::add_consumer(std::shared_ptr<CANMessagingConsumer> consumer)
 	{
-		// Ensure the consumer is not already in the list
-		remove_consumer(consumer);
-		consumer->messagingProvider = messagingProvider;
-		consumers.push_back(consumer);
+		if (nullptr != consumer)
+		{
+			// Ensure the consumer is not already in the list
+			remove_consumer(consumer);
+			consumer->messagingProvider = messagingProvider;
+			consumers.push_back(consumer);
+		}
 	}
 
 	void CANMessageHandler::remove_consumer(std::shared_ptr<CANMessagingConsumer> consumer)

isobus/src/can_network_manager.cpp

@@ -41,8 +41,14 @@ namespace isobus
 			get_next_can_message_from_tx_queue();
 		}
 
-		//! @todo: Remove this unsafe way of making a shared pointer to the network manager once the network manager is no longer a singleton
-		messageHandler.set_messaging_provider(std::shared_ptr<CANNetworkManager>(this));
+		//! @todo: Remove this unsafe way of making a shared pointer to the network manager once the network manager is no longer a singleton,
+		//! it's the cause of a double-free right now, but since it must be at the end of an application since the CANNetwork is a sngleton, it shouldn't harm too much for now
+		messagingProvider = std::shared_ptr<CANNetworkManager>(this);
+		messageHandler.set_messaging_provider(messagingProvider);
+		for (std::uint8_t i = 0; i < CAN_PORT_MAXIMUM; i++)
+		{
+			messageHandler.add_consumer(heartBeatInterfaces.at(i));
+		}
 		initialized = true;
 	}
 
@@ -501,6 +507,7 @@ namespace isobus
 		currentBusloadBitAccumulator.fill(0);
 		lastAddressClaimRequestTimestamp_ms.fill(0);
 		controlFunctionTable.fill({ nullptr });
+		heartBeatInterfaces.fill({ std::make_shared<HeartbeatInterface>() });
 
 		auto send_frame_callback = [this](std::uint32_t parameterGroupNumber,
 		                                  CANDataSpan data,
@@ -523,7 +530,6 @@ namespace isobus
 			transportProtocols.at(i).reset(new TransportProtocolManager(send_frame_callback, receive_message_callback, &configuration));
 			extendedTransportProtocols.at(i).reset(new ExtendedTransportProtocolManager(send_frame_callback, receive_message_callback, &configuration));
 			fastPacketProtocol.at(i).reset(new FastPacketProtocol(send_frame_callback));
-			heartBeatInterfaces.at(i).reset(new HeartbeatInterface(send_frame_callback));
 		}
 	}
 
@@ -1029,7 +1035,6 @@ namespace isobus
 			transportProtocols.at(currentMessage.get_can_port_index())->process_message(currentMessage);
 			extendedTransportProtocols.at(currentMessage.get_can_port_index())->process_message(currentMessage);
 			fastPacketProtocol.at(currentMessage.get_can_port_index())->process_message(currentMessage);
-			heartBeatInterfaces.at(currentMessage.get_can_port_index())->process_rx_message(currentMessage);
 			process_protocol_pgn_callbacks(currentMessage);
 			process_any_control_function_pgn_callbacks(currentMessage);
 			messageHandler.process_rx_message(currentMessage);

isobus/src/isobus_heartbeat.cpp

@@ -23,11 +23,6 @@
 
 namespace isobus
 {
-	HeartbeatInterface::HeartbeatInterface(const CANMessageFrameCallback &sendCANFrameCallback) :
-	  sendCANFrameCallback(sendCANFrameCallback)
-	{
-	}
-
 	void HeartbeatInterface::set_enabled(bool enable)
 	{
 		if ((!enable) && (enable != enabled))
@@ -139,11 +134,12 @@ namespace isobus
 		bool retVal = false;
 		const std::array<std::uint8_t, 1> buffer = { sequenceCounter };
 
-		retVal = parent.sendCANFrameCallback(static_cast<std::uint32_t>(CANLibParameterGroupNumber::HeartbeatMessage),
-		                                     CANDataSpan(buffer.data(), buffer.size()),
-		                                     CANNetworkManager::CANNetwork.get_internal_control_function(controlFunction),
-		                                     nullptr,
-		                                     CANIdentifier::CANPriority::Priority3);
+		retVal = parent.send_can_message(static_cast<std::uint32_t>(CANLibParameterGroupNumber::HeartbeatMessage),
+		                                 buffer.data(),
+		                                 buffer.size(),
+		                                 std::static_pointer_cast<InternalControlFunction>(controlFunction),
+		                                 nullptr,
+		                                 CANIdentifier::CANPriority::Priority3);
 		if (retVal)
 		{
 			timestamp_ms = SystemTiming::get_timestamp_ms(); // Sent OK