Skip to content

Commit

Permalink
refactor: remove #if directives in favor of a single line macro for l…
Browse files Browse the repository at this point in the history
…ock-guards

also added some missing directives around <thread> includes
  • Loading branch information
GwnDaan committed Feb 13, 2024
1 parent ad545ee commit b1afdd1
Show file tree
Hide file tree
Showing 19 changed files with 108 additions and 296 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include <cstring>
#include <string>
#include <thread>
#include <vector>

#include "isobus/hardware_integration/can_hardware_plugin.hpp"
Expand Down
9 changes: 2 additions & 7 deletions isobus/include/isobus/isobus/can_control_function.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@
#define CAN_CONTROL_FUNCTION_HPP

#include "isobus/isobus/can_NAME.hpp"
#include "isobus/utility/thread_synchronization.hpp"

#include <memory>

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#endif

namespace isobus
{
//================================================================================================
Expand Down Expand Up @@ -84,9 +81,7 @@ namespace isobus
ControlFunction(NAME NAMEValue, std::uint8_t addressValue, std::uint8_t CANPort, Type type = Type::External);

friend class CANNetworkManager; ///< The network manager needs access to the control function's internals
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
static std::mutex controlFunctionProcessingMutex; ///< Protects the control function tables
#endif
static Mutex controlFunctionProcessingMutex; ///< Protects the control function tables
const Type controlFunctionType; ///< The Type of the control function
NAME controlFunctionNAME; ///< The NAME of the control function
bool claimedAddressSinceLastAddressClaimRequest = false; ///< Used to mark CFs as stale if they don't claim within a certain time
Expand Down
17 changes: 6 additions & 11 deletions isobus/include/isobus/isobus/can_network_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@
#include "isobus/isobus/can_transport_protocol.hpp"
#include "isobus/isobus/nmea2000_fast_packet_protocol.hpp"
#include "isobus/utility/event_dispatcher.hpp"
#include "isobus/utility/thread_synchronization.hpp"

#include <array>
#include <deque>
#include <list>
#include <memory>
#include <queue>

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#endif

/// @brief This namespace encompasses all of the ISO11783 stack's functionality to reduce global namespace pollution
namespace isobus
{
Expand Down Expand Up @@ -402,13 +399,11 @@ namespace isobus
std::vector<ParameterGroupNumberCallbackData> anyControlFunctionParameterGroupNumberCallbacks; ///< A list of all global PGN callbacks
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
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex receivedMessageQueueMutex; ///< A mutex for receive messages thread safety
std::mutex protocolPGNCallbacksMutex; ///< A mutex for PGN callback thread safety
std::mutex anyControlFunctionCallbacksMutex; ///< Mutex to protect the "any CF" callbacks
std::mutex busloadUpdateMutex; ///< A mutex that protects the busload metrics since we calculate it on our own thread
std::mutex controlFunctionStatusCallbacksMutex; ///< A Mutex that protects access to the control function status callback list
#endif
Mutex receivedMessageQueueMutex; ///< A mutex for receive messages thread safety
Mutex protocolPGNCallbacksMutex; ///< A mutex for PGN callback thread safety
Mutex anyControlFunctionCallbacksMutex; ///< Mutex to protect the "any CF" callbacks
Mutex busloadUpdateMutex; ///< A mutex that protects the busload metrics since we calculate it on our own thread
Mutex controlFunctionStatusCallbacksMutex; ///< A Mutex that protects access to the control function status callback list
Mutex transmittedMessageQueueMutex; ///< A mutex for protecting the transmitted message queue
std::uint32_t busloadUpdateTimestamp_ms = 0; ///< Tracks a time window for determining approximate busload
std::uint32_t updateTimestamp_ms = 0; ///< Keeps track of the last time the CAN stack was update in milliseconds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,7 @@ namespace isobus
std::shared_ptr<InternalControlFunction> myControlFunction; ///< The internal control function that this protocol will send from
std::vector<PGNRequestCallbackInfo> pgnRequestCallbacks; ///< A list of all registered PGN callbacks and the PGN associated with each callback
std::vector<PGNRequestForRepetitionRateCallbackInfo> repetitionRateCallbacks; ///< A list of all registered request for repetition rate callbacks and the PGN associated with the callback
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex pgnRequestMutex; ///< A mutex to protect the callback lists
#endif
Mutex pgnRequestMutex; ///< A mutex to protect the callback lists
};
}

Expand Down
10 changes: 3 additions & 7 deletions isobus/include/isobus/isobus/can_stack_logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@
#ifndef CAN_STACK_LOGGER_HPP
#define CAN_STACK_LOGGER_HPP

#include "isobus/utility/thread_synchronization.hpp"

#include <memory>
#include <string>

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#endif

namespace isobus
{
//================================================================================================
Expand Down Expand Up @@ -170,9 +168,7 @@ namespace isobus

static CANStackLogger *logger; ///< A static pointer to an instance of a logger
static LoggingLevel currentLogLevel; ///< The current log level. Logs for levels below the current one will be dropped.
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
static std::mutex loggerMutex; ///< A mutex that protects the logger so it can be used from multiple threads
#endif
static Mutex loggerMutex; ///< A mutex that protects the logger so it can be used from multiple threads
};
} // namespace isobus

Expand Down
9 changes: 2 additions & 7 deletions isobus/include/isobus/isobus/isobus_functionalities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,11 @@
#include "isobus/isobus/can_parameter_group_number_request_protocol.hpp"
#include "isobus/isobus/can_protocol.hpp"
#include "isobus/utility/processing_flags.hpp"
#include "isobus/utility/thread_synchronization.hpp"

#include <list>
#include <vector>

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#endif

namespace isobus
{
class DiagnosticProtocol; // Forward declaration
Expand Down Expand Up @@ -473,9 +470,7 @@ namespace isobus
std::shared_ptr<InternalControlFunction> myControlFunction; ///< The control function to send messages as
std::list<FunctionalityData> supportedFunctionalities; ///< A list of all configured functionalities and their data
ProcessingFlags txFlags; ///< Handles retries for sending the CF functionalities message
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex functionalitiesMutex; ///< Since messages come in on a different thread than the main app (probably), this mutex protects the functionality data
#endif
Mutex functionalitiesMutex; ///< Since messages come in on a different thread than the main app (probably), this mutex protects the functionality data
};
} // namespace isobus
#endif // ISOBUS_FUNCTIONALITIES_HPP
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
#include "isobus/utility/processing_flags.hpp"

#include <list>
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <thread>
#endif

namespace isobus
{
Expand Down Expand Up @@ -625,8 +627,8 @@ namespace isobus
std::list<ProcessDataCallbackInfo> measurementMinimumThresholdCommands; ///< A list of measurement commands that will be processed when the value drops below a threshold
std::list<ProcessDataCallbackInfo> measurementMaximumThresholdCommands; ///< A list of measurement commands that will be processed when the value above a threshold
std::list<ProcessDataCallbackInfo> measurementOnChangeThresholdCommands; ///< A list of measurement commands that will be processed when the value changes by the specified amount
Mutex clientMutex; ///< A general mutex to protect data in the worker thread against data accessed by the app or the network manager
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex clientMutex; ///< A general mutex to protect data in the worker thread against data accessed by the app or the network manager
std::thread *workerThread = nullptr; ///< The worker thread that updates this interface
#endif
std::string ddopStructureLabel; ///< Stores a pre-parsed structure label, helps to avoid processing the whole DDOP during a CAN message callback
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "isobus/isobus/isobus_virtual_terminal_objects.hpp"
#include "isobus/utility/event_dispatcher.hpp"
#include "isobus/utility/processing_flags.hpp"
#include "isobus/utility/thread_synchronization.hpp"

#include <functional>
#include <map>
Expand All @@ -23,7 +24,6 @@
#include <vector>

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#include <thread>
#endif

Expand Down Expand Up @@ -1648,9 +1648,7 @@ namespace isobus
std::vector<std::vector<std::uint8_t>> commandQueue; ///< A queue of commands to send to the VT server
bool commandAwaitingResponse = false; ///< Determines if we are currently waiting for a response to a command
std::uint32_t lastCommandTimestamp_ms = 0; ///< The timestamp of the last command sent
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex commandQueueMutex; ///< A mutex to protect the command queue
#endif
Mutex commandQueueMutex; ///< A mutex to protect the command queue

// Activation event callbacks
EventDispatcher<VTKeyEvent> softKeyEventDispatcher; ///< A list of all soft key event callbacks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@

#include "isobus/isobus/can_internal_control_function.hpp"
#include "isobus/isobus/can_protocol.hpp"

#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
#include <mutex>
#endif
#include "isobus/utility/thread_synchronization.hpp"

namespace isobus
{
Expand Down Expand Up @@ -209,9 +206,7 @@ namespace isobus
std::vector<FastPacketProtocolSession *> activeSessions; ///< A list of all active TP sessions
std::vector<FastPacketHistory> sessionHistory; ///< Used to keep track of sequence numbers for future sessions
std::vector<ParameterGroupNumberCallbackData> parameterGroupNumberCallbacks; ///< A list of all parameter group number callbacks that will be parsed as fast packet messages
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex sessionMutex; ///< A mutex to lock the sessions list in case someone starts a Tx while the stack is processing sessions
#endif
Mutex sessionMutex; ///< A mutex to lock the sessions list in case someone starts a Tx while the stack is processing sessions
};

} // namespace isobus
Expand Down
8 changes: 2 additions & 6 deletions isobus/src/can_control_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@

namespace isobus
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::mutex ControlFunction::controlFunctionProcessingMutex;
#endif
Mutex ControlFunction::controlFunctionProcessingMutex;

isobus::ControlFunction::ControlFunction(NAME NAMEValue, std::uint8_t addressValue, std::uint8_t CANPort, Type type) :
controlFunctionType(type),
Expand All @@ -39,9 +37,7 @@ namespace isobus

bool ControlFunction::destroy(std::uint32_t expectedRefCount)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::lock_guard<std::mutex> lock(controlFunctionProcessingMutex);
#endif
LOCK_GUARD(Mutex, controlFunctionProcessingMutex);

CANNetworkManager::CANNetwork.on_control_function_destroyed(shared_from_this(), {});

Expand Down
61 changes: 16 additions & 45 deletions isobus/src/can_network_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,14 @@ namespace isobus

void CANNetworkManager::add_any_control_function_parameter_group_number_callback(std::uint32_t parameterGroupNumber, CANLibCallback callback, void *parent)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::lock_guard<std::mutex> lock(anyControlFunctionCallbacksMutex);
#endif
LOCK_GUARD(Mutex, anyControlFunctionCallbacksMutex);
anyControlFunctionParameterGroupNumberCallbacks.emplace_back(parameterGroupNumber, callback, parent, nullptr);
}

void CANNetworkManager::remove_any_control_function_parameter_group_number_callback(std::uint32_t parameterGroupNumber, CANLibCallback callback, void *parent)
{
ParameterGroupNumberCallbackData tempObject(parameterGroupNumber, callback, parent, nullptr);
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::lock_guard<std::mutex> lock(anyControlFunctionCallbacksMutex);
#endif
LOCK_GUARD(Mutex, anyControlFunctionCallbacksMutex);
auto callbackLocation = std::find(anyControlFunctionParameterGroupNumberCallbacks.begin(), anyControlFunctionParameterGroupNumberCallbacks.end(), tempObject);
if (anyControlFunctionParameterGroupNumberCallbacks.end() != callbackLocation)
{
Expand All @@ -108,9 +104,7 @@ namespace isobus

float CANNetworkManager::get_estimated_busload(std::uint8_t canChannel)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(busloadUpdateMutex);
#endif
LOCK_GUARD(Mutex, busloadUpdateMutex);
constexpr float ISOBUS_BAUD_RATE_BPS = 250000.0f;
float retVal = 0.0f;

Expand Down Expand Up @@ -225,9 +219,8 @@ namespace isobus

void CANNetworkManager::update()
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(ControlFunction::controlFunctionProcessingMutex);
#endif
auto &processingMutex = ControlFunction::controlFunctionProcessingMutex;
LOCK_GUARD(Mutex, processingMutex);

if (!initialized)
{
Expand Down Expand Up @@ -322,9 +315,7 @@ namespace isobus

if (initialized)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::lock_guard<std::mutex> lock(receivedMessageQueueMutex);
#endif
LOCK_GUARD(Mutex, receivedMessageQueueMutex);
receivedMessageQueue.push(std::move(message));
}
}
Expand Down Expand Up @@ -417,9 +408,7 @@ namespace isobus
{
if (nullptr != callback)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(controlFunctionStatusCallbacksMutex);
#endif
LOCK_GUARD(Mutex, controlFunctionStatusCallbacksMutex);
controlFunctionStateCallbacks.emplace_back(callback);
}
}
Expand All @@ -428,9 +417,7 @@ namespace isobus
{
if (nullptr != callback)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(controlFunctionStatusCallbacksMutex);
#endif
LOCK_GUARD(Mutex, controlFunctionStatusCallbacksMutex);
ControlFunctionStateCallback targetCallback(callback);
auto callbackLocation = std::find(controlFunctionStateCallbacks.begin(), controlFunctionStateCallbacks.end(), targetCallback);

Expand Down Expand Up @@ -501,9 +488,7 @@ namespace isobus
{
bool retVal = false;
ParameterGroupNumberCallbackData callbackInfo(parameterGroupNumber, callback, parentPointer, nullptr);
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(protocolPGNCallbacksMutex);
#endif
LOCK_GUARD(Mutex, protocolPGNCallbacksMutex);
if ((nullptr != callback) && (protocolPGNCallbacks.end() == find(protocolPGNCallbacks.begin(), protocolPGNCallbacks.end(), callbackInfo)))
{
protocolPGNCallbacks.push_back(callbackInfo);
Expand All @@ -516,9 +501,7 @@ namespace isobus
{
bool retVal = false;
ParameterGroupNumberCallbackData callbackInfo(parameterGroupNumber, callback, parentPointer, nullptr);
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(protocolPGNCallbacksMutex);
#endif
LOCK_GUARD(Mutex, protocolPGNCallbacksMutex);
if (nullptr != callback)
{
std::list<ParameterGroupNumberCallbackData>::iterator callbackLocation;
Expand Down Expand Up @@ -672,17 +655,13 @@ namespace isobus

void CANNetworkManager::update_busload(std::uint8_t channelIndex, std::uint32_t numberOfBitsProcessed)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(CANNetworkManager::CANNetwork.busloadUpdateMutex);
#endif
LOCK_GUARD(Mutex, busloadUpdateMutex);
currentBusloadBitAccumulator.at(channelIndex) += numberOfBitsProcessed;
}

void CANNetworkManager::update_busload_history()
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(busloadUpdateMutex);
#endif
LOCK_GUARD(Mutex, busloadUpdateMutex);
if (SystemTiming::time_expired_ms(busloadUpdateTimestamp_ms, BUSLOAD_UPDATE_FREQUENCY_MS))
{
for (std::size_t i = 0; i < busloadMessageBitsHistory.size(); i++)
Expand Down Expand Up @@ -923,9 +902,7 @@ namespace isobus

CANMessage CANNetworkManager::get_next_can_message_from_rx_queue()
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
std::lock_guard<std::mutex> lock(receivedMessageQueueMutex);
#endif
LOCK_GUARD(Mutex, receivedMessageQueueMutex);
if (!receivedMessageQueue.empty())
{
CANMessage retVal = std::move(receivedMessageQueue.front());
Expand Down Expand Up @@ -961,9 +938,7 @@ namespace isobus

void CANNetworkManager::process_any_control_function_pgn_callbacks(const CANMessage &currentMessage)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(anyControlFunctionCallbacksMutex);
#endif
LOCK_GUARD(Mutex, anyControlFunctionCallbacksMutex);
for (const auto &currentCallback : anyControlFunctionParameterGroupNumberCallbacks)
{
if ((currentCallback.get_parameter_group_number() == currentMessage.get_identifier().get_parameter_group_number()) &&
Expand Down Expand Up @@ -997,9 +972,7 @@ namespace isobus

void CANNetworkManager::process_control_function_state_change_callback(std::shared_ptr<ControlFunction> controlFunction, ControlFunctionState state)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(controlFunctionStatusCallbacksMutex);
#endif
LOCK_GUARD(Mutex, controlFunctionStatusCallbacksMutex);
for (const auto &callback : controlFunctionStateCallbacks)
{
callback(controlFunction, state);
Expand All @@ -1008,9 +981,7 @@ namespace isobus

void CANNetworkManager::process_protocol_pgn_callbacks(const CANMessage &currentMessage)
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
const std::lock_guard<std::mutex> lock(protocolPGNCallbacksMutex);
#endif
LOCK_GUARD(Mutex, protocolPGNCallbacksMutex);
for (const auto &currentCallback : protocolPGNCallbacks)
{
if (currentCallback.get_parameter_group_number() == currentMessage.get_identifier().get_parameter_group_number())
Expand Down
Loading

0 comments on commit b1afdd1

Please sign in to comment.