Skip to content

Commit

Permalink
[Core]: Fix some various higher priority code smells
Browse files Browse the repository at this point in the history
Fixed some sonar code smells
  • Loading branch information
ad3154 committed Mar 15, 2023
1 parent 574d399 commit 84e97c1
Show file tree
Hide file tree
Showing 23 changed files with 116 additions and 114 deletions.
2 changes: 1 addition & 1 deletion isobus/include/isobus/isobus/can_NAME.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ namespace isobus
bool operator==(const NAME &obj) const;

/// @brief A structure that tracks the pair of a NAME parameter and associated value
typedef std::pair<const NAMEParameters, const std::uint32_t> NameParameterFilter;
using NameParameterFilter = std::pair<const NAMEParameters, const std::uint32_t>;

/// @brief Constructor for a NAME
/// @param[in] rawNAMEData The raw 64 bit NAME of an ECU
Expand Down
11 changes: 6 additions & 5 deletions isobus/include/isobus/isobus/can_address_claim_state_machine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#define CAN_ADDRESS_CLAIM_STATE_MACHINE_HPP

#include "isobus/isobus/can_NAME.hpp"
#include "isobus/isobus/can_constants.hpp"

namespace isobus
{
Expand Down Expand Up @@ -80,20 +81,20 @@ namespace isobus
void set_current_state(State value);

/// @brief Sends the PGN request for the address claim PGN
bool send_request_to_claim();
bool send_request_to_claim() const;

/// @brief Sends the address claim message
/// @param[in] address The address to claim
bool send_address_claim(std::uint8_t address);

NAME m_isoname; ///< The ISO NAME to claim as
State m_currentState; ///< The address claim state machine state
std::uint32_t m_timestamp_ms; ///< A generic timestamp in milliseconds used to find timeouts
State m_currentState = State::None; ///< The address claim state machine state
std::uint32_t m_timestamp_ms = 0; ///< A generic timestamp in milliseconds used to find timeouts
std::uint8_t m_portIndex; ///< The CAN channel index to claim on
std::uint8_t m_preferredAddress; ///< The address we'd prefer to claim as (we may not get it)
std::uint8_t m_randomClaimDelay_ms; ///< The random delay as required by the ISO11783 standard
std::uint8_t m_claimedAddress; ///< The actual address we ended up claiming
bool m_enabled; ///< Enable/disable state for this state machine
std::uint8_t m_claimedAddress = NULL_CAN_ADDRESS; ///< The actual address we ended up claiming
bool m_enabled = true; ///< Enable/disable state for this state machine
};

} // namespace isobus
Expand Down
2 changes: 1 addition & 1 deletion isobus/include/isobus/isobus/can_badge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace isobus
private:
friend T; ///< Our best friend, T
//! @brief An empty function for our best friend <T>
CANLibBadge(){};
CANLibBadge() = default;
};
//! @endcond

Expand Down
42 changes: 21 additions & 21 deletions isobus/include/isobus/isobus/can_callbacks.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,29 @@ namespace isobus
/// @brief A callback for control functions to get CAN messages
typedef void (*CANLibCallback)(CANMessage *message, void *parentPointer);
/// @brief A callback to get chunks of data for transfer by a protocol
typedef bool (*DataChunkCallback)(std::uint32_t callbackIndex,
std::uint32_t bytesOffset,
std::uint32_t numberOfBytesNeeded,
std::uint8_t *chunkBuffer,
void *parentPointer);
using DataChunkCallback = bool (*)(std::uint32_t callbackIndex,
std::uint32_t bytesOffset,
std::uint32_t numberOfBytesNeeded,
std::uint8_t *chunkBuffer,
void *parentPointer);
/// @brief A callback for when a transmit is completed by the stack
typedef void (*TransmitCompleteCallback)(std::uint32_t parameterGroupNumber,
std::uint32_t dataLength,
InternalControlFunction *sourceControlFunction,
ControlFunction *destinationControlFunction,
bool successful,
void *parentPointer);
using TransmitCompleteCallback = void (*)(std::uint32_t parameterGroupNumber,
std::uint32_t dataLength,
InternalControlFunction *sourceControlFunction,
ControlFunction *destinationControlFunction,
bool successful,
void *parentPointer);
/// @brief A callback for handling a PGN request
typedef bool (*PGNRequestCallback)(std::uint32_t parameterGroupNumber,
ControlFunction *requestingControlFunction,
bool &acknowledge,
AcknowledgementType &acknowledgeType,
void *parentPointer);
using PGNRequestCallback = bool (*)(std::uint32_t parameterGroupNumber,
ControlFunction *requestingControlFunction,
bool &acknowledge,
AcknowledgementType &acknowledgeType,
void *parentPointer);
/// @brief A callback for handling a request for repetition rate for a specific PGN
typedef bool (*PGNRequestForRepetitionRateCallback)(std::uint32_t parameterGroupNumber,
ControlFunction *requestingControlFunction,
std::uint32_t repetitionRate,
void *parentPointer);
using PGNRequestForRepetitionRateCallback = bool (*)(std::uint32_t parameterGroupNumber,
ControlFunction *requestingControlFunction,
std::uint32_t repetitionRate,
void *parentPointer);

//================================================================================================
/// @class ParameterGroupNumberCallbackData
Expand All @@ -75,7 +75,7 @@ namespace isobus
/// @brief Equality operator for this class
/// @param[in] obj The object to check equality against
/// @returns true if the objects have equivalent data
bool operator==(const ParameterGroupNumberCallbackData &obj);
bool operator==(const ParameterGroupNumberCallbackData &obj) const;

/// @brief Assignment operator for this class
/// @param[in] obj The object to assign data from
Expand Down
14 changes: 7 additions & 7 deletions isobus/include/isobus/isobus/can_extended_transport_protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ namespace isobus
ExtendedTransportProtocolManager();

/// @brief The destructor for the TransportProtocolManager
virtual ~ExtendedTransportProtocolManager();
~ExtendedTransportProtocolManager() final;

/// @brief The protocol's initializer function
void initialize(CANLibBadge<CANNetworkManager>) override;
Expand Down Expand Up @@ -182,14 +182,14 @@ namespace isobus
/// @param[in] source The source control function for the session
/// @param[in] destination The destination control function for the session
/// @param[out] session The found session, or nullptr if no session matched the supplied parameters
bool get_session(ExtendedTransportProtocolSession *&session, ControlFunction *source, ControlFunction *destination);
bool get_session(ExtendedTransportProtocolSession *&session, ControlFunction *source, ControlFunction *destination) const;

/// @brief Gets an ETP session from the passed in source and destination and PGN combination
/// @param[in] source The source control function for the session
/// @param[in] destination The destination control function for the session
/// @param[in] parameterGroupNumber The PGN of the session
/// @param[out] session The found session, or nullptr if no session matched the supplied parameters
bool get_session(ExtendedTransportProtocolSession *&session, ControlFunction *source, ControlFunction *destination, std::uint32_t parameterGroupNumber);
bool get_session(ExtendedTransportProtocolSession *&session, ControlFunction *source, ControlFunction *destination, std::uint32_t parameterGroupNumber) const;

/// @brief Processes end of session callbacks
/// @param[in] session The session we've just completed
Expand All @@ -199,22 +199,22 @@ namespace isobus
/// @brief Sends the "end of message acknowledgement" message for the provided session
/// @param[in] session The session for which we're sending the EOM ACK
/// @returns true if the EOM was sent, false if sending was not successful
bool send_end_of_session_acknowledgement(ExtendedTransportProtocolSession *session); // ETP.CM_EOMA
bool send_end_of_session_acknowledgement(ExtendedTransportProtocolSession *session) const; // ETP.CM_EOMA

/// @brief Sends the "clear to send" message
/// @param[in] session The session for which we're sending the CTS
/// @returns true if the CTS was sent, false if sending was not successful
bool send_extended_connection_mode_clear_to_send(ExtendedTransportProtocolSession *session); // ETP.CM_CTS
bool send_extended_connection_mode_clear_to_send(ExtendedTransportProtocolSession *session) const; // ETP.CM_CTS

/// @brief Sends the "request to send" message as part of initiating a transmit
/// @param[in] session The session for which we're sending the RTS
/// @returns true if the RTS was sent, false if sending was not successful
bool send_extended_connection_mode_request_to_send(const ExtendedTransportProtocolSession *session); // ETP.CM_RTS
bool send_extended_connection_mode_request_to_send(const ExtendedTransportProtocolSession *session) const; // ETP.CM_RTS

/// @brief Sends the data packet offset message for the supplied session
/// @param[in] session The session for which we're sending the EDPO
/// @returns true if the EDPO was sent, false if sending was not successful
bool send_extended_connection_mode_data_packet_offset(const ExtendedTransportProtocolSession *session); // ETP.CM_DPO
bool send_extended_connection_mode_data_packet_offset(const ExtendedTransportProtocolSession *session) const; // ETP.CM_DPO

/// @brief Sets the state machine state of the ETP session
/// @param[in] session The session to update
Expand Down
2 changes: 1 addition & 1 deletion isobus/include/isobus/isobus/can_managed_message.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace isobus
public:
/// @brief Constructor for the CANLibManagedMessage
/// @param[in] CANPort The can channel index the message uses
CANLibManagedMessage(std::uint8_t CANPort);
explicit CANLibManagedMessage(std::uint8_t CANPort);

/// @brief Sets the message data to the value supplied. Creates a copy.
/// @param[in] dataBuffer The data payload
Expand Down
2 changes: 1 addition & 1 deletion isobus/include/isobus/isobus/can_message.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace isobus

/// @brief Constructor for a CAN message
/// @param[in] CANPort The can channel index the message uses
CANMessage(std::uint8_t CANPort);
explicit CANMessage(std::uint8_t CANPort);

/// @brief Destructor for a CAN message
virtual ~CANMessage() = default;
Expand Down
8 changes: 4 additions & 4 deletions isobus/include/isobus/isobus/can_network_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace isobus

/// @brief Returns the number of global PGN callbacks that have been registered with the network manager
/// @returns The number of global PGN callbacks that have been registered with the network manager
std::uint32_t get_number_global_parameter_group_number_callbacks() const;
std::size_t get_number_global_parameter_group_number_callbacks() const;

/// @brief Registers a callback for ANY control function sending the associated PGN
/// @param[in] parameterGroupNumber The PGN you want to register for
Expand Down Expand Up @@ -162,7 +162,7 @@ namespace isobus
std::uint8_t priority,
const void *data,
std::uint32_t size,
CANLibBadge<AddressClaimStateMachine>);
CANLibBadge<AddressClaimStateMachine>) const;

/// @brief Processes completed protocol messages. Causes PGN callbacks to trigger.
/// @param[in] protocolMessage The completed protocol message
Expand Down Expand Up @@ -205,7 +205,7 @@ namespace isobus
std::uint32_t parameterGroupNumber,
std::uint8_t priority,
const void *data,
std::uint32_t size);
std::uint32_t size) const;

/// @brief Returns a control function based on a CAN address and channel index
/// @param[in] CANPort The CAN channel index of the CAN message being processed
Expand Down Expand Up @@ -252,7 +252,7 @@ namespace isobus
std::uint32_t parameterGroupNumber,
std::uint8_t priority,
const void *data,
std::uint32_t size);
std::uint32_t size) const;

/// @brief Gets a PGN callback for the global address by index
/// @param[in] index The index of the callback to get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ namespace isobus
/// @param[in] NAMEFilters A list of filters that describe the identity of the CF based on NAME components
PartneredControlFunction(std::uint8_t CANPort, const std::vector<NAMEFilter> NAMEFilters);

/// @brief Deleted copy constructor for PartneredControlFunction to avoid slicing
PartneredControlFunction(PartneredControlFunction &) = delete;

/// @brief The destructor for PartneredControlFunction
virtual ~PartneredControlFunction();

Expand Down
3 changes: 3 additions & 0 deletions isobus/include/isobus/isobus/can_protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ namespace isobus
/// @brief The base class constructor for a CANLibProtocol
CANLibProtocol();

/// @brief Deleted copy constructor for a CANLibProtocol
CANLibProtocol(CANLibProtocol &) = delete;

/// @brief The base class destructor for a CANLibProtocol
virtual ~CANLibProtocol();

Expand Down
10 changes: 5 additions & 5 deletions isobus/include/isobus/isobus/can_transport_protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ namespace isobus
TransportProtocolManager();

/// @brief The destructor for the TransportProtocolManager
virtual ~TransportProtocolManager();
~TransportProtocolManager() final;

/// @brief The protocol's initializer function
void initialize(CANLibBadge<CANNetworkManager>) override;
Expand Down Expand Up @@ -186,22 +186,22 @@ namespace isobus
/// @brief Sends the "broadcast announce" message
/// @param[in] session The session for which we're sending the BAM
/// @returns true if the BAM was sent, false if sending was not successful
bool send_broadcast_announce_message(TransportProtocolSession *session);
bool send_broadcast_announce_message(TransportProtocolSession *session) const;

/// @brief Sends the "clear to send" message
/// @param[in] session The session for which we're sending the CTS
/// @returns true if the CTS was sent, false if sending was not successful
bool send_clear_to_send(TransportProtocolSession *session);
bool send_clear_to_send(TransportProtocolSession *session) const;

/// @brief Sends the "request to send" message as part of initiating a transmit
/// @param[in] session The session for which we're sending the RTS
/// @returns true if the RTS was sent, false if sending was not successful
bool send_request_to_send(TransportProtocolSession *session);
bool send_request_to_send(TransportProtocolSession *session) const;

/// @brief Sends the "end of message acknowledgement" message for the provided session
/// @param[in] session The session for which we're sending the EOM ACK
/// @returns true if the EOM was sent, false if sending was not successful
bool send_end_of_session_acknowledgement(TransportProtocolSession *session);
bool send_end_of_session_acknowledgement(TransportProtocolSession *session) const;

/// @brief Sets the state machine state of the TP session
/// @param[in] session The session to update
Expand Down
34 changes: 17 additions & 17 deletions isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "isobus/isobus/can_protocol.hpp"
#include "isobus/utility/processing_flags.hpp"

#include <array>
#include <list>
#include <memory>
#include <string>
Expand Down Expand Up @@ -174,8 +175,7 @@ namespace isobus
{
public:
/// @brief Constructor for a DTC, sets default values at construction time
/// @param[in] internalControlFunction The internal control function to use for sending messages
DiagnosticTroubleCode();
DiagnosticTroubleCode() = default;

/// @brief Constructor for a DTC, sets all values explicitly
/// @param[in] spn The suspect parameter number
Expand All @@ -191,12 +191,12 @@ namespace isobus
/// @brief Returns the occurance count, which will be kept track of by the protocol
std::uint8_t get_occurrance_count() const;

std::uint32_t suspectParameterNumber; ///< This 19-bit number is used to identify the item for which diagnostics are being reported
std::uint8_t failureModeIdentifier; ///< The FMI defines the type of failure detected in the sub-system identified by an SPN
LampStatus lampState; ///< The J1939 lamp state for this DTC
std::uint32_t suspectParameterNumber = 0xFFFFFFFF; ///< This 19-bit number is used to identify the item for which diagnostics are being reported
std::uint8_t failureModeIdentifier = static_cast<std::uint8_t>(FailureModeIdentifier::ConditionExists); ///< The FMI defines the type of failure detected in the sub-system identified by an SPN
LampStatus lampState = LampStatus::None; ///< The J1939 lamp state for this DTC
private:
friend class DiagnosticProtocol; ///< Allow the protocol to have write access the occurance but require other to use getter only
std::uint8_t occuranceCount; ///< Number of times the DTC has been active (0 to 126 with 127 being not available)
std::uint8_t occuranceCount = 0; ///< Number of times the DTC has been active (0 to 126 with 127 being not available)
};

/// @brief Used to tell the CAN stack that diagnostic messages should be sent from the specified internal control function
Expand Down Expand Up @@ -383,17 +383,17 @@ namespace isobus
static constexpr std::uint8_t DM13_BITS_PER_NETWORK = 2; ///< Number of bits for the network SPNs

/// @brief Lists the J1939 networks by index rather than by definition in J1939-73 5.7.13
static constexpr Network J1939NetworkIndicies[DM13_NUMBER_OF_J1939_NETWORKS] = { Network::SAEJ1939Network1PrimaryVehicleNetwork,
Network::SAEJ1939Network2,
Network::SAEJ1939Network3,
Network::SAEJ1939Network4,
Network::SAEJ1939Network5,
Network::SAEJ1939Network6,
Network::SAEJ1939Network7,
Network::SAEJ1939Network8,
Network::SAEJ1939Network9,
Network::SAEJ1939Network10,
Network::SAEJ1939Network11 };
static constexpr std::array<Network, DM13_NUMBER_OF_J1939_NETWORKS> J1939NetworkIndicies = { Network::SAEJ1939Network1PrimaryVehicleNetwork,
Network::SAEJ1939Network2,
Network::SAEJ1939Network3,
Network::SAEJ1939Network4,
Network::SAEJ1939Network5,
Network::SAEJ1939Network6,
Network::SAEJ1939Network7,
Network::SAEJ1939Network8,
Network::SAEJ1939Network9,
Network::SAEJ1939Network10,
Network::SAEJ1939Network11 };

/// @brief The constructor for this protocol
explicit DiagnosticProtocol(std::shared_ptr<InternalControlFunction> internalControlFunction);
Expand Down
Loading

0 comments on commit 84e97c1

Please sign in to comment.