From fc121d00f243d5f9c3ae1b74e8516a540df4e035 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Wed, 28 Jun 2023 18:56:20 -0500 Subject: [PATCH 1/7] [TP]: Fix multiple BAM Tx sessions could be created simultaneously Fixed an issue where we'd incorrectly allow multiple BAM Tx sessions at one time for a given ICF, which is not allowed by the standard. The issue here was that when we were checking for extant sessions, we were calling only the destination specific session getter. --- isobus/src/can_transport_protocol.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/isobus/src/can_transport_protocol.cpp b/isobus/src/can_transport_protocol.cpp index a9120b2d..63c03f20 100644 --- a/isobus/src/can_transport_protocol.cpp +++ b/isobus/src/can_transport_protocol.cpp @@ -369,7 +369,10 @@ namespace isobus (true == source->get_address_valid()) && ((nullptr == destination) || (destination->get_address_valid())) && - (!get_session(session, source, destination, parameterGroupNumber))) + (!get_session(session, source, destination, parameterGroupNumber)) && + ((nullptr != destination) || + ((nullptr == destination) && + (!get_session(session, source, destination))))) { TransportProtocolSession *newSession = new TransportProtocolSession(TransportProtocolSession::Direction::Transmit, source->get_can_port()); From c025dfb2841f76e9ed7c27392205798cb58664a6 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Wed, 28 Jun 2023 18:59:08 -0500 Subject: [PATCH 2/7] [Core]: Fix a crash associated to a CF eviction log message Fixed order of operations when nulling out an evicted CF. --- isobus/src/can_network_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/isobus/src/can_network_manager.cpp b/isobus/src/can_network_manager.cpp index a78fc070..cccfcf20 100644 --- a/isobus/src/can_network_manager.cpp +++ b/isobus/src/can_network_manager.cpp @@ -389,12 +389,12 @@ namespace isobus // Need to evict them from the table and move them to the inactive list targetControlFunction->address = NULL_CAN_ADDRESS; inactiveControlFunctions.push_back(targetControlFunction); - targetControlFunction = nullptr; CANStackLogger::debug("[NM]: %s CF '%016llx' is evicted from address '%d' on channel '%d', as their address is probably stolen.", targetControlFunction->get_type_string().c_str(), targetControlFunction->get_NAME().get_full_name(), claimedAddress, channelIndex); + targetControlFunction = nullptr; } if (targetControlFunction != nullptr) From bbe386796ddaa0186b1f2e1f51c5b12b1d8a0d9b Mon Sep 17 00:00:00 2001 From: Daan Steenbergen Date: Fri, 23 Jun 2023 19:57:41 +0200 Subject: [PATCH 3/7] [DP] Refactor away from diagnosticProtocolList - Stop using CANLibProtocol to be more inline with newly added interfaces (Guidance/speed/maintain power) - Add dedicated NetworkType configuration parameter - use in-class initializers where possible - Fixed several typos - Moved `parse_j1939_network_states` function from public to private - Introduced `initialize` / `terminate` as replacements for `assign_...` / `deassign_....` --- examples/diagnostic_protocol/main.cpp | 114 +++--- .../isobus/isobus_diagnostic_protocol.hpp | 173 +++------ isobus/src/isobus_diagnostic_protocol.cpp | 352 ++++++------------ test/diagnostic_protocol_tests.cpp | 41 +- 4 files changed, 241 insertions(+), 439 deletions(-) diff --git a/examples/diagnostic_protocol/main.cpp b/examples/diagnostic_protocol/main.cpp index dcb825a8..330e63b9 100644 --- a/examples/diagnostic_protocol/main.cpp +++ b/examples/diagnostic_protocol/main.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -66,61 +67,74 @@ int main() auto TestInternalECU = isobus::InternalControlFunction::create(TestDeviceNAME, 0x1C, 0); - // Wait to make sure address claiming is done. The time is arbitrary. - //! @todo Check this instead of asuming it is done - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + // Make sure address claiming is done before we continue + auto addressClaimedFuture = std::async(std::launch::async, [&TestInternalECU]() { + while (!TestInternalECU->get_address_valid()) + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + }); + if (addressClaimedFuture.wait_for(std::chrono::seconds(5)) == std::future_status::timeout) + { + std::cout << "Address claiming failed. Please make sure that your internal control function can claim a valid address." << std::endl; + return -3; + } - isobus::DiagnosticProtocol::assign_diagnostic_protocol_to_internal_control_function(TestInternalECU); + isobus::DiagnosticProtocol diagnosticProtocol(TestInternalECU); + diagnosticProtocol.initialize(); - isobus::DiagnosticProtocol *diagnosticProtocol = isobus::DiagnosticProtocol::get_diagnostic_protocol_by_internal_control_function(TestInternalECU); + // Important: we need to update the diagnostic protocol using the hardware interface periodic update event, + // otherwise the diagnostic protocol will not be able to update its internal state. + isobus::CANHardwareInterface::get_periodic_update_event_dispatcher().add_listener([&diagnosticProtocol]() { + diagnosticProtocol.update(); + }); // Make a few test DTCs isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC1(1234, isobus::DiagnosticProtocol::FailureModeIdentifier::ConditionExists, isobus::DiagnosticProtocol::LampStatus::None); isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC2(567, isobus::DiagnosticProtocol::FailureModeIdentifier::DataErratic, isobus::DiagnosticProtocol::LampStatus::AmberWarningLampSlowFlash); - isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC3(8910, isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntellegentDevice, isobus::DiagnosticProtocol::LampStatus::RedStopLampSolid); - - if (nullptr != diagnosticProtocol) - { - // Set a product identification string (in case someone requests it) - diagnosticProtocol->set_product_identification_code("1234567890ABC"); - diagnosticProtocol->set_product_identification_brand("Open-Agriculture"); - diagnosticProtocol->set_product_identification_model("AgIsoStack++ CAN Stack DP Example"); - - // Set a software ID string (This is what tells other ECUs what version your software is) - diagnosticProtocol->set_software_id_field(0, "Diagnostic Protocol Example 1.0.0"); - diagnosticProtocol->set_software_id_field(1, "Another version string x.x.x.x"); - - // Set an ECU ID (This is what tells other ECUs more details about your specific physical ECU) - diagnosticProtocol->set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::HardwareID, "Hardware ID"); - diagnosticProtocol->set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::Location, "The Aether"); - diagnosticProtocol->set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::ManufacturerName, "None"); - diagnosticProtocol->set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::PartNumber, "1234"); - diagnosticProtocol->set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::SerialNumber, "1"); - - // Let's say that our ECU has the capability of a universal terminal working set (as an example) and - // contains weak internal bus termination. - // This info gets reported to any ECU on the bus that requests our capabilities through the - // control function functionalities message. - diagnosticProtocol->ControlFunctionFunctionalitiesMessageInterface.set_functionality_is_supported(isobus::ControlFunctionFunctionalities::Functionalities::MinimumControlFunction, 1, true); - diagnosticProtocol->ControlFunctionFunctionalitiesMessageInterface.set_minimum_control_function_option_state(isobus::ControlFunctionFunctionalities::MinimumControlFunctionOptions::Type1ECUInternalWeakTermination, true); - diagnosticProtocol->ControlFunctionFunctionalitiesMessageInterface.set_functionality_is_supported(isobus::ControlFunctionFunctionalities::Functionalities::UniversalTerminalWorkingSet, 1, true); - - // Set the DTCs active. This should put them in the DM1 message - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC1, true); - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC2, true); - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC3, true); - - std::this_thread::sleep_for(std::chrono::milliseconds(5000)); // Send the DM1 for a while - - // Set the DTCs inactive. This should put them in the DM2 message - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC1, false); - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC2, false); - diagnosticProtocol->set_diagnostic_trouble_code_active(testDTC3, false); - - std::this_thread::sleep_for(std::chrono::milliseconds(5000)); // Send the DM2 for a while - - diagnosticProtocol->clear_inactive_diagnostic_trouble_codes(); // All messages should now be clear! - } + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC3(8910, isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntelligentDevice, isobus::DiagnosticProtocol::LampStatus::RedStopLampSolid); + + // Set a product identification string (in case someone requests it) + diagnosticProtocol.set_product_identification_code("1234567890ABC"); + diagnosticProtocol.set_product_identification_brand("Open-Agriculture"); + diagnosticProtocol.set_product_identification_model("AgIsoStack++ CAN Stack DP Example"); + + // Set a software ID string (This is what tells other ECUs what version your software is) + diagnosticProtocol.set_software_id_field(0, "Diagnostic Protocol Example 1.0.0"); + diagnosticProtocol.set_software_id_field(1, "Another version string x.x.x.x"); + + // Set an ECU ID (This is what tells other ECUs more details about your specific physical ECU) + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::HardwareID, "Hardware ID"); + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::Location, "The Aether"); + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::ManufacturerName, "None"); + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::PartNumber, "1234"); + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::SerialNumber, "1"); + + // Let's say that our ECU has the capability of a universal terminal working set (as an example) and + // contains weak internal bus termination. + // This info gets reported to any ECU on the bus that requests our capabilities through the + // control function functionalities message. + diagnosticProtocol.ControlFunctionFunctionalitiesMessageInterface.set_functionality_is_supported(isobus::ControlFunctionFunctionalities::Functionalities::MinimumControlFunction, 1, true); + diagnosticProtocol.ControlFunctionFunctionalitiesMessageInterface.set_minimum_control_function_option_state(isobus::ControlFunctionFunctionalities::MinimumControlFunctionOptions::Type1ECUInternalWeakTermination, true); + diagnosticProtocol.ControlFunctionFunctionalitiesMessageInterface.set_functionality_is_supported(isobus::ControlFunctionFunctionalities::Functionalities::UniversalTerminalWorkingSet, 1, true); + + std::cout << "Diagnostic Protocol initialized." << std::endl; + // Set the DTCs active. This should put them in the DM1 message + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC1, true); + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC2, true); + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC3, true); + + std::cout << "Diagnostic Trouble Codes set active. (DM1)" << std::endl; + std::this_thread::sleep_for(std::chrono::milliseconds(5000)); // Send the DM1 for a while + + // Set the DTCs inactive. This should put them in the DM2 message + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC1, false); + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC2, false); + diagnosticProtocol.set_diagnostic_trouble_code_active(testDTC3, false); + + std::cout << "Diagnostic Trouble Codes set inactive. (DM2)" << std::endl; + std::this_thread::sleep_for(std::chrono::milliseconds(5000)); // Send the DM2 for a while + + diagnosticProtocol.clear_inactive_diagnostic_trouble_codes(); // All messages should now be clear! + std::cout << "Diagnostic Trouble Codes cleared." << std::endl; while (running) { @@ -128,7 +142,7 @@ int main() std::this_thread::sleep_for(std::chrono::milliseconds(1000)); } + diagnosticProtocol.terminate(); isobus::CANHardwareInterface::stop(); - isobus::DiagnosticProtocol::deassign_all_diagnostic_protocol_to_internal_control_functions(); return 0; } diff --git a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp index fb68bd67..758dfc96 100644 --- a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp +++ b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp @@ -52,7 +52,7 @@ namespace isobus /// @class DiagnosticProtocol /// @brief Manages the DM1, DM2, and DM3 messages for ISO11783 or J1939 //================================================================================================ - class DiagnosticProtocol : public CANLibProtocol + class DiagnosticProtocol { public: /// @brief Enumerates the different fields in the ECU identification message @@ -72,7 +72,7 @@ namespace isobus { None, MalfunctionIndicatorLampSolid, ///< A lamp used to relay only emissions-related trouble code information - MalfuctionIndicatorLampSlowFlash, ///< A lamp used to relay only emissions-related trouble code information + MalfunctionIndicatorLampSlowFlash, ///< A lamp used to relay only emissions-related trouble code information MalfunctionIndicatorLampFastFlash, ///< A lamp used to relay only emissions-related trouble code information RedStopLampSolid, ///< This lamp is used to relay trouble code information that is of a severe-enough condition that it warrants stopping the vehicle RedStopLampSlowFlash, ///< This lamp is used to relay trouble code information that is of a severe-enough condition that it warrants stopping the vehicle @@ -97,10 +97,10 @@ namespace isobus CurrentAboveNormal = 6, ///< A current signal, data or otherwise, is above the predefined limits that bound the range MechanicalSystemNotResponding = 7, ///< Any fault that is detected as the result of an improper mechanical adjustment, an improper response or action of a mechanical system AbnormalFrequency = 8, ///< Any frequency or PWM signal that is outside the predefined limits which bound the signal range for frequency or duty cycle - AbnotmalUpdateRate = 9, ///< Any failure that is detected when receipt of data through the data network is not at the update rate expected or required + AbnormalUpdateRate = 9, ///< Any failure that is detected when receipt of data through the data network is not at the update rate expected or required AbnormalRateOfChange = 10, ///< Any data, exclusive of FMI 2, that are considered valid but which are changing at a rate that is outside the predefined limits that bound the rate of change for the system RootCauseNotKnown = 11, ///< It has been detected that a failure has occurred in a particular subsystem but the exact nature of the fault is not known - BadIntellegentDevice = 12, ///< Internal diagnostic procedures have determined that the failure is one which requires the replacement of the ECU + BadIntelligentDevice = 12, ///< Internal diagnostic procedures have determined that the failure is one which requires the replacement of the ECU OutOfCalibration = 13, ///< A failure that can be identified as the result of improper calibration SpecialInstructions = 14, ///< Used when the on-board system can isolate the failure to a small number of choices but not to a single point of failure. See 11783-12 Annex E DataValidAboveNormalLeastSevere = 15, ///< Condition is above what would be considered normal as determined by the predefined least severe level limits for that particular measure of the condition @@ -124,7 +124,7 @@ namespace isobus }; /// @brief Enumerates the different networks in the DM13 - enum class Network : std::uint8_t + enum class NetworkType : std::uint8_t { SAEJ1939Network1PrimaryVehicleNetwork = 0, SAEJ1922Network = 1, @@ -187,51 +187,44 @@ namespace isobus /// @brief A useful way to compare DTC objects to each other for equality /// @param[in] obj The "rhs" of the comparison /// @returns `true` if the objects were equal - bool operator==(const DiagnosticTroubleCode &obj); + bool operator==(const DiagnosticTroubleCode &obj) const; - /// @brief Returns the occurance count, which will be kept track of by the protocol - std::uint8_t get_occurrance_count() const; + /// @brief Returns the occurrence count, which will be kept track of by the protocol + /// @returns The occurrence count (0 to 126 with 127 being not available) + std::uint8_t get_occurrence_count() const; + /// @brief Returns the suspect parameter number + /// @returns The suspect parameter number + std::uint32_t get_suspect_parameter_number() const; + + /// @brief Returns the failure mode indicator + /// @returns The failure mode indicator + FailureModeIdentifier get_failure_mode_identifier() const; + + private: + friend class DiagnosticProtocol; 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(FailureModeIdentifier::ConditionExists); ///< The FMI defines the type of failure detected in the sub-system identified by an SPN + FailureModeIdentifier failureModeIdentifier = 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 = 0; ///< Number of times the DTC has been active (0 to 126 with 127 being not available) + std::uint8_t occurrenceCount = 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 - /// @details This will allocate an instance of this protocol - /// @note Assigning the diagnostic protocol to an ICF will automatically create an instance of the PGN request protocol if needed - /// as this protocol uses that protocol to abstract away PGN request implementation details. That protocol instance will - /// only be deleted if you call deassign_diagnostic_protocol_to_internal_control_function and the DP PGNs were the only registered PGNs in - /// the protocol OR if you manually deassign the PGN request protocol. - /// Most people will not need to worry about this detail. - /// @returns `true` If the protocol instance was created OK with the passed in ICF - static bool assign_diagnostic_protocol_to_internal_control_function(std::shared_ptr internalControlFunction); - - /// @brief Used to tell the CAN stack that diagnostic messages should no longer be sent from the specified internal control function - /// @details This will delete an instance of this protocol and may delete an associated but unused instance of the PGN request protocol. - /// @returns `true` If the protocol instance was deleted OK according to the passed in ICF - static bool deassign_diagnostic_protocol_to_internal_control_function(std::shared_ptr internalControlFunction); - - /// @brief Used to tell the CAN stack that diagnostic messages should no longer be sent from any internal control function - /// @details This will delete all instances of this protocol and may delete associated but unused instances of the PGN request protocol. - static void deassign_all_diagnostic_protocol_to_internal_control_functions(); - - /// @brief Retuns the diagnostic protocol assigned to an internal control function, if any - /// @param internalControlFunction The internal control function to search against - /// @returns The protocol object associated to the passed in ICF, or `nullptr` if none found that match the passed in ICF - static DiagnosticProtocol *get_diagnostic_protocol_by_internal_control_function(std::shared_ptr internalControlFunction); + /// @brief The constructor for this protocol + /// @param[in] internalControlFunction The internal control function that owns this protocol and will be used to send messages + /// @param[in] networkType The type of diagnostic network that this protocol will reflect + explicit DiagnosticProtocol(std::shared_ptr internalControlFunction, NetworkType networkType = NetworkType::ProprietaryNetwork1); - /// @brief Parses out the DM13 J1939 network states from a CAN message - /// @param[in] message The message to parse from - /// @param[in] networkStates The returned network state bitfield based on the message contents - /// @returns `true` if the message was parsed, `false` if the message was invalid - static bool parse_j1939_network_states(const CANMessage &message, std::uint32_t &networkStates); + /// @brief The destructor for this protocol + ~DiagnosticProtocol(); /// @brief The protocol's initializer function - void initialize(CANLibBadge) override; + void initialize(); + + /// @brief The protocol's terminate function + void terminate(); + + /// @brief Updates the diagnostic protocol + void update(); /// @brief Enables the protocol to run in J1939 mode instead of ISO11783 mode /// @details See ISO11783-12 and J1939-73 for a complete explanation of the differences @@ -252,17 +245,12 @@ namespace isobus /// @brief Clears the list of active DTCs and makes them all inactive void clear_active_diagnostic_trouble_codes(); - /// @brief Clears the list of inactive DTCs and clears occurance counts + /// @brief Clears the list of inactive DTCs and clears occurrence counts void clear_inactive_diagnostic_trouble_codes(); /// @brief Clears all previously configured software ID fields set with set_software_id_field void clear_software_id_fields(); - /// @brief Returns if broadcasts are suspended for the specified CAN channel (requested by DM13) - /// @param[in] canChannelIndex The CAN channel to check for suspended broadcasts - /// @returns `true` if broadcasts should are suspended for the specified channel - bool get_are_broadcasts_stopped_for_channel(std::uint8_t canChannelIndex) const; - /// @brief Sets one of the ECU identification strings for the ECU ID message /// @details See ECUIdentificationFields for a brief description of the fields /// @note The fields in this message are optional and separated by an ASCII �*�. It is not necessary to include parametric @@ -310,7 +298,7 @@ namespace isobus /// @brief Adds an ascii string to this internal control function's software ID /// @details Use this to identify the software version of your application. - /// Seperate fields will be transmitted with a `*` delimeter. + /// Separate fields will be transmitted with a `*` delimeter. /// For example, if your main application's version is 1.00, and you have a bootloader /// that is version 2.00, you could set field `0` to be "App v1.00" and /// field `1` to be "Bootloader v2.00", and it will be transmitted on request as: @@ -320,15 +308,10 @@ namespace isobus /// @param[in] value The software ID string to add void set_software_id_field(std::uint32_t index, std::string value); - /// @brief Informs the network that you are going to suspend broadcasts - /// @param[in] canChannelIndex The CAN channel you will suspend broadcasts on. Will be converted to the proper message `Network` by the stack - /// @param[in] sourceControlFunction The internal control function to send the DM13 from + /// @brief Informs the diagnostic protocol that you are going to suspend broadcasts /// @param[in] suspendTime_seconds If you know the time for which broadcasts will be suspended, put it here, otherwise 0xFFFF /// @returns `true` if the message was sent, otherwise `false` - bool suspend_broadcasts(std::uint8_t canChannelIndex, std::shared_ptr sourceControlFunction, std::uint16_t suspendTime_seconds = 0xFFFF); - - /// @brief Updates the protocol cyclically - void update(CANLibBadge) override; + bool suspend_broadcasts(std::uint16_t suspendTime_seconds = 0xFFFF); private: /// @brief Lists the different lamps in J1939-73 @@ -348,7 +331,7 @@ namespace isobus Fast ///< Fast flash }; - /// @brief The DM22 multiplexor bytes. All bytes not given a value here are reserved by SAE + /// @brief The DM22 multiplexer bytes. All bytes not given a value here are reserved by SAE enum class DM22ControlByte : std::uint8_t { RequestToClearPreviouslyActiveDTC = 0x01, ///< Clear a previously active DTC @@ -365,7 +348,7 @@ namespace isobus General = 0x00, ///< General negative acknowledge AccessDenied = 0x01, ///< Security denied access UnknownOrDoesNotExist = 0x02, ///< The DTC is unknown or does not exist - DTCUNoLongerPreviouslyActive = 0x03, ///< The DTC in in the active list but it was requested to clear from inactive list + DTCNoLongerPreviouslyActive = 0x03, ///< The DTC in in the active list but it was requested to clear from inactive list DTCNoLongerActive = 0x04 ///< DTC is inactive, not active, but active was requested to be cleared }; @@ -380,35 +363,16 @@ namespace isobus bool nack; ///< true if we are sending a NACK instead of PACK. Determines if we use nackIndicator }; - static constexpr std::uint32_t DM_MAX_FREQUENCY_MS = 1000; ///< You are techically allowed to send more than this under limited circumstances, but a hard limit saves 4 RAM bytes per DTC and has BAM benefits + static constexpr std::uint32_t DM_MAX_FREQUENCY_MS = 1000; ///< You are technically allowed to send more than this under limited circumstances, but a hard limit saves 4 RAM bytes per DTC and has BAM benefits static constexpr std::uint32_t DM13_HOLD_SIGNAL_TRANSMIT_INTERVAL_MS = 5000; ///< Defined in 5.7.13.13 SPN 1236 - static constexpr std::uint32_t DM13_TIMEOUT_MS = 6000; ///< The timout in 5.7.13 after which nodes shall revert back to the normal broadcast state - static constexpr std::uint16_t MAX_PAYLOAD_SIZE_BYTES = 1785; ///< DM 1 and 2 are limited to the BAM message max, becuase ETP does not allow global destinations + static constexpr std::uint32_t DM13_TIMEOUT_MS = 6000; ///< The timeout in 5.7.13 after which nodes shall revert back to the normal broadcast state + static constexpr std::uint16_t MAX_PAYLOAD_SIZE_BYTES = 1785; ///< DM 1 and 2 are limited to the BAM message max, because ETP does not allow global destinations static constexpr std::uint8_t DM_PAYLOAD_BYTES_PER_DTC = 4; ///< The number of payload bytes per DTC that gets encoded into the messages static constexpr std::uint8_t PRODUCT_IDENTIFICATION_MAX_STRING_LENGTH = 50; ///< The max string length allowed in the fields of product ID, as defined in ISO 11783-12 static constexpr std::uint8_t DM13_NUMBER_OF_J1939_NETWORKS = 11; ///< The number of networks in DM13 that are set aside for J1939 static constexpr std::uint8_t DM13_NETWORK_BITMASK = 0x03; ///< Used to mask the network SPN values 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 std::array 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); - - /// @brief The destructor for this protocol - ~DiagnosticProtocol(); - /// @brief A utility function to get the CAN representation of a FlashState /// @param flash The flash state to convert /// @returns The two bit lamp state for CAN @@ -433,26 +397,6 @@ namespace isobus /// @param[out] lampOn If the lamp state is on for any DTC void get_inactive_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn); - /// @brief The network manager calls this to see if the protocol can accept a non-raw CAN message for processing - /// @note In this protocol, we do not accept messages from the network manager for transmission - /// @param[in] parameterGroupNumber The PGN of the message - /// @param[in] data The data to be sent - /// @param[in] messageLength The length of the data to be sent - /// @param[in] source The source control function - /// @param[in] destination The destination control function - /// @param[in] transmitCompleteCallback A callback for when the protocol completes its work - /// @param[in] parentPointer A generic context object for the tx complete and chunk callbacks - /// @param[in] frameChunkCallback A callback to get some data to send - /// @returns true if the message was accepted by the protocol for processing - bool protocol_transmit_message(std::uint32_t parameterGroupNumber, - const std::uint8_t *data, - std::uint32_t messageLength, - std::shared_ptr source, - std::shared_ptr destination, - TransmitCompleteCallback transmitCompleteCallback, - void *parentPointer, - DataChunkCallback frameChunkCallback) override; - /// @brief Sends a DM1 encoded CAN message /// @returns true if the message was sent, otherwise false bool send_diagnostic_message_1(); @@ -461,18 +405,14 @@ namespace isobus /// @returns true if the message was sent, otherwise false bool send_diagnostic_message_2(); - /// @brief Sends a DM22 response message - /// @param data The components of the DM22 response - /// @returns true if the message was sent - bool send_diagnostic_message_22_response(DM22Data data); - /// @brief Sends a message that identifies which diagnostic protocols are supported /// @returns true if the message was sent, otherwise false bool send_diagnostic_protocol_identification(); /// @brief Sends the DM13 to alert network devices of impending suspended broadcasts + /// @param suspendTime_seconds The number of seconds that the broadcast will be suspended for /// @returns `true` if the message was sent, otherwise `false` - bool send_dm13_announce_suspension(std::shared_ptr sourceControlFunction, std::uint16_t suspendTime_seconds); + bool send_dm13_announce_suspension(std::uint16_t suspendTime_seconds); /// @brief Sends the ECU ID message /// @returns true if the message was sent @@ -493,13 +433,18 @@ namespace isobus /// @brief A generic way for a protocol to process a received message /// @param[in] message A received CAN message - void process_message(const CANMessage &message) override; + void process_message(const CANMessage &message); /// @brief A generic way for a protocol to process a received message /// @param[in] message A received CAN message /// @param[in] parent Provides the context to the actual TP manager object static void process_message(const CANMessage &message, void *parent); + /// @brief Parses out the DM13 J1939 network states from a CAN message + /// @param[in] message The message to parse from + /// @returns `true` if the message was parsed, `false` if the message was invalid + bool parse_j1939_network_states(const CANMessage &message); + /// @brief Handles PGN requests for the diagnostic protocol /// @param[in] parameterGroupNumber The PGN being requested /// @param[in] requestingControlFunction The control function that is requesting the PGN @@ -516,7 +461,7 @@ namespace isobus /// @param[in] requestingControlFunction The control function that is requesting the PGN /// @param[out] acknowledge Tells the PGN request protocol if it should respond to the request /// @param[out] acknowledgementType The type of acknowledgement to send to the requestor - /// @param[in] parentPointer Generic context variable, usually a pointer to the class that the callback was registed for + /// @param[in] parentPointer Generic context variable, usually a pointer to the class that the callback was registered for /// @returns true if any callback was able to handle the PGN request static bool process_parameter_group_number_request(std::uint32_t parameterGroupNumber, std::shared_ptr requestingControlFunction, @@ -529,22 +474,22 @@ namespace isobus /// @param[in] parentPointer A generic context pointer to reference a specific instance of this protocol in the callback static void process_flags(std::uint32_t flag, void *parentPointer); - static std::list diagnosticProtocolList; ///< List of all diagnostic protocol instances (one per ICF) - std::shared_ptr myControlFunction; ///< The internal control function that this protocol will send from + NetworkType networkType; ///< The diagnostic network type that this protocol will use std::vector activeDTCList; ///< Keeps track of all the active DTCs std::vector inactiveDTCList; ///< Keeps track of all the previously active DTCs std::vector dm22ResponseQueue; ///< Maintaining a list of DM22 responses we need to send to allow for retrying in case of Tx failures - std::vector ecuIdentificationFields; ///< Stores the ECU ID fields so we can transmit them when ECUID's PGN is requested + std::vector ecuIdentificationFields; ///< Stores the ECU ID fields so we can transmit them when ECU ID's PGN is requested std::vector softwareIdentificationFields; ///< Stores the Software ID fields so we can transmit them when the PGN is requested ProcessingFlags txFlags; ///< An instance of the processing flags to handle retries of some messages std::string productIdentificationCode; ///< The product identification code for sending the product identification message std::string productIdentificationBrand; ///< The product identification brand for sending the product identification message std::string productIdentificationModel; ///< The product identification model name for sending the product identification message - std::uint32_t lastDM1SentTimestamp; ///< A timestamp in milliseconds of the last time a DM1 was sent - std::uint32_t stopBroadcastNetworkBitfield; ///< Bitfield for tracking the network broadcast states for DM13 - std::uint32_t lastDM13ReceivedTimestamp; ///< A timestamp in milliseconds when we last got a DM13 message - bool j1939Mode; ///< Tells the protocol to operate according to J1939 instead of ISO11783 + std::uint32_t lastDM1SentTimestamp = 0; ///< A timestamp in milliseconds of the last time a DM1 was sent + bool broadcastState = true; ///< Bitfield for tracking the network broadcast state for DM13 + std::uint32_t lastDM13ReceivedTimestamp = 0; ///< A timestamp in milliseconds when we last got a DM13 message + bool j1939Mode = false; ///< Tells the protocol to operate according to J1939 instead of ISO11783 + bool initialized = false; ///< Stores if the interface has been initialized }; } diff --git a/isobus/src/isobus_diagnostic_protocol.cpp b/isobus/src/isobus_diagnostic_protocol.cpp index b5abad21..215bd8fc 100644 --- a/isobus/src/isobus_diagnostic_protocol.cpp +++ b/isobus/src/isobus_diagnostic_protocol.cpp @@ -45,38 +45,31 @@ namespace isobus { - std::list DiagnosticProtocol::diagnosticProtocolList; - constexpr std::array DiagnosticProtocol::J1939NetworkIndicies; - - DiagnosticProtocol::DiagnosticTroubleCode::DiagnosticTroubleCode(std::uint32_t spn, FailureModeIdentifier fmi, LampStatus lamp) : + DiagnosticProtocol::DiagnosticTroubleCode::DiagnosticTroubleCode(std::uint32_t spn, FailureModeIdentifier failureMode, LampStatus lamp) : suspectParameterNumber(spn), - failureModeIdentifier(static_cast(fmi)), + failureModeIdentifier(failureMode), lampState(lamp) { } - bool DiagnosticProtocol::DiagnosticTroubleCode::operator==(const DiagnosticTroubleCode &obj) + bool DiagnosticProtocol::DiagnosticTroubleCode::operator==(const DiagnosticTroubleCode &obj) const { return ((suspectParameterNumber == obj.suspectParameterNumber) && (failureModeIdentifier == obj.failureModeIdentifier) && (lampState == obj.lampState)); } - std::uint8_t DiagnosticProtocol::DiagnosticTroubleCode::get_occurrance_count() const + std::uint8_t DiagnosticProtocol::DiagnosticTroubleCode::get_occurrence_count() const { - return occuranceCount; + return occurrenceCount; } - DiagnosticProtocol::DiagnosticProtocol(std::shared_ptr internalControlFunction) : + DiagnosticProtocol::DiagnosticProtocol(std::shared_ptr internalControlFunction, NetworkType networkType) : ControlFunctionFunctionalitiesMessageInterface(internalControlFunction), myControlFunction(internalControlFunction), - txFlags(static_cast(TransmitFlags::NumberOfFlags), process_flags, this), - lastDM1SentTimestamp(0), - stopBroadcastNetworkBitfield(0), - lastDM13ReceivedTimestamp(0), - j1939Mode(false) + networkType(networkType), + txFlags(static_cast(TransmitFlags::NumberOfFlags), process_flags, this) { - diagnosticProtocolList.push_back(this); ecuIdentificationFields.resize(static_cast(ECUIdentificationFields::NumberOfFields)); for (auto &ecuIDField : ecuIdentificationFields) @@ -87,176 +80,71 @@ namespace isobus DiagnosticProtocol::~DiagnosticProtocol() { - auto protocolLocation = find(diagnosticProtocolList.begin(), diagnosticProtocolList.end(), this); - - if (nullptr != ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)) - { - // If we're being destructed but have not been deassigned, that is not ideal. - // So, we'll log it here, and try to clean ourselves up. - CANStackLogger::CAN_stack_log(CANStackLogger::LoggingLevel::Warning, "[DP]: DiagnosticProtocol instance is being destroyed without being deassigned first! It is suggested that you deassign the protocol before deleting this object!"); - deregister_all_pgns(); - } - - if (diagnosticProtocolList.end() != protocolLocation) - { - diagnosticProtocolList.erase(protocolLocation); - } - - if (initialized) - { - initialized = false; - CANNetworkManager::CANNetwork.remove_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage22), process_message, this); - CANNetworkManager::CANNetwork.remove_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); - CANNetworkManager::CANNetwork.remove_global_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); - } + terminate(); } - bool DiagnosticProtocol::assign_diagnostic_protocol_to_internal_control_function(std::shared_ptr internalControlFunction) + void DiagnosticProtocol::initialize() { - bool retVal = true; - - for (auto protocolLocation : diagnosticProtocolList) + if (!initialized) { - if (protocolLocation->myControlFunction == internalControlFunction) - { - retVal = false; - break; - } - } + initialized = true; + CANNetworkManager::CANNetwork.add_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage22), process_message, this); + CANNetworkManager::CANNetwork.add_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); + CANNetworkManager::CANNetwork.add_global_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); - if (retVal) - { - DiagnosticProtocol *newProtocol = new DiagnosticProtocol(internalControlFunction); // PGN protocol will check for duplicates, so no worries if there's already a request protocol registered. - ParameterGroupNumberRequestProtocol::assign_pgn_request_protocol_to_internal_control_function(internalControlFunction); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage2), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage3), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage11), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::ProductIdentification), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticProtocolIdentification), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::SoftwareIdentification), process_parameter_group_number_request, newProtocol); - ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(internalControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::ECUIdentificationInformation), process_parameter_group_number_request, newProtocol); - } - return retVal; - } - - bool DiagnosticProtocol::deassign_diagnostic_protocol_to_internal_control_function(std::shared_ptr internalControlFunction) - { - bool retVal = false; - - for (auto protocolLocation = diagnosticProtocolList.begin(); protocolLocation != diagnosticProtocolList.end(); protocolLocation++) - { - if ((*protocolLocation)->myControlFunction == internalControlFunction) - { - retVal = true; - - // First, remove callbacks from PGN requests - (*protocolLocation)->deregister_all_pgns(); - - // Then, remove the instance of the diagnostic protocol - delete *protocolLocation; - break; - } + ParameterGroupNumberRequestProtocol::assign_pgn_request_protocol_to_internal_control_function(myControlFunction); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage2), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage3), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage11), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::ProductIdentification), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticProtocolIdentification), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::SoftwareIdentification), process_parameter_group_number_request, this); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::ECUIdentificationInformation), process_parameter_group_number_request, this); } - return retVal; } - void DiagnosticProtocol::deassign_all_diagnostic_protocol_to_internal_control_functions() + void DiagnosticProtocol::terminate() { - while (0 != diagnosticProtocolList.size()) + if (initialized) { - DiagnosticProtocol *tempProtocol = diagnosticProtocolList.front(); - tempProtocol->deregister_all_pgns(); - delete tempProtocol; // The destructor removes it from the list, so just deleting it is enough to prune it + initialized = false; + deregister_all_pgns(); + CANNetworkManager::CANNetwork.remove_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage22), process_message, this); + CANNetworkManager::CANNetwork.remove_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); + CANNetworkManager::CANNetwork.remove_global_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); } } - DiagnosticProtocol *DiagnosticProtocol::get_diagnostic_protocol_by_internal_control_function(std::shared_ptr internalControlFunction) + void DiagnosticProtocol::update() { - DiagnosticProtocol *retVal = nullptr; - for (auto protocol : diagnosticProtocolList) + if (SystemTiming::time_expired_ms(lastDM13ReceivedTimestamp, DM13_TIMEOUT_MS)) { - if (protocol->myControlFunction == internalControlFunction) - { - retVal = protocol; - break; - } + broadcastState = true; } - return retVal; - } - bool DiagnosticProtocol::parse_j1939_network_states(const CANMessage &message, std::uint32_t &networkStates) - { - bool retVal = false; - - if ((CAN_DATA_LENGTH == message.get_data_length()) && - (static_cast(CANLibParameterGroupNumber::DiagnosticMessage13) == message.get_identifier().get_parameter_group_number())) + if (broadcastState) { - const auto &messageData = message.get_data(); - - for (std::uint8_t i = 0; i < DM13_NUMBER_OF_J1939_NETWORKS; i++) + if (j1939Mode) { - StopStartCommand command = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(J1939NetworkIndicies[i])))); - switch (command) + if (SystemTiming::time_expired_ms(lastDM1SentTimestamp, DM_MAX_FREQUENCY_MS)) { - case StopStartCommand::StopBroadcast: - { - networkStates |= (1 << i); - } - break; - - case StopStartCommand::StartBroadcast: - { - networkStates &= ~(1 << i); - } - break; - - default: - case StopStartCommand::DontCareNoAction: - case StopStartCommand::Reserved: - { - } - break; + txFlags.set_flag(static_cast(TransmitFlags::DM1)); + lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); } } - - // Check current data link - StopStartCommand currentLinkCommand = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(Network::CurrentDataLink)))); - switch (currentLinkCommand) + else { - case StopStartCommand::StopBroadcast: - { - networkStates |= (1 << message.get_can_port_index()); - } - break; - - case StopStartCommand::StartBroadcast: - { - networkStates &= ~(1 << message.get_can_port_index()); - } - break; - - default: - case StopStartCommand::DontCareNoAction: - case StopStartCommand::Reserved: + if ((0 != activeDTCList.size()) && + (SystemTiming::time_expired_ms(lastDM1SentTimestamp, DM_MAX_FREQUENCY_MS))) { + txFlags.set_flag(static_cast(TransmitFlags::DM1)); + lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); } - break; } - retVal = true; - } - return retVal; - } - - void DiagnosticProtocol::initialize(CANLibBadge) - { - if (!initialized) - { - initialized = true; - CANNetworkManager::CANNetwork.add_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage22), process_message, this); - CANNetworkManager::CANNetwork.add_protocol_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); - CANNetworkManager::CANNetwork.add_global_parameter_group_number_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), process_message, this); } + txFlags.process_all_flags(); + ControlFunctionFunctionalitiesMessageInterface.update(); } void DiagnosticProtocol::set_j1939_mode(bool value) @@ -274,7 +162,7 @@ namespace isobus inactiveDTCList.insert(std::end(inactiveDTCList), std::begin(activeDTCList), std::end(activeDTCList)); activeDTCList.clear(); - if (!get_are_broadcasts_stopped_for_channel(myControlFunction->get_can_port())) + if (broadcastState) { txFlags.set_flag(static_cast(TransmitFlags::DM1)); } @@ -290,17 +178,6 @@ namespace isobus softwareIdentificationFields.clear(); } - bool DiagnosticProtocol::get_are_broadcasts_stopped_for_channel(std::uint8_t canChannelIndex) const - { - bool retVal = false; - - if ((canChannelIndex < CAN_PORT_MAXIMUM) && (canChannelIndex < DM13_NUMBER_OF_J1939_NETWORKS)) - { - retVal = (0 != (static_cast(1 << canChannelIndex) & stopBroadcastNetworkBitfield)); - } - return retVal; - } - void DiagnosticProtocol::set_ecu_id_field(ECUIdentificationFields field, std::string value) { if (field <= ECUIdentificationFields::NumberOfFields) @@ -325,17 +202,17 @@ namespace isobus if (inactiveDTCList.end() != inactiveLocation) { - inactiveLocation->occuranceCount++; + inactiveLocation->occurrenceCount++; activeDTCList.push_back(*inactiveLocation); inactiveDTCList.erase(inactiveLocation); } else { activeDTCList.push_back(dtc); - activeDTCList[activeDTCList.size() - 1].occuranceCount = 1; + activeDTCList[activeDTCList.size() - 1].occurrenceCount = 1; if ((SystemTiming::get_time_elapsed_ms(lastDM1SentTimestamp) > DM_MAX_FREQUENCY_MS) && - (!get_are_broadcasts_stopped_for_channel(myControlFunction->get_can_port()))) + (broadcastState)) { txFlags.set_flag(static_cast(TransmitFlags::DM1)); lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); @@ -436,46 +313,9 @@ namespace isobus softwareIdentificationFields[index] = value; } - bool DiagnosticProtocol::suspend_broadcasts(std::uint8_t canChannelIndex, std::shared_ptr sourceControlFunction, std::uint16_t suspendTime_seconds) + bool DiagnosticProtocol::suspend_broadcasts(std::uint16_t suspendTime_seconds) { - bool retVal = false; - - if ((nullptr != sourceControlFunction) && - (canChannelIndex == sourceControlFunction->get_can_port())) - { - retVal = send_dm13_announce_suspension(sourceControlFunction, suspendTime_seconds); - } - return retVal; - } - - void DiagnosticProtocol::update(CANLibBadge) - { - if (SystemTiming::time_expired_ms(lastDM13ReceivedTimestamp, DM13_TIMEOUT_MS)) - { - stopBroadcastNetworkBitfield = 0; - } - if (!get_are_broadcasts_stopped_for_channel(myControlFunction->get_can_port())) - { - if (j1939Mode) - { - if (SystemTiming::time_expired_ms(lastDM1SentTimestamp, DM_MAX_FREQUENCY_MS)) - { - txFlags.set_flag(static_cast(TransmitFlags::DM1)); - lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); - } - } - else - { - if ((0 != activeDTCList.size()) && - (SystemTiming::time_expired_ms(lastDM1SentTimestamp, DM_MAX_FREQUENCY_MS))) - { - txFlags.set_flag(static_cast(TransmitFlags::DM1)); - lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); - } - } - } - txFlags.process_all_flags(); - ControlFunctionFunctionalitiesMessageInterface.update(); + return send_dm13_announce_suspension(suspendTime_seconds); } std::uint8_t DiagnosticProtocol::convert_flash_state_to_byte(FlashState flash) @@ -564,7 +404,7 @@ namespace isobus { lampOn = true; } - else if (dtc.lampState == LampStatus::MalfuctionIndicatorLampSlowFlash) + else if (dtc.lampState == LampStatus::MalfunctionIndicatorLampSlowFlash) { lampOn = true; if (flash != FlashState::Fast) @@ -669,7 +509,7 @@ namespace isobus { lampOn = true; } - else if (dtc.lampState == LampStatus::MalfuctionIndicatorLampSlowFlash) + else if (dtc.lampState == LampStatus::MalfunctionIndicatorLampSlowFlash) { lampOn = true; if (flash != FlashState::Fast) @@ -737,18 +577,6 @@ namespace isobus } } - bool DiagnosticProtocol::protocol_transmit_message(std::uint32_t, - const std::uint8_t *, - std::uint32_t, - std::shared_ptr, - std::shared_ptr, - TransmitCompleteCallback, - void *, - DataChunkCallback) - { - return false; - } - bool DiagnosticProtocol::send_diagnostic_message_1() { bool retVal = false; @@ -816,8 +644,8 @@ namespace isobus { buffer[2 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast(activeDTCList[i].suspectParameterNumber & 0xFF); buffer[3 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast((activeDTCList[i].suspectParameterNumber >> 8) & 0xFF); - buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((activeDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | static_cast(activeDTCList[i].failureModeIdentifier & 0x1F)); - buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (activeDTCList[i].occuranceCount & 0x7F); + buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((activeDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(activeDTCList[i].failureModeIdentifier) & 0x1F)); + buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (activeDTCList[i].occurrenceCount & 0x7F); } if (payloadSize < CAN_DATA_LENGTH) @@ -904,8 +732,8 @@ namespace isobus { buffer[2 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast(inactiveDTCList[i].suspectParameterNumber & 0xFF); buffer[3 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast((inactiveDTCList[i].suspectParameterNumber >> 8) & 0xFF); - buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((inactiveDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | static_cast(inactiveDTCList[i].failureModeIdentifier & 0x1F)); - buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (inactiveDTCList[i].occuranceCount & 0x7F); + buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((inactiveDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(inactiveDTCList[i].failureModeIdentifier) & 0x1F)); + buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (inactiveDTCList[i].occurrenceCount & 0x7F); } if (payloadSize < CAN_DATA_LENGTH) @@ -945,7 +773,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::send_dm13_announce_suspension(std::shared_ptr sourceControlFunction, std::uint16_t suspendTime_seconds) + bool DiagnosticProtocol::send_dm13_announce_suspension(std::uint16_t suspendTime_seconds) { const std::array buffer = { 0xFF, @@ -959,8 +787,8 @@ namespace isobus }; return CANNetworkManager::CANNetwork.send_can_message(static_cast(CANLibParameterGroupNumber::DiagnosticMessage13), buffer.data(), - 8, - sourceControlFunction); + buffer.size(), + myControlFunction); } bool DiagnosticProtocol::send_ecu_identification() @@ -1085,7 +913,7 @@ namespace isobus { case static_cast(CANLibParameterGroupNumber::DiagnosticMessage13): { - if (parse_j1939_network_states(message, stopBroadcastNetworkBitfield)) + if (parse_j1939_network_states(message)) { lastDM13ReceivedTimestamp = SystemTiming::get_timestamp_ms(); } @@ -1118,7 +946,7 @@ namespace isobus for (auto dtc = activeDTCList.begin(); dtc != activeDTCList.end(); dtc++) { if ((tempDM22Data.suspectParameterNumber == dtc->suspectParameterNumber) && - (tempDM22Data.failureModeIdentifier == dtc->failureModeIdentifier)) + (tempDM22Data.failureModeIdentifier == static_cast(dtc->failureModeIdentifier))) { inactiveDTCList.push_back(*dtc); activeDTCList.erase(dtc); @@ -1139,7 +967,7 @@ namespace isobus for (auto &dtc : inactiveDTCList) { if ((tempDM22Data.suspectParameterNumber == dtc.suspectParameterNumber) && - (tempDM22Data.failureModeIdentifier == dtc.failureModeIdentifier)) + (tempDM22Data.failureModeIdentifier == static_cast(dtc.failureModeIdentifier))) { // The DTC was active, but is inactive now, so we NACK with the proper reason tempDM22Data.nackIndicator = static_cast(DM22NegativeAcknowledgeIndicator::DTCNoLongerActive); @@ -1163,7 +991,7 @@ namespace isobus for (auto dtc = inactiveDTCList.begin(); dtc != inactiveDTCList.end(); dtc++) { if ((tempDM22Data.suspectParameterNumber == dtc->suspectParameterNumber) && - (tempDM22Data.failureModeIdentifier == dtc->failureModeIdentifier)) + (tempDM22Data.failureModeIdentifier == static_cast(dtc->failureModeIdentifier))) { inactiveDTCList.erase(dtc); wasDTCCleared = true; @@ -1183,10 +1011,10 @@ namespace isobus for (auto &dtc : activeDTCList) { if ((tempDM22Data.suspectParameterNumber == dtc.suspectParameterNumber) && - (tempDM22Data.failureModeIdentifier == dtc.failureModeIdentifier)) + (tempDM22Data.failureModeIdentifier == static_cast(dtc.failureModeIdentifier))) { // The DTC was inactive, but is active now, so we NACK with the proper reason - tempDM22Data.nackIndicator = static_cast(DM22NegativeAcknowledgeIndicator::DTCUNoLongerPreviouslyActive); + tempDM22Data.nackIndicator = static_cast(DM22NegativeAcknowledgeIndicator::DTCNoLongerPreviouslyActive); break; } } @@ -1227,6 +1055,54 @@ namespace isobus } } + bool DiagnosticProtocol::parse_j1939_network_states(const CANMessage &message) + { + bool retVal = false; + + if ((CAN_DATA_LENGTH == message.get_data_length()) && + (static_cast(CANLibParameterGroupNumber::DiagnosticMessage13) == message.get_identifier().get_parameter_group_number())) + { + const auto &messageData = message.get_data(); + + auto command = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(networkType)))); + switch (command) + { + case StopStartCommand::StopBroadcast: + broadcastState = false; + break; + + case StopStartCommand::StartBroadcast: + broadcastState = true; + break; + + case StopStartCommand::DontCareNoAction: + case StopStartCommand::Reserved: + default: + break; + } + + // Check current data link + command = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(NetworkType::CurrentDataLink)))); + switch (command) + { + case StopStartCommand::StopBroadcast: + broadcastState = false; + break; + + case StopStartCommand::StartBroadcast: + broadcastState = true; + break; + + case StopStartCommand::DontCareNoAction: + case StopStartCommand::Reserved: + default: + break; + } + retVal = true; + } + return retVal; + } + bool DiagnosticProtocol::process_parameter_group_number_request(std::uint32_t parameterGroupNumber, std::shared_ptr requestingControlFunction, bool &acknowledge, diff --git a/test/diagnostic_protocol_tests.cpp b/test/diagnostic_protocol_tests.cpp index f06a314d..ddb3b5c4 100644 --- a/test/diagnostic_protocol_tests.cpp +++ b/test/diagnostic_protocol_tests.cpp @@ -4,49 +4,16 @@ using namespace isobus; -TEST(DIAGNOSTIC_PROTOCOL_TESTS, DM13TestNetworkParsing) -{ - std::uint32_t testNetworkStates = 0; - CANIdentifier testID(CANIdentifier::Type::Extended, - 0xDF00, - CANIdentifier::CANPriority::PriorityDefault6, - 0xFF, - 0x80); - CANMessage testDM13Message(0); - testDM13Message.set_identifier(testID); - testDM13Message.set_data_size(8); - EXPECT_EQ(true, DiagnosticProtocol::parse_j1939_network_states(testDM13Message, testNetworkStates)); -} - -TEST(DIAGNOSTIC_PROTOCOL_TESTS, TestInvalidDM13Rejection) -{ - std::uint32_t testNetworkStates = 0; - CANIdentifier testID(CANIdentifier::Type::Extended, - 0xDF00, - CANIdentifier::CANPriority::PriorityDefault6, - 0xFF, - 0x80); - CANMessage testDM13Message(0); - testDM13Message.set_identifier(testID); - testDM13Message.set_data_size(4); - EXPECT_EQ(false, DiagnosticProtocol::parse_j1939_network_states(testDM13Message, testNetworkStates)); -} - TEST(DIAGNOSTIC_PROTOCOL_TESTS, CreateAndDestroyProtocolObjects) { NAME TestDeviceNAME(0); - auto TestInternalECU = InternalControlFunction::create(TestDeviceNAME, 0x1C, 0); - DiagnosticProtocol::assign_diagnostic_protocol_to_internal_control_function(TestInternalECU); - DiagnosticProtocol *diagnosticProtocol = DiagnosticProtocol::get_diagnostic_protocol_by_internal_control_function(TestInternalECU); - EXPECT_NE(nullptr, diagnosticProtocol); + auto diagnosticProtocol = std::make_unique(TestInternalECU); + EXPECT_NO_THROW(diagnosticProtocol->initialize()); - if (nullptr != diagnosticProtocol) - { - EXPECT_NO_THROW(DiagnosticProtocol::deassign_all_diagnostic_protocol_to_internal_control_functions()); - EXPECT_EQ(nullptr, DiagnosticProtocol::get_diagnostic_protocol_by_internal_control_function(TestInternalECU)); - } + EXPECT_NO_THROW(diagnosticProtocol->terminate()); + diagnosticProtocol.reset(); //! @todo try to reduce the reference count, such that that we don't use a control function after it is destroyed ASSERT_TRUE(TestInternalECU->destroy(2)); From aeb26c8fd48836f7be415c4dbe202e3b6fec98e3 Mon Sep 17 00:00:00 2001 From: Daan Steenbergen Date: Fri, 23 Jun 2023 20:30:40 +0200 Subject: [PATCH 4/7] [DP] Apply reference-to-const and const functions where possible - Remove redundant case ' clauses' - Remove redundant parenthesis These changes were suggested by SonarCloud --- .../isobus/isobus_diagnostic_protocol.hpp | 32 ++--- isobus/src/isobus_diagnostic_protocol.cpp | 119 ++++++++---------- 2 files changed, 67 insertions(+), 84 deletions(-) diff --git a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp index 758dfc96..92ff2af0 100644 --- a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp +++ b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp @@ -258,7 +258,7 @@ namespace isobus /// @attention Do not include the "*" character in your field values /// @param[in] field The field to set /// @param[in] value The string value associated with the ECU ID field - void set_ecu_id_field(ECUIdentificationFields field, std::string value); + void set_ecu_id_field(ECUIdentificationFields field, const std::string &value); /// @brief Adds a DTC to the active list, or removes one from the active list /// @details When you call this function with a DTC and `true`, it will be added to the DM1 message. @@ -281,20 +281,20 @@ namespace isobus /// make the product globally unique. /// @param value The ascii product identification code, up to 50 characters long /// @returns true if the value was set, false if the string is too long - bool set_product_identification_code(std::string value); + bool set_product_identification_code(const std::string &value); /// @brief Sets the product identification brand used in the diagnostic protocol "Product Identification" message (PGN 0xFC8D) /// @details The product identification brand specifies the brand of a product. The combination of the product ID code and brand /// shall make the product unique in the world. /// @param value The ascii product brand, up to 50 characters long /// @returns true if the value was set, false if the string is too long - bool set_product_identification_brand(std::string value); + bool set_product_identification_brand(const std::string &value); /// @brief Sets the product identification model used in the diagnostic protocol "Product Identification" message (PGN 0xFC8D) /// @details The product identification model specifies a unique product within a brand. /// @param value The ascii model string, up to 50 characters /// @returns true if the value was set, false if the string is too long - bool set_product_identification_model(std::string value); + bool set_product_identification_model(const std::string &value); /// @brief Adds an ascii string to this internal control function's software ID /// @details Use this to identify the software version of your application. @@ -306,12 +306,12 @@ namespace isobus /// You can remove a field by setting it to "" /// @param[in] index The field index to set /// @param[in] value The software ID string to add - void set_software_id_field(std::uint32_t index, std::string value); + void set_software_id_field(std::uint32_t index, const std::string &value); /// @brief Informs the diagnostic protocol that you are going to suspend broadcasts /// @param[in] suspendTime_seconds If you know the time for which broadcasts will be suspended, put it here, otherwise 0xFFFF /// @returns `true` if the message was sent, otherwise `false` - bool suspend_broadcasts(std::uint16_t suspendTime_seconds = 0xFFFF); + bool suspend_broadcasts(std::uint16_t suspendTime_seconds = 0xFFFF) const; private: /// @brief Lists the different lamps in J1939-73 @@ -376,7 +376,7 @@ namespace isobus /// @brief A utility function to get the CAN representation of a FlashState /// @param flash The flash state to convert /// @returns The two bit lamp state for CAN - std::uint8_t convert_flash_state_to_byte(FlashState flash); + std::uint8_t convert_flash_state_to_byte(FlashState flash) const; /// @brief A utility function that will clean up PGN registrations void deregister_all_pgns(); @@ -387,7 +387,7 @@ namespace isobus /// @param[in] targetLamp The lamp to find the status of /// @param[out] flash How the lamp should be flashing /// @param[out] lampOn If the lamp state is on for any DTC - void get_active_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn); + void get_active_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) const; /// @brief This is a way to find the overall lamp states to report /// @details This searches the inactive DTC list to find if a lamp is on or off, and to find the overall flash state for that lamp. @@ -395,36 +395,36 @@ namespace isobus /// @param[in] targetLamp The lamp to find the status of /// @param[out] flash How the lamp should be flashing /// @param[out] lampOn If the lamp state is on for any DTC - void get_inactive_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn); + void get_inactive_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) const; /// @brief Sends a DM1 encoded CAN message /// @returns true if the message was sent, otherwise false - bool send_diagnostic_message_1(); + bool send_diagnostic_message_1() const; /// @brief Sends a DM2 encoded CAN message /// @returns true if the message was sent, otherwise false - bool send_diagnostic_message_2(); + bool send_diagnostic_message_2() const; /// @brief Sends a message that identifies which diagnostic protocols are supported /// @returns true if the message was sent, otherwise false - bool send_diagnostic_protocol_identification(); + bool send_diagnostic_protocol_identification() const; /// @brief Sends the DM13 to alert network devices of impending suspended broadcasts /// @param suspendTime_seconds The number of seconds that the broadcast will be suspended for /// @returns `true` if the message was sent, otherwise `false` - bool send_dm13_announce_suspension(std::uint16_t suspendTime_seconds); + bool send_dm13_announce_suspension(std::uint16_t suspendTime_seconds) const; /// @brief Sends the ECU ID message /// @returns true if the message was sent - bool send_ecu_identification(); + bool send_ecu_identification() const; /// @brief Sends the product identification message (PGN 0xFC8D) /// @returns true if the message was sent, otherwise false - bool send_product_identification(); + bool send_product_identification() const; /// @brief Sends the software ID message /// @returns true if the message was sent, otherwise false - bool send_software_identification(); + bool send_software_identification() const; /// @brief Processes any DM22 responses from the queue /// @details We queue responses so that we can do Tx retries if needed diff --git a/isobus/src/isobus_diagnostic_protocol.cpp b/isobus/src/isobus_diagnostic_protocol.cpp index 215bd8fc..c5888da5 100644 --- a/isobus/src/isobus_diagnostic_protocol.cpp +++ b/isobus/src/isobus_diagnostic_protocol.cpp @@ -178,7 +178,7 @@ namespace isobus softwareIdentificationFields.clear(); } - void DiagnosticProtocol::set_ecu_id_field(ECUIdentificationFields field, std::string value) + void DiagnosticProtocol::set_ecu_id_field(ECUIdentificationFields field, const std::string &value) { if (field <= ECUIdentificationFields::NumberOfFields) { @@ -212,7 +212,7 @@ namespace isobus activeDTCList[activeDTCList.size() - 1].occurrenceCount = 1; if ((SystemTiming::get_time_elapsed_ms(lastDM1SentTimestamp) > DM_MAX_FREQUENCY_MS) && - (broadcastState)) + broadcastState) { txFlags.set_flag(static_cast(TransmitFlags::DM1)); lastDM1SentTimestamp = SystemTiming::get_timestamp_ms(); @@ -261,7 +261,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::set_product_identification_code(std::string value) + bool DiagnosticProtocol::set_product_identification_code(const std::string &value) { bool retVal = false; @@ -273,7 +273,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::set_product_identification_brand(std::string value) + bool DiagnosticProtocol::set_product_identification_brand(const std::string &value) { bool retVal = false; @@ -285,7 +285,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::set_product_identification_model(std::string value) + bool DiagnosticProtocol::set_product_identification_model(const std::string &value) { bool retVal = false; @@ -297,28 +297,25 @@ namespace isobus return retVal; } - void DiagnosticProtocol::set_software_id_field(std::uint32_t index, std::string value) + void DiagnosticProtocol::set_software_id_field(std::uint32_t index, const std::string &value) { if (index >= softwareIdentificationFields.size()) { softwareIdentificationFields.resize(index + 1); } - else if ("" == value) + else if (("" == value) && (index == softwareIdentificationFields.size())) { - if (index == softwareIdentificationFields.size()) - { - softwareIdentificationFields.pop_back(); - } + softwareIdentificationFields.pop_back(); } softwareIdentificationFields[index] = value; } - bool DiagnosticProtocol::suspend_broadcasts(std::uint16_t suspendTime_seconds) + bool DiagnosticProtocol::suspend_broadcasts(std::uint16_t suspendTime_seconds) const { return send_dm13_announce_suspension(suspendTime_seconds); } - std::uint8_t DiagnosticProtocol::convert_flash_state_to_byte(FlashState flash) + std::uint8_t DiagnosticProtocol::convert_flash_state_to_byte(FlashState flash) const { std::uint8_t retVal = 0; @@ -336,7 +333,6 @@ namespace isobus } break; - case FlashState::Solid: default: { retVal = 0x03; @@ -367,7 +363,7 @@ namespace isobus } } - void DiagnosticProtocol::get_active_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) + void DiagnosticProtocol::get_active_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) const { flash = FlashState::Solid; lampOn = false; @@ -465,14 +461,12 @@ namespace isobus break; default: - { - } - break; + break; } } } - void DiagnosticProtocol::get_inactive_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) + void DiagnosticProtocol::get_inactive_list_lamp_state_and_flash_state(Lamps targetLamp, FlashState &flash, bool &lampOn) const { flash = FlashState::Solid; lampOn = false; @@ -570,20 +564,18 @@ namespace isobus break; default: - { - } - break; + break; } } } - bool DiagnosticProtocol::send_diagnostic_message_1() + bool DiagnosticProtocol::send_diagnostic_message_1() const { bool retVal = false; if (nullptr != myControlFunction) { - std::uint16_t payloadSize = (activeDTCList.size() * DM_PAYLOAD_BYTES_PER_DTC) + 2; // 2 Bytes (0 and 1) are reserved + std::uint16_t payloadSize = static_cast(activeDTCList.size() * DM_PAYLOAD_BYTES_PER_DTC) + 2; // 2 Bytes (0 and 1) are reserved if (payloadSize <= MAX_PAYLOAD_SIZE_BYTES) { @@ -603,19 +595,19 @@ namespace isobus get_active_list_lamp_state_and_flash_state(Lamps::AmberWarningLamp, tempLampFlashState, tempLampState); /// Encode amber warning lamp state and flash - buffer[0] |= (tempLampState << 2); + buffer[0] |= (static_cast(tempLampState) << 2); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 2); get_active_list_lamp_state_and_flash_state(Lamps::RedStopLamp, tempLampFlashState, tempLampState); /// Encode red stop lamp state and flash - buffer[0] |= (tempLampState << 4); + buffer[0] |= (static_cast(tempLampState) << 4); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 4); get_active_list_lamp_state_and_flash_state(Lamps::MalfunctionIndicatorLamp, tempLampFlashState, tempLampState); /// Encode malfunction indicator lamp state and flash - buffer[0] |= (tempLampState << 6); + buffer[0] |= (static_cast(tempLampState) << 6); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 6); } else @@ -625,7 +617,7 @@ namespace isobus buffer[1] = 0xFF; } - if (0 == activeDTCList.size()) + if (activeDTCList.empty()) { buffer[2] = 0x00; buffer[3] = 0x00; @@ -644,7 +636,7 @@ namespace isobus { buffer[2 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast(activeDTCList[i].suspectParameterNumber & 0xFF); buffer[3 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast((activeDTCList[i].suspectParameterNumber >> 8) & 0xFF); - buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((activeDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(activeDTCList[i].failureModeIdentifier) & 0x1F)); + buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (static_cast(((activeDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(activeDTCList[i].failureModeIdentifier) & 0x1F)); buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (activeDTCList[i].occurrenceCount & 0x7F); } @@ -665,13 +657,13 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::send_diagnostic_message_2() + bool DiagnosticProtocol::send_diagnostic_message_2() const { bool retVal = false; if (nullptr != myControlFunction) { - std::uint16_t payloadSize = (inactiveDTCList.size() * DM_PAYLOAD_BYTES_PER_DTC) + 2; // 2 Bytes (0 and 1) are reserved or used for lamp + flash + std::uint16_t payloadSize = static_cast(inactiveDTCList.size() * DM_PAYLOAD_BYTES_PER_DTC) + 2; // 2 Bytes (0 and 1) are reserved or used for lamp + flash if (payloadSize <= MAX_PAYLOAD_SIZE_BYTES) { @@ -691,19 +683,19 @@ namespace isobus get_inactive_list_lamp_state_and_flash_state(Lamps::AmberWarningLamp, tempLampFlashState, tempLampState); /// Encode amber warning lamp state and flash - buffer[0] |= (tempLampState << 2); + buffer[0] |= (static_cast(tempLampState) << 2); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 2); get_inactive_list_lamp_state_and_flash_state(Lamps::RedStopLamp, tempLampFlashState, tempLampState); /// Encode red stop lamp state and flash - buffer[0] |= (tempLampState << 4); + buffer[0] |= (static_cast(tempLampState) << 4); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 4); get_inactive_list_lamp_state_and_flash_state(Lamps::MalfunctionIndicatorLamp, tempLampFlashState, tempLampState); /// Encode malfunction indicator lamp state and flash - buffer[0] |= (tempLampState << 6); + buffer[0] |= (static_cast(tempLampState) << 6); buffer[1] |= (convert_flash_state_to_byte(tempLampFlashState) << 6); } else @@ -713,7 +705,7 @@ namespace isobus buffer[1] = 0xFF; } - if (0 == inactiveDTCList.size()) + if (inactiveDTCList.empty()) { buffer[2] = 0x00; buffer[3] = 0x00; @@ -732,7 +724,7 @@ namespace isobus { buffer[2 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast(inactiveDTCList[i].suspectParameterNumber & 0xFF); buffer[3 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = static_cast((inactiveDTCList[i].suspectParameterNumber >> 8) & 0xFF); - buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = ((static_cast((inactiveDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(inactiveDTCList[i].failureModeIdentifier) & 0x1F)); + buffer[4 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (static_cast(((inactiveDTCList[i].suspectParameterNumber >> 16) & 0xFF) << 5) | (static_cast(inactiveDTCList[i].failureModeIdentifier) & 0x1F)); buffer[5 + (DM_PAYLOAD_BYTES_PER_DTC * i)] = (inactiveDTCList[i].occurrenceCount & 0x7F); } @@ -753,7 +745,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::send_diagnostic_protocol_identification() + bool DiagnosticProtocol::send_diagnostic_protocol_identification() const { bool retVal = false; @@ -773,7 +765,7 @@ namespace isobus return retVal; } - bool DiagnosticProtocol::send_dm13_announce_suspension(std::uint16_t suspendTime_seconds) + bool DiagnosticProtocol::send_dm13_announce_suspension(std::uint16_t suspendTime_seconds) const { const std::array buffer = { 0xFF, @@ -791,11 +783,11 @@ namespace isobus myControlFunction); } - bool DiagnosticProtocol::send_ecu_identification() + bool DiagnosticProtocol::send_ecu_identification() const { std::string ecuIdString = ""; - for (auto &stringComponent : ecuIdentificationFields) + for (const auto &stringComponent : ecuIdentificationFields) { ecuIdString.append(stringComponent); } @@ -807,7 +799,7 @@ namespace isobus myControlFunction); } - bool DiagnosticProtocol::send_product_identification() + bool DiagnosticProtocol::send_product_identification() const { std::string productIdString = productIdentificationCode + "*" + productIdentificationBrand + "*" + productIdentificationModel + "*"; std::vector buffer(productIdString.begin(), productIdString.end()); @@ -818,19 +810,20 @@ namespace isobus myControlFunction); } - bool DiagnosticProtocol::send_software_identification() + bool DiagnosticProtocol::send_software_identification() const { bool retVal = false; - if (0 != softwareIdentificationFields.size()) + if (!softwareIdentificationFields.empty()) { std::string softIDString = ""; - for (auto softIdString = softwareIdentificationFields.begin(); softIdString != softwareIdentificationFields.end(); softIdString++) - { - softIDString.append(*softIdString); - softIDString.append("*"); - } + std::for_each(softwareIdentificationFields.begin(), + softwareIdentificationFields.end(), + [&softIDString](const std::string &field) { + softIDString.append(field); + softIDString.append("*"); + }); std::vector buffer(softIDString.begin(), softIDString.end()); retVal = CANNetworkManager::CANNetwork.send_can_message(static_cast(CANLibParameterGroupNumber::SoftwareIdentification), @@ -845,7 +838,7 @@ namespace isobus { bool retVal = false; - if (0 != dm22ResponseQueue.size()) + if (!dm22ResponseQueue.empty()) { std::size_t numberOfMessage = dm22ResponseQueue.size(); @@ -905,9 +898,9 @@ namespace isobus void DiagnosticProtocol::process_message(const CANMessage &message) { - if ((((nullptr == message.get_destination_control_function()) && - (BROADCAST_CAN_ADDRESS == message.get_identifier().get_destination_address())) || - (message.get_destination_control_function() == myControlFunction))) + if (((nullptr == message.get_destination_control_function()) && + (BROADCAST_CAN_ADDRESS == message.get_identifier().get_destination_address())) || + (message.get_destination_control_function() == myControlFunction)) { switch (message.get_identifier().get_parameter_group_number()) { @@ -964,7 +957,7 @@ namespace isobus tempDM22Data.nack = true; // Since we didn't find the DTC in the active list, we check the inactive to determine the proper NACK reason - for (auto &dtc : inactiveDTCList) + for (const auto &dtc : inactiveDTCList) { if ((tempDM22Data.suspectParameterNumber == dtc.suspectParameterNumber) && (tempDM22Data.failureModeIdentifier == static_cast(dtc.failureModeIdentifier))) @@ -1008,7 +1001,7 @@ namespace isobus tempDM22Data.nack = true; // Since we didn't find the DTC in the inactive list, we check the active to determine the proper NACK reason - for (auto &dtc : activeDTCList) + for (const auto &dtc : activeDTCList) { if ((tempDM22Data.suspectParameterNumber == dtc.suspectParameterNumber) && (tempDM22Data.failureModeIdentifier == static_cast(dtc.failureModeIdentifier))) @@ -1031,18 +1024,14 @@ namespace isobus break; default: - { - } - break; + break; } } } break; default: - { - } - break; + break; } } } @@ -1075,8 +1064,6 @@ namespace isobus broadcastState = true; break; - case StopStartCommand::DontCareNoAction: - case StopStartCommand::Reserved: default: break; } @@ -1093,8 +1080,6 @@ namespace isobus broadcastState = true; break; - case StopStartCommand::DontCareNoAction: - case StopStartCommand::Reserved: default: break; } @@ -1196,7 +1181,7 @@ namespace isobus { if (nullptr != parentPointer) { - DiagnosticProtocol *parent = reinterpret_cast(parentPointer); + auto *parent = reinterpret_cast(parentPointer); bool transmitSuccessful = false; switch (flag) @@ -1237,9 +1222,7 @@ namespace isobus break; default: - { - } - break; + break; } if (false == transmitSuccessful) From 0c269c929bba86b61282db907bf952614a9418c6 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Wed, 28 Jun 2023 23:06:09 -0500 Subject: [PATCH 5/7] [DP]: Diagnostic Protocol tests and various enhancements Added numerous DP unit tests. Fixed DP example not saving the periodic event dispatcher. Added example use of ECUID Type. Added a way to get the broadcast state and init state of the diagnostic protocol. Added processing of the suspend type for DM13 to allow for custom broadcast suspension times. Added support for PGN requests for the DM1 PGN. Fixed issues with ECU ID sending too many fields when operating in J1939 mode. Enhanced message queuing for requests for ECU and software ID. Fixed busload unit test which was affected by adding DP tests. Slightly increased the timeout on language command timestamp unit test. Fixed an intermittent issue with TC unit tests that came from changing to shared ptr for control functions due to the tests claiming the same address with no network manager updates between destroying a partner and claiming a new one. --- examples/diagnostic_protocol/main.cpp | 3 +- .../isobus/isobus_diagnostic_protocol.hpp | 25 +- isobus/src/isobus_diagnostic_protocol.cpp | 141 ++- test/core_network_management_tests.cpp | 1 - test/diagnostic_protocol_tests.cpp | 957 ++++++++++++++++++ test/language_command_interface_tests.cpp | 2 +- test/tc_client_tests.cpp | 12 +- 7 files changed, 1120 insertions(+), 21 deletions(-) diff --git a/examples/diagnostic_protocol/main.cpp b/examples/diagnostic_protocol/main.cpp index 330e63b9..770d72e3 100644 --- a/examples/diagnostic_protocol/main.cpp +++ b/examples/diagnostic_protocol/main.cpp @@ -83,7 +83,7 @@ int main() // Important: we need to update the diagnostic protocol using the hardware interface periodic update event, // otherwise the diagnostic protocol will not be able to update its internal state. - isobus::CANHardwareInterface::get_periodic_update_event_dispatcher().add_listener([&diagnosticProtocol]() { + auto listenerHandle = isobus::CANHardwareInterface::get_periodic_update_event_dispatcher().add_listener([&diagnosticProtocol]() { diagnosticProtocol.update(); }); @@ -107,6 +107,7 @@ int main() diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::ManufacturerName, "None"); diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::PartNumber, "1234"); diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::SerialNumber, "1"); + diagnosticProtocol.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::Type, "AgISOStack"); // Let's say that our ECU has the capability of a universal terminal working set (as an example) and // contains weak internal bus termination. diff --git a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp index 92ff2af0..689ba750 100644 --- a/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp +++ b/isobus/include/isobus/isobus/isobus_diagnostic_protocol.hpp @@ -3,7 +3,7 @@ /// /// @brief A protocol that handles the ISO 11783-12 Diagnostic Protocol and some J1939 DMs. /// @details This protocol manages many of the messages defined in ISO 11783-12 -/// and a subset of the messages defined in SAE J1939-73. +/// and a subset of the messages defined in SAE J1939-73 and SAE J1939-71. /// The ISO-11783 definition of some of these is based on the J1939 definition with some tweaks. /// You can select if you want the protocol to behave like J1939 by calling set_j1939_mode. /// One of the messages this protocol supports is the DM1 message. @@ -119,6 +119,8 @@ namespace isobus DiagnosticProtocolID, ///< A flag to manage sending the Diagnostic protocol ID message ProductIdentification, ///< A flag to manage sending the product identification message DM22, ///< Process queued up DM22 responses + ECUIdentification, ///< A flag to manage sending the ECU ID message + SoftwareIdentification, ///< A flag to manage sending the software ID message NumberOfFlags ///< The number of flags in the enum }; @@ -212,7 +214,7 @@ namespace isobus /// @brief The constructor for this protocol /// @param[in] internalControlFunction The internal control function that owns this protocol and will be used to send messages /// @param[in] networkType The type of diagnostic network that this protocol will reflect - explicit DiagnosticProtocol(std::shared_ptr internalControlFunction, NetworkType networkType = NetworkType::ProprietaryNetwork1); + DiagnosticProtocol(std::shared_ptr internalControlFunction, NetworkType networkType = NetworkType::ProprietaryNetwork1); /// @brief The destructor for this protocol ~DiagnosticProtocol(); @@ -220,6 +222,10 @@ namespace isobus /// @brief The protocol's initializer function void initialize(); + /// @brief Returns if the protocol has been initialized + /// @returns true if the protocol has been initialized, otherwise false + bool get_initialized() const; + /// @brief The protocol's terminate function void terminate(); @@ -253,9 +259,9 @@ namespace isobus /// @brief Sets one of the ECU identification strings for the ECU ID message /// @details See ECUIdentificationFields for a brief description of the fields - /// @note The fields in this message are optional and separated by an ASCII �*�. It is not necessary to include parametric + /// @note The fields in this message are optional and separated by an ASCII "*". It is not necessary to include parametric /// data for all fields. Any additional ECU identification fields defined in the future will be appended at the end. - /// @attention Do not include the "*" character in your field values + /// @attention Do not include the "*" character in your field values and only use HardwareID when not in J1939 mode. /// @param[in] field The field to set /// @param[in] value The string value associated with the ECU ID field void set_ecu_id_field(ECUIdentificationFields field, const std::string &value); @@ -309,9 +315,14 @@ namespace isobus void set_software_id_field(std::uint32_t index, const std::string &value); /// @brief Informs the diagnostic protocol that you are going to suspend broadcasts + /// @details When you call this, DM1 and other broadcasts that come from this protocol will be stopped for the duration specified. /// @param[in] suspendTime_seconds If you know the time for which broadcasts will be suspended, put it here, otherwise 0xFFFF /// @returns `true` if the message was sent, otherwise `false` - bool suspend_broadcasts(std::uint16_t suspendTime_seconds = 0xFFFF) const; + bool suspend_broadcasts(std::uint16_t suspendTime_seconds = 0xFFFF); + + /// @brief Gets the current broadcast state for the connected network type + /// @returns True if broadcasts are currently allowed, false if broadcasts are suspended for the connected network + bool get_broadcast_state() const; private: /// @brief Lists the different lamps in J1939-73 @@ -367,6 +378,7 @@ namespace isobus static constexpr std::uint32_t DM13_HOLD_SIGNAL_TRANSMIT_INTERVAL_MS = 5000; ///< Defined in 5.7.13.13 SPN 1236 static constexpr std::uint32_t DM13_TIMEOUT_MS = 6000; ///< The timeout in 5.7.13 after which nodes shall revert back to the normal broadcast state static constexpr std::uint16_t MAX_PAYLOAD_SIZE_BYTES = 1785; ///< DM 1 and 2 are limited to the BAM message max, because ETP does not allow global destinations + static constexpr std::uint16_t MAX_DM13_CUSTOM_SUSPEND_TIME_MS = 64255; ///< The max valid value for a DM13 suspension time in milliseconds static constexpr std::uint8_t DM_PAYLOAD_BYTES_PER_DTC = 4; ///< The number of payload bytes per DTC that gets encoded into the messages static constexpr std::uint8_t PRODUCT_IDENTIFICATION_MAX_STRING_LENGTH = 50; ///< The max string length allowed in the fields of product ID, as defined in ISO 11783-12 static constexpr std::uint8_t DM13_NUMBER_OF_J1939_NETWORKS = 11; ///< The number of networks in DM13 that are set aside for J1939 @@ -486,8 +498,9 @@ namespace isobus std::string productIdentificationBrand; ///< The product identification brand for sending the product identification message std::string productIdentificationModel; ///< The product identification model name for sending the product identification message std::uint32_t lastDM1SentTimestamp = 0; ///< A timestamp in milliseconds of the last time a DM1 was sent - bool broadcastState = true; ///< Bitfield for tracking the network broadcast state for DM13 std::uint32_t lastDM13ReceivedTimestamp = 0; ///< A timestamp in milliseconds when we last got a DM13 message + std::uint16_t customDM13SuspensionTime = 0; ///< If using a non-standard DM13 suspension time, this tracks that duration in milliseconds + bool broadcastState = true; ///< Bitfield for tracking the network broadcast state for DM13 bool j1939Mode = false; ///< Tells the protocol to operate according to J1939 instead of ISO11783 bool initialized = false; ///< Stores if the interface has been initialized }; diff --git a/isobus/src/isobus_diagnostic_protocol.cpp b/isobus/src/isobus_diagnostic_protocol.cpp index c5888da5..103ad053 100644 --- a/isobus/src/isobus_diagnostic_protocol.cpp +++ b/isobus/src/isobus_diagnostic_protocol.cpp @@ -64,6 +64,16 @@ namespace isobus return occurrenceCount; } + std::uint32_t DiagnosticProtocol::DiagnosticTroubleCode::get_suspect_parameter_number() const + { + return suspectParameterNumber; + } + + DiagnosticProtocol::FailureModeIdentifier DiagnosticProtocol::DiagnosticTroubleCode::get_failure_mode_identifier() const + { + return failureModeIdentifier; + } + DiagnosticProtocol::DiagnosticProtocol(std::shared_ptr internalControlFunction, NetworkType networkType) : ControlFunctionFunctionalitiesMessageInterface(internalControlFunction), myControlFunction(internalControlFunction), @@ -94,6 +104,7 @@ namespace isobus // PGN protocol will check for duplicates, so no worries if there's already a request protocol registered. ParameterGroupNumberRequestProtocol::assign_pgn_request_protocol_to_internal_control_function(myControlFunction); + ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage1), process_parameter_group_number_request, this); ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage2), process_parameter_group_number_request, this); ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage3), process_parameter_group_number_request, this); ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction)->register_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage11), process_parameter_group_number_request, this); @@ -104,6 +115,11 @@ namespace isobus } } + bool DiagnosticProtocol::get_initialized() const + { + return initialized; + } + void DiagnosticProtocol::terminate() { if (initialized) @@ -118,7 +134,15 @@ namespace isobus void DiagnosticProtocol::update() { - if (SystemTiming::time_expired_ms(lastDM13ReceivedTimestamp, DM13_TIMEOUT_MS)) + if (0 != customDM13SuspensionTime) + { + if (SystemTiming::time_expired_ms(lastDM13ReceivedTimestamp, customDM13SuspensionTime)) + { + broadcastState = true; + customDM13SuspensionTime = 0; + } + } + else if (SystemTiming::time_expired_ms(lastDM13ReceivedTimestamp, DM13_TIMEOUT_MS)) { broadcastState = true; } @@ -310,11 +334,23 @@ namespace isobus softwareIdentificationFields[index] = value; } - bool DiagnosticProtocol::suspend_broadcasts(std::uint16_t suspendTime_seconds) const + bool DiagnosticProtocol::suspend_broadcasts(std::uint16_t suspendTime_seconds) { + broadcastState = false; + lastDM13ReceivedTimestamp = SystemTiming::get_timestamp_ms(); + + if (customDM13SuspensionTime < MAX_DM13_CUSTOM_SUSPEND_TIME_MS) + { + customDM13SuspensionTime = suspendTime_seconds; + } return send_dm13_announce_suspension(suspendTime_seconds); } + bool DiagnosticProtocol::get_broadcast_state() const + { + return broadcastState; + } + std::uint8_t DiagnosticProtocol::convert_flash_state_to_byte(FlashState flash) const { std::uint8_t retVal = 0; @@ -347,6 +383,7 @@ namespace isobus ParameterGroupNumberRequestProtocol *pgnRequestProtocol = ParameterGroupNumberRequestProtocol::get_pgn_request_protocol_by_internal_control_function(myControlFunction); if (nullptr != pgnRequestProtocol) { + pgnRequestProtocol->remove_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage1), process_parameter_group_number_request, this); pgnRequestProtocol->remove_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage2), process_parameter_group_number_request, this); pgnRequestProtocol->remove_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage3), process_parameter_group_number_request, this); pgnRequestProtocol->remove_pgn_request_callback(static_cast(CANLibParameterGroupNumber::DiagnosticMessage11), process_parameter_group_number_request, this); @@ -580,7 +617,7 @@ namespace isobus if (payloadSize <= MAX_PAYLOAD_SIZE_BYTES) { std::vector buffer; - buffer.resize(payloadSize); + buffer.resize(payloadSize < CAN_DATA_LENGTH ? CAN_DATA_LENGTH : payloadSize); if (get_j1939_mode()) { @@ -668,7 +705,7 @@ namespace isobus if (payloadSize <= MAX_PAYLOAD_SIZE_BYTES) { std::vector buffer; - buffer.resize(payloadSize); + buffer.resize(payloadSize < CAN_DATA_LENGTH ? CAN_DATA_LENGTH : payloadSize); if (get_j1939_mode()) { @@ -786,10 +823,11 @@ namespace isobus bool DiagnosticProtocol::send_ecu_identification() const { std::string ecuIdString = ""; + const std::size_t maxComponent = get_j1939_mode() ? static_cast(ECUIdentificationFields::HardwareID) : static_cast(ECUIdentificationFields::NumberOfFields); - for (const auto &stringComponent : ecuIdentificationFields) + for (std::size_t i = 0; i < maxComponent; i++) { - ecuIdString.append(stringComponent); + ecuIdString.append(ecuIdentificationFields.at(i)); } std::vector buffer(ecuIdString.begin(), ecuIdString.end()); @@ -1057,8 +1095,39 @@ namespace isobus switch (command) { case StopStartCommand::StopBroadcast: + { broadcastState = false; - break; + + switch (static_cast(messageData.at(3) & 0x0F)) + { + case SuspendSignalState::TemporarySuspension: + case SuspendSignalState::PartialTemporarySuspension: + { + std::uint16_t suspensionTime = message.get_uint16_at(3); + + if (suspensionTime < MAX_DM13_CUSTOM_SUSPEND_TIME_MS) + { + customDM13SuspensionTime = suspensionTime; + } + else + { + customDM13SuspensionTime = 0; + } + } + break; + + case SuspendSignalState::IndefiniteSuspension: + case SuspendSignalState::PartialIndefiniteSuspension: + { + customDM13SuspensionTime = 0; + } + break; + + default: + break; + } + } + break; case StopStartCommand::StartBroadcast: broadcastState = true; @@ -1073,8 +1142,39 @@ namespace isobus switch (command) { case StopStartCommand::StopBroadcast: + { broadcastState = false; - break; + + switch (static_cast(messageData.at(3) & 0x0F)) + { + case SuspendSignalState::TemporarySuspension: + case SuspendSignalState::PartialTemporarySuspension: + { + std::uint16_t suspensionTime = message.get_uint16_at(3); + + if (suspensionTime < MAX_DM13_CUSTOM_SUSPEND_TIME_MS) + { + customDM13SuspensionTime = suspensionTime; + } + else + { + customDM13SuspensionTime = 0; + } + } + break; + + case SuspendSignalState::IndefiniteSuspension: + case SuspendSignalState::PartialIndefiniteSuspension: + { + customDM13SuspensionTime = 0; + } + break; + + default: + break; + } + } + break; case StopStartCommand::StartBroadcast: broadcastState = true; @@ -1101,6 +1201,13 @@ namespace isobus { switch (parameterGroupNumber) { + case static_cast(CANLibParameterGroupNumber::DiagnosticMessage1): + { + txFlags.set_flag(static_cast(TransmitFlags::DM1)); + retVal = true; + } + break; + case static_cast(CANLibParameterGroupNumber::DiagnosticMessage2): { txFlags.set_flag(static_cast(TransmitFlags::DM2)); @@ -1142,13 +1249,15 @@ namespace isobus case static_cast(CANLibParameterGroupNumber::SoftwareIdentification): { - retVal = send_software_identification(); + txFlags.set_flag(static_cast(TransmitFlags::SoftwareIdentification)); + retVal = true; } break; case static_cast(CANLibParameterGroupNumber::ECUIdentificationInformation): { - retVal = send_ecu_identification(); + txFlags.set_flag(static_cast(TransmitFlags::ECUIdentification)); + retVal = true; } break; @@ -1221,6 +1330,18 @@ namespace isobus } break; + case static_cast(TransmitFlags::ECUIdentification): + { + transmitSuccessful = parent->send_ecu_identification(); + } + break; + + case static_cast(TransmitFlags::SoftwareIdentification): + { + transmitSuccessful = parent->send_software_identification(); + } + break; + default: break; } diff --git a/test/core_network_management_tests.cpp b/test/core_network_management_tests.cpp index 7bc54b2b..c75a75d7 100644 --- a/test/core_network_management_tests.cpp +++ b/test/core_network_management_tests.cpp @@ -55,7 +55,6 @@ TEST(CORE_TESTS, TestCreateAndDestroyICFs) TEST(CORE_TESTS, BusloadTest) { - EXPECT_EQ(0.0f, CANNetworkManager::CANNetwork.get_estimated_busload(0)); // This test runs early in the testing, so load should be zero. EXPECT_EQ(0.0f, CANNetworkManager::CANNetwork.get_estimated_busload(200)); // Invalid channel should return zero load // Send a bunch of messages through the receive process diff --git a/test/diagnostic_protocol_tests.cpp b/test/diagnostic_protocol_tests.cpp index ddb3b5c4..305b8bf0 100644 --- a/test/diagnostic_protocol_tests.cpp +++ b/test/diagnostic_protocol_tests.cpp @@ -1,6 +1,9 @@ #include +#include "isobus/hardware_integration/can_hardware_interface.hpp" +#include "isobus/hardware_integration/virtual_can_plugin.hpp" #include "isobus/isobus/isobus_diagnostic_protocol.hpp" +#include "isobus/utility/system_timing.hpp" using namespace isobus; @@ -18,3 +21,957 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, CreateAndDestroyProtocolObjects) //! @todo try to reduce the reference count, such that that we don't use a control function after it is destroyed ASSERT_TRUE(TestInternalECU->destroy(2)); } + +TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) +{ + NAME TestDeviceNAME(0); + + TestDeviceNAME.set_arbitrary_address_capable(true); + TestDeviceNAME.set_industry_group(2); + TestDeviceNAME.set_device_class(6); + TestDeviceNAME.set_function_code(static_cast(isobus::NAME::Function::DriveAxleControlBrakes)); + TestDeviceNAME.set_identity_number(2); + TestDeviceNAME.set_ecu_instance(0); + TestDeviceNAME.set_function_instance(0); + TestDeviceNAME.set_device_class_instance(0); + TestDeviceNAME.set_manufacturer_code(64); + + auto TestInternalECU = InternalControlFunction::create(TestDeviceNAME, 0xAA, 0); + + DiagnosticProtocol protocolUnderTest(TestInternalECU, DiagnosticProtocol::NetworkType::SAEJ1939Network1PrimaryVehicleNetwork); + + EXPECT_FALSE(protocolUnderTest.get_initialized()); + protocolUnderTest.initialize(); + EXPECT_TRUE(protocolUnderTest.get_initialized()); + + VirtualCANPlugin testPlugin; + testPlugin.open(); + + CANHardwareInterface::set_number_of_can_channels(1); + CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared()); + CANHardwareInterface::start(); + + std::uint32_t waitingTimestamp_ms = SystemTiming::get_timestamp_ms(); + + while ((!TestInternalECU->get_address_valid()) && + (!SystemTiming::time_expired_ms(waitingTimestamp_ms, 2000))) + { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + + ASSERT_TRUE(TestInternalECU->get_address_valid()); + + CANMessageFrame testFrame; + + testFrame.timestamp_us = 0; + testFrame.identifier = 0; + testFrame.channel = 0; + std::memset(testFrame.data, 0, sizeof(testFrame.data)); + testFrame.dataLength = 0; + testFrame.isExtendedFrame = true; + + // Force claim some other partner + testFrame.dataLength = 8; + testFrame.channel = 0; + testFrame.isExtendedFrame = true; + testFrame.identifier = 0x18EEFFAB; + testFrame.data[0] = 0x04; + testFrame.data[1] = 0x05; + testFrame.data[2] = 0x07; + testFrame.data[3] = 0x12; + testFrame.data[4] = 0x01; + testFrame.data[5] = 0x82; + testFrame.data[6] = 0x01; + testFrame.data[7] = 0xA0; + CANNetworkManager::process_receive_can_message_frame(testFrame); + + // Get the virtual CAN plugin back to a known state + while (!testPlugin.get_queue_empty()) + { + testPlugin.read_frame(testFrame); + } + ASSERT_TRUE(testPlugin.get_queue_empty()); + + // Ready to run some tests + std::cerr << "These tests use BAM to transmit, so they may take several seconds.." << std::endl; + + { + // Test ECU ID format against J1939-71 + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::HardwareID, "Some Hardware ID"); + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::Location, "The Internet"); + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::ManufacturerName, "None"); + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::PartNumber, "1234"); + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::SerialNumber, "9876"); + protocolUnderTest.set_ecu_id_field(isobus::DiagnosticProtocol::ECUIdentificationFields::Type, "AgISOStack"); + + // Use a PGN request to trigger sending it from the protocol + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xC5; + testFrame.data[1] = 0xFD; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Make sure we're using ISO mode for this parsing to work + ASSERT_FALSE(protocolUnderTest.get_j1939_mode()); + + // This message gets sent with BAM with PGN 0xFDC5, so we'll have to wait a while for the message to send. + // This a a nice test because it exercises the transport protocol as well + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + std::uint16_t expectedBAMLength = 56; // This is all strings lengths plus delimiters + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x08, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0xC5, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFD, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ('1', testFrame.data[1]); // Part Number index 0 + EXPECT_EQ('2', testFrame.data[2]); // Part Number index 1 + EXPECT_EQ('3', testFrame.data[3]); // Part Number index 2 + EXPECT_EQ('4', testFrame.data[4]); // Part Number index 3 + EXPECT_EQ('*', testFrame.data[5]); // Delimiter + EXPECT_EQ('9', testFrame.data[6]); // Serial number index 0 + EXPECT_EQ('8', testFrame.data[7]); // Serial number index 1 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ('7', testFrame.data[1]); // Serial number index 2 + EXPECT_EQ('6', testFrame.data[2]); // Serial number index 3 + EXPECT_EQ('*', testFrame.data[3]); // Delimiter + EXPECT_EQ('T', testFrame.data[4]); // Location index 0 + EXPECT_EQ('h', testFrame.data[5]); // Location index 1 + EXPECT_EQ('e', testFrame.data[6]); // Location index 2 + EXPECT_EQ(' ', testFrame.data[7]); // Location index 3 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 3 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x03, testFrame.data[0]); // Sequence 3 + EXPECT_EQ('I', testFrame.data[1]); // Location index 4 + EXPECT_EQ('n', testFrame.data[2]); // Location index 5 + EXPECT_EQ('t', testFrame.data[3]); // Location index 6 + EXPECT_EQ('e', testFrame.data[4]); // Location index 7 + EXPECT_EQ('r', testFrame.data[5]); // Location index 8 + EXPECT_EQ('n', testFrame.data[6]); // Location index 9 + EXPECT_EQ('e', testFrame.data[7]); // Location index 10 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 4 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x04, testFrame.data[0]); // Sequence 4 + EXPECT_EQ('t', testFrame.data[1]); // Location index 11 + EXPECT_EQ('*', testFrame.data[2]); // Delimiter + EXPECT_EQ('A', testFrame.data[3]); // Type Index 0 + EXPECT_EQ('g', testFrame.data[4]); // Type Index 1 + EXPECT_EQ('I', testFrame.data[5]); // Type Index 2 + EXPECT_EQ('S', testFrame.data[6]); // Type Index 3 + EXPECT_EQ('O', testFrame.data[7]); // Type Index 4 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 5 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x05, testFrame.data[0]); // Sequence 5 + EXPECT_EQ('S', testFrame.data[1]); // Type Index 5 + EXPECT_EQ('t', testFrame.data[2]); // Type Index 6 + EXPECT_EQ('a', testFrame.data[3]); // Type Index 7 + EXPECT_EQ('c', testFrame.data[4]); // Type Index 8 + EXPECT_EQ('k', testFrame.data[5]); // Type Index 9 + EXPECT_EQ('*', testFrame.data[6]); // Delimiter + EXPECT_EQ('N', testFrame.data[7]); // Manufacturer index 0 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 6 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x06, testFrame.data[0]); // Sequence 6 + EXPECT_EQ('o', testFrame.data[1]); // Manufacturer index 1 + EXPECT_EQ('n', testFrame.data[2]); // Manufacturer index 2 + EXPECT_EQ('e', testFrame.data[3]); // Manufacturer index 3 + EXPECT_EQ('*', testFrame.data[4]); // Delimiter + EXPECT_EQ('S', testFrame.data[5]); // Hardware ID Index 0 + EXPECT_EQ('o', testFrame.data[6]); // Hardware ID Index 1 + EXPECT_EQ('m', testFrame.data[7]); // Hardware ID Index 2 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 7 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x07, testFrame.data[0]); // Sequence 7 + EXPECT_EQ('e', testFrame.data[1]); // Hardware ID Index 3 + EXPECT_EQ(' ', testFrame.data[2]); // Hardware ID Index 4 + EXPECT_EQ('H', testFrame.data[3]); // Hardware ID Index 5 + EXPECT_EQ('a', testFrame.data[4]); // Hardware ID Index 6 + EXPECT_EQ('r', testFrame.data[5]); // Hardware ID Index 7 + EXPECT_EQ('d', testFrame.data[6]); // Hardware ID Index 8 + EXPECT_EQ('w', testFrame.data[7]); // Hardware ID Index 9 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 7 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x08, testFrame.data[0]); // Sequence 8 + EXPECT_EQ('a', testFrame.data[1]); // Hardware ID Index 10 + EXPECT_EQ('r', testFrame.data[2]); // Hardware ID Index 11 + EXPECT_EQ('e', testFrame.data[3]); // Hardware ID Index 12 + EXPECT_EQ(' ', testFrame.data[4]); // Hardware ID Index 13 + EXPECT_EQ('I', testFrame.data[5]); // Hardware ID Index 14 + EXPECT_EQ('D', testFrame.data[6]); // Hardware ID Index 15 + EXPECT_EQ('*', testFrame.data[7]); // Delimiter (end of the message) + } + + { + // Re-test in J1939 mode. Should omit the hardware ID + protocolUnderTest.set_j1939_mode(true); + + // Use a PGN request to trigger sending it from the protocol + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xC5; + testFrame.data[1] = 0xFD; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + // Make sure we're using ISO mode for this parsing to work + ASSERT_TRUE(protocolUnderTest.get_j1939_mode()); + + // This message gets sent with BAM with PGN 0xFDC5, so we'll have to wait a while for the message to send. + // This a a nice test because it exercises the transport protocol as well + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + std::uint16_t expectedBAMLength = 39; // This is all strings lengths plus delimiters + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x06, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0xC5, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFD, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ('1', testFrame.data[1]); // Part Number index 0 + EXPECT_EQ('2', testFrame.data[2]); // Part Number index 1 + EXPECT_EQ('3', testFrame.data[3]); // Part Number index 2 + EXPECT_EQ('4', testFrame.data[4]); // Part Number index 3 + EXPECT_EQ('*', testFrame.data[5]); // Delimiter + EXPECT_EQ('9', testFrame.data[6]); // Serial number index 0 + EXPECT_EQ('8', testFrame.data[7]); // Serial number index 1 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ('7', testFrame.data[1]); // Serial number index 2 + EXPECT_EQ('6', testFrame.data[2]); // Serial number index 3 + EXPECT_EQ('*', testFrame.data[3]); // Delimiter + EXPECT_EQ('T', testFrame.data[4]); // Location index 0 + EXPECT_EQ('h', testFrame.data[5]); // Location index 1 + EXPECT_EQ('e', testFrame.data[6]); // Location index 2 + EXPECT_EQ(' ', testFrame.data[7]); // Location index 3 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 3 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x03, testFrame.data[0]); // Sequence 3 + EXPECT_EQ('I', testFrame.data[1]); // Location index 4 + EXPECT_EQ('n', testFrame.data[2]); // Location index 5 + EXPECT_EQ('t', testFrame.data[3]); // Location index 6 + EXPECT_EQ('e', testFrame.data[4]); // Location index 7 + EXPECT_EQ('r', testFrame.data[5]); // Location index 8 + EXPECT_EQ('n', testFrame.data[6]); // Location index 9 + EXPECT_EQ('e', testFrame.data[7]); // Location index 10 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 4 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x04, testFrame.data[0]); // Sequence 4 + EXPECT_EQ('t', testFrame.data[1]); // Location index 11 + EXPECT_EQ('*', testFrame.data[2]); // Delimiter + EXPECT_EQ('A', testFrame.data[3]); // Type Index 0 + EXPECT_EQ('g', testFrame.data[4]); // Type Index 1 + EXPECT_EQ('I', testFrame.data[5]); // Type Index 2 + EXPECT_EQ('S', testFrame.data[6]); // Type Index 3 + EXPECT_EQ('O', testFrame.data[7]); // Type Index 4 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 5 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x05, testFrame.data[0]); // Sequence 5 + EXPECT_EQ('S', testFrame.data[1]); // Type Index 5 + EXPECT_EQ('t', testFrame.data[2]); // Type Index 6 + EXPECT_EQ('a', testFrame.data[3]); // Type Index 7 + EXPECT_EQ('c', testFrame.data[4]); // Type Index 8 + EXPECT_EQ('k', testFrame.data[5]); // Type Index 9 + EXPECT_EQ('*', testFrame.data[6]); // Delimiter + EXPECT_EQ('N', testFrame.data[7]); // Manufacturer index 0 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // DM1 might be sent in j1939 mode, need to screen it out + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + // BAM Payload Frame 6 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x06, testFrame.data[0]); // Sequence 6 + EXPECT_EQ('o', testFrame.data[1]); // Manufacturer index 1 + EXPECT_EQ('n', testFrame.data[2]); // Manufacturer index 2 + EXPECT_EQ('e', testFrame.data[3]); // Manufacturer index 3 + EXPECT_EQ('*', testFrame.data[4]); // Delimiter + EXPECT_EQ(0xFF, testFrame.data[5]); // Padding + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + protocolUnderTest.set_j1939_mode(false); + EXPECT_FALSE(protocolUnderTest.get_j1939_mode()); + } + + { + /// Now, test software ID against J1939-71 + protocolUnderTest.set_software_id_field(0, "Unit Test 1.0.0"); + protocolUnderTest.set_software_id_field(1, "Another version x.x.x.x"); + + // Use a PGN request to trigger sending it from the protocol + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xDA; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + // This message gets sent with BAM, so we'll have to wait a while for the message to send. + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + std::uint16_t expectedBAMLength = 40; // This is all strings lengths plus delimiters + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x06, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0xDA, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFE, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ('U', testFrame.data[1]); // Version 0, index 0 + EXPECT_EQ('n', testFrame.data[2]); // Version 0, index 1 + EXPECT_EQ('i', testFrame.data[3]); // Version 0, index 2 + EXPECT_EQ('t', testFrame.data[4]); // Version 0, index 3 + EXPECT_EQ(' ', testFrame.data[5]); // Version 0, index 4 + EXPECT_EQ('T', testFrame.data[6]); // Version 0, index 5 + EXPECT_EQ('e', testFrame.data[7]); // Version 0, index 6 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ('s', testFrame.data[1]); // Version 0, index 7 + EXPECT_EQ('t', testFrame.data[2]); // Version 0, index 8 + EXPECT_EQ(' ', testFrame.data[3]); // Version 0, index 9 + EXPECT_EQ('1', testFrame.data[4]); // Version 0, index 10 + EXPECT_EQ('.', testFrame.data[5]); // Version 0, index 11 + EXPECT_EQ('0', testFrame.data[6]); // Version 0, index 12 + EXPECT_EQ('.', testFrame.data[7]); // Version 0, index 13 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 3 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x03, testFrame.data[0]); // Sequence 3 + EXPECT_EQ('0', testFrame.data[1]); // Version 0, index 7 + EXPECT_EQ('*', testFrame.data[2]); // Delimiter + EXPECT_EQ('A', testFrame.data[3]); // Version 1, index 0 + EXPECT_EQ('n', testFrame.data[4]); // Version 1, index 1 + EXPECT_EQ('o', testFrame.data[5]); // Version 1, index 2 + EXPECT_EQ('t', testFrame.data[6]); // Version 1, index 3 + EXPECT_EQ('h', testFrame.data[7]); // Version 1, index 4 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 4 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x04, testFrame.data[0]); // Sequence 4 + EXPECT_EQ('e', testFrame.data[1]); // Version 0, index 7 + EXPECT_EQ('r', testFrame.data[2]); // Delimiter + EXPECT_EQ(' ', testFrame.data[3]); // Version 1, index 5 + EXPECT_EQ('v', testFrame.data[4]); // Version 1, index 6 + EXPECT_EQ('e', testFrame.data[5]); // Version 1, index 7 + EXPECT_EQ('r', testFrame.data[6]); // Version 1, index 8 + EXPECT_EQ('s', testFrame.data[7]); // Version 1, index 9 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 5 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x05, testFrame.data[0]); // Sequence 5 + EXPECT_EQ('i', testFrame.data[1]); // Version 0, index 7 + EXPECT_EQ('o', testFrame.data[2]); // Delimiter + EXPECT_EQ('n', testFrame.data[3]); // Version 1, index 5 + EXPECT_EQ(' ', testFrame.data[4]); // Version 1, index 6 + EXPECT_EQ('x', testFrame.data[5]); // Version 1, index 7 + EXPECT_EQ('.', testFrame.data[6]); // Version 1, index 8 + EXPECT_EQ('x', testFrame.data[7]); // Version 1, index 9 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 6 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x06, testFrame.data[0]); // Sequence 6 + EXPECT_EQ('.', testFrame.data[1]); // Version 0, index 10 + EXPECT_EQ('x', testFrame.data[2]); // Version 0, index 11 + EXPECT_EQ('.', testFrame.data[3]); // Version 1, index 12 + EXPECT_EQ('x', testFrame.data[4]); // Version 1, index 13 + EXPECT_EQ('*', testFrame.data[5]); // Delimiter + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + } + + { + // Test diagnostic protocol identification message + // Use a PGN request to trigger sending it from the protocol + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0x32; + testFrame.data[1] = 0xFD; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FD32AA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // J1939–73 + EXPECT_EQ(0xFF, testFrame.data[1]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[5]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + } + + { + // Test Product Identification + protocolUnderTest.set_product_identification_code("1234567890ABC"); + protocolUnderTest.set_product_identification_brand("Open-Agriculture"); + protocolUnderTest.set_product_identification_model("AgIsoStack++"); + // Use a PGN request to trigger sending it + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0x8D; + testFrame.data[1] = 0xFC; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + // This message gets sent with BAM, so we'll have to wait a while for the message to send. + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // More manual BAM parsing... + std::uint16_t expectedBAMLength = 44; // This is all strings lengths plus delimiters + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x07, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0x8D, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFC, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ('1', testFrame.data[1]); // ID Code index 0 + EXPECT_EQ('2', testFrame.data[2]); // ID Code index 1 + EXPECT_EQ('3', testFrame.data[3]); // ID Code index 2 + EXPECT_EQ('4', testFrame.data[4]); // ID Code index 3 + EXPECT_EQ('5', testFrame.data[5]); // ID Code index 4 + EXPECT_EQ('6', testFrame.data[6]); // ID Code index 5 + EXPECT_EQ('7', testFrame.data[7]); // ID Code index 6 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ('8', testFrame.data[1]); // ID Code index 7 + EXPECT_EQ('9', testFrame.data[2]); // ID Code index 8 + EXPECT_EQ('0', testFrame.data[3]); // ID Code index 9 + EXPECT_EQ('A', testFrame.data[4]); // ID Code index 10 + EXPECT_EQ('B', testFrame.data[5]); // ID Code index 11 + EXPECT_EQ('C', testFrame.data[6]); // ID Code index 12 + EXPECT_EQ('*', testFrame.data[7]); // Delimiter + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 3 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x03, testFrame.data[0]); // Sequence 3 + EXPECT_EQ('O', testFrame.data[1]); // Brand index 0 + EXPECT_EQ('p', testFrame.data[2]); // Brand index 1 + EXPECT_EQ('e', testFrame.data[3]); // Brand index 2 + EXPECT_EQ('n', testFrame.data[4]); // Brand index 3 + EXPECT_EQ('-', testFrame.data[5]); // Brand index 4 + EXPECT_EQ('A', testFrame.data[6]); // Brand index 5 + EXPECT_EQ('g', testFrame.data[7]); // Brand index 6 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 4 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x04, testFrame.data[0]); // Sequence 4 + EXPECT_EQ('r', testFrame.data[1]); // Brand index 7 + EXPECT_EQ('i', testFrame.data[2]); // Brand index 8 + EXPECT_EQ('c', testFrame.data[3]); // Brand index 9 + EXPECT_EQ('u', testFrame.data[4]); // Brand index 10 + EXPECT_EQ('l', testFrame.data[5]); // Brand index 11 + EXPECT_EQ('t', testFrame.data[6]); // Brand index 12 + EXPECT_EQ('u', testFrame.data[7]); // Brand index 13 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 5 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x05, testFrame.data[0]); // Sequence 5 + EXPECT_EQ('r', testFrame.data[1]); // Brand index 14 + EXPECT_EQ('e', testFrame.data[2]); // Brand index 15 + EXPECT_EQ('*', testFrame.data[3]); // Delimiter + EXPECT_EQ('A', testFrame.data[4]); // Model index 0 + EXPECT_EQ('g', testFrame.data[5]); // Model index 1 + EXPECT_EQ('I', testFrame.data[6]); // Model index 2 + EXPECT_EQ('s', testFrame.data[7]); // Model index 3 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 6 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x06, testFrame.data[0]); // Sequence 6 + EXPECT_EQ('o', testFrame.data[1]); // Model index 4 + EXPECT_EQ('S', testFrame.data[2]); // Model index 5 + EXPECT_EQ('t', testFrame.data[3]); // Model index 6 + EXPECT_EQ('a', testFrame.data[4]); // Model index 7 + EXPECT_EQ('c', testFrame.data[5]); // Model index 8 + EXPECT_EQ('k', testFrame.data[6]); // Model index 9 + EXPECT_EQ('+', testFrame.data[7]); // Model index 10 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 7 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x07, testFrame.data[0]); // Sequence 7 + EXPECT_EQ('+', testFrame.data[1]); // Model index 11 + EXPECT_EQ('*', testFrame.data[2]); // Delimiter + EXPECT_EQ(0xFF, testFrame.data[3]); // Padding + EXPECT_EQ(0xFF, testFrame.data[4]); // Padding + EXPECT_EQ(0xFF, testFrame.data[5]); // Padding + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + } + + // Make a few test DTCs + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC1(1234, isobus::DiagnosticProtocol::FailureModeIdentifier::ConditionExists, isobus::DiagnosticProtocol::LampStatus::None); + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC2(567, isobus::DiagnosticProtocol::FailureModeIdentifier::DataErratic, isobus::DiagnosticProtocol::LampStatus::AmberWarningLampSlowFlash); + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC3(8910, isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntelligentDevice, isobus::DiagnosticProtocol::LampStatus::RedStopLampSolid); + + { + // Test DM1 + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC1(1234, isobus::DiagnosticProtocol::FailureModeIdentifier::ConditionExists, isobus::DiagnosticProtocol::LampStatus::None); + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC2(567, isobus::DiagnosticProtocol::FailureModeIdentifier::DataErratic, isobus::DiagnosticProtocol::LampStatus::AmberWarningLampSlowFlash); + isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC3(8910, isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntelligentDevice, isobus::DiagnosticProtocol::LampStatus::RedStopLampSolid); + + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + + // Use a PGN request to trigger sending it immediately + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCA; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // A single DTC is 1 frame + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECAAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0xFF, testFrame.data[0]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0xD2, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x04, testFrame.data[3]); // SPN + EXPECT_EQ(31, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(1, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + protocolUnderTest.set_j1939_mode(true); + EXPECT_TRUE(protocolUnderTest.get_j1939_mode()); + + // Validate in J1939 mode + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCA; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECAAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x00, testFrame.data[0]); // Lamp + EXPECT_EQ(0xFF, testFrame.data[1]); // Flash (do not flash / solid) + EXPECT_EQ(0xD2, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x04, testFrame.data[3]); // SPN + EXPECT_EQ(31, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(1, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + protocolUnderTest.set_j1939_mode(false); + EXPECT_FALSE(protocolUnderTest.get_j1939_mode()); + + // Test a DM1 with multiple DTCs in it + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, true); + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCA; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Wait for BAM + std::this_thread::sleep_for(std::chrono::milliseconds(250)); + std::uint16_t expectedBAMLength = 14; // This is 2 + 4 * number of DTCs + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x02, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0xCA, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFE, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp / reserved + EXPECT_EQ(0xFF, testFrame.data[2]); // Flash / reserved + EXPECT_EQ(0xD2, testFrame.data[3]); // SPN 1 + EXPECT_EQ(0x04, testFrame.data[4]); // SPN 1 + EXPECT_EQ(31, testFrame.data[5]); // FMI 1 + EXPECT_EQ(1, testFrame.data[6]); // Count 1 + EXPECT_EQ(0x37, testFrame.data[7]); // SPN2 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ(0x02, testFrame.data[1]); // SPN 2 + EXPECT_EQ(2, testFrame.data[2]); // FMI 2 + EXPECT_EQ(01, testFrame.data[3]); // Count 2 + EXPECT_EQ(0xCE, testFrame.data[4]); // SPN 3 + EXPECT_EQ(0x22, testFrame.data[5]); // SPN 3 + EXPECT_EQ(12, testFrame.data[6]); // FMI 3 + EXPECT_EQ(1, testFrame.data[7]); // Count 3 + } + + { + // Test DM2 + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, false); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, false); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, false); + + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCB; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Wait for BAM + std::this_thread::sleep_for(std::chrono::milliseconds(250)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + std::uint16_t expectedBAMLength = 14; // This is 2 + 4 * number of DTCs + + // Broadcast Announce Message + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18ECFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x20, testFrame.data[0]); // BAM Multiplexer + EXPECT_EQ(expectedBAMLength & 0xFF, testFrame.data[1]); // Length LSB + EXPECT_EQ((expectedBAMLength >> 8) & 0xFF, testFrame.data[2]); // Length MSB + EXPECT_EQ(0x02, testFrame.data[3]); // Number of frames in session (based on length) + EXPECT_EQ(0xFF, testFrame.data[4]); // Always 0xFF + EXPECT_EQ(0xCB, testFrame.data[5]); // PGN LSB + EXPECT_EQ(0xFE, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN MSB + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 1 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x01, testFrame.data[0]); // Sequence 1 + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp / reserved + EXPECT_EQ(0xFF, testFrame.data[2]); // Flash / reserved + EXPECT_EQ(0xD2, testFrame.data[3]); // SPN 1 + EXPECT_EQ(0x04, testFrame.data[4]); // SPN 1 + EXPECT_EQ(31, testFrame.data[5]); // FMI 1 + EXPECT_EQ(1, testFrame.data[6]); // Count 1 + EXPECT_EQ(0x37, testFrame.data[7]); // SPN2 + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // BAM Payload Frame 2 + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x1CEBFFAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0x02, testFrame.data[0]); // Sequence 2 + EXPECT_EQ(0x02, testFrame.data[1]); // SPN 2 + EXPECT_EQ(2, testFrame.data[2]); // FMI 2 + EXPECT_EQ(01, testFrame.data[3]); // Count 2 + EXPECT_EQ(0xCE, testFrame.data[4]); // SPN 3 + EXPECT_EQ(0x22, testFrame.data[5]); // SPN 3 + EXPECT_EQ(12, testFrame.data[6]); // FMI 3 + EXPECT_EQ(1, testFrame.data[7]); // Count 3 + + // Clear the DTCs + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); + + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCB; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // Now zero DTCs + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECBAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0xFF, testFrame.data[0]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0x00, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x00, testFrame.data[3]); // SPN + EXPECT_EQ(0x00, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(0x00, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + } + + { + // Test DM13 against J1939-73 + EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); + EXPECT_TRUE(protocolUnderTest.suspend_broadcasts(5)); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // When we are announcing a suspension, we're supposed to set + // all values to NA except for the time, which we set to 5 in this case + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18DFFFAA, testFrame.identifier); // DM13 from address AA + EXPECT_EQ(0xFF, testFrame.data[0]); + EXPECT_EQ(0xFF, testFrame.data[1]); + EXPECT_EQ(0xFF, testFrame.data[2]); + EXPECT_EQ(0xFF, testFrame.data[3]); + EXPECT_EQ(0x05, testFrame.data[4]); + EXPECT_EQ(0x00, testFrame.data[5]); + EXPECT_EQ(0xFF, testFrame.data[6]); + EXPECT_EQ(0xFF, testFrame.data[7]); + + EXPECT_FALSE(protocolUnderTest.get_broadcast_state()); + + // Wait suspension to be lifted + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + protocolUnderTest.update(); + EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); + + // Test a suspension by another ECU. Set only our network. + testFrame.dataLength = 8; + testFrame.data[0] = 0xFC; + testFrame.data[1] = 0xFF; + testFrame.data[2] = 0xFF; + testFrame.data[3] = 0x00; + testFrame.data[4] = 0x0A; + testFrame.data[5] = 0x00; + testFrame.data[6] = 0xFF; + testFrame.data[7] = 0xFF; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + EXPECT_FALSE(protocolUnderTest.get_broadcast_state()); + + // Restart broadcasts + testFrame.dataLength = 8; + testFrame.data[0] = 0xFD; + testFrame.data[1] = 0xFF; + testFrame.data[2] = 0xFF; + testFrame.data[3] = 0x00; + testFrame.data[4] = 0xFF; + testFrame.data[5] = 0xFF; + testFrame.data[6] = 0xFF; + testFrame.data[7] = 0xFF; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); + } + protocolUnderTest.terminate(); + EXPECT_FALSE(protocolUnderTest.get_initialized()); + CANHardwareInterface::stop(); +} diff --git a/test/language_command_interface_tests.cpp b/test/language_command_interface_tests.cpp index b50810ce..a216877e 100644 --- a/test/language_command_interface_tests.cpp +++ b/test/language_command_interface_tests.cpp @@ -121,7 +121,7 @@ TEST(LANGUAGE_COMMAND_INTERFACE_TESTS, MessageContentParsing) EXPECT_EQ(LanguageCommandInterface::PressureUnits::Reserved, interfaceUnderTest.get_commanded_pressure_units()); EXPECT_EQ(LanguageCommandInterface::ForceUnits::ImperialUS, interfaceUnderTest.get_commanded_force_units()); EXPECT_EQ(LanguageCommandInterface::UnitSystem::Metric, interfaceUnderTest.get_commanded_generic_units()); - EXPECT_LT(SystemTiming::get_timestamp_ms() - interfaceUnderTest.get_language_command_timestamp(), 1); + EXPECT_LT(SystemTiming::get_timestamp_ms() - interfaceUnderTest.get_language_command_timestamp(), 2); EXPECT_EQ("", interfaceUnderTest.get_country_code()); // Use the language code as a way to assert against if we processed the message. diff --git a/test/tc_client_tests.cpp b/test/tc_client_tests.cpp index f3afddcd..51deb7cb 100644 --- a/test/tc_client_tests.cpp +++ b/test/tc_client_tests.cpp @@ -290,6 +290,8 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, MessageEncoding) auto tcPartner = PartneredControlFunction::create(0, vtNameFilters); + CANNetworkManager::CANNetwork.update(); + // Force claim a partner testFrame.dataLength = 8; testFrame.channel = 0; @@ -483,6 +485,8 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, MessageEncoding) //! @todo try to reduce the reference count, such that that we don't use a control function after it is destroyed ASSERT_TRUE(tcPartner->destroy(3)); ASSERT_TRUE(internalECU->destroy(3)); + + CANNetworkManager::CANNetwork.update(); } TEST(TASK_CONTROLLER_CLIENT_TESTS, BadPartnerDeathTest) @@ -569,7 +573,7 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, StateMachineTests) testFrame.channel = 0; testFrame.isExtendedFrame = true; testFrame.identifier = 0x18EEFFF7; - testFrame.data[0] = 0x03; + testFrame.data[0] = 0x04; testFrame.data[1] = 0x04; testFrame.data[2] = 0x00; testFrame.data[3] = 0x12; @@ -1175,6 +1179,8 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, TimeoutTests) auto tcPartner = PartneredControlFunction::create(0, vtNameFilters); + CANNetworkManager::CANNetwork.update(); + DerivedTestTCClient interfaceUnderTest(tcPartner, internalECU); interfaceUnderTest.initialize(false); @@ -1371,6 +1377,8 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, WorkerThread) auto tcPartner = PartneredControlFunction::create(0, vtNameFilters); + CANNetworkManager::CANNetwork.update(); + DerivedTestTCClient interfaceUnderTest(tcPartner, internalECU); EXPECT_NO_THROW(interfaceUnderTest.initialize(true)); @@ -1442,7 +1450,7 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, CallbackTests) testFrame.channel = 0; testFrame.isExtendedFrame = true; testFrame.identifier = 0x18EEFFF7; - testFrame.data[0] = 0x03; + testFrame.data[0] = 0x05; testFrame.data[1] = 0x04; testFrame.data[2] = 0x00; testFrame.data[3] = 0x13; From 125671d49331ad1f72ee96df899e2938c891261c Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Thu, 29 Jun 2023 23:14:50 -0500 Subject: [PATCH 6/7] [DP]: Add DM22, DM11, DM3 tests Added additional diagnostic protocol tests. Fixed issues with DTC getters returning false all the time. --- isobus/src/isobus_diagnostic_protocol.cpp | 14 +- test/diagnostic_protocol_tests.cpp | 300 +++++++++++++++++++++- 2 files changed, 304 insertions(+), 10 deletions(-) diff --git a/isobus/src/isobus_diagnostic_protocol.cpp b/isobus/src/isobus_diagnostic_protocol.cpp index 103ad053..13693b07 100644 --- a/isobus/src/isobus_diagnostic_protocol.cpp +++ b/isobus/src/isobus_diagnostic_protocol.cpp @@ -222,6 +222,7 @@ namespace isobus if (activeDTCList.end() == activeLocation) { // Not already active. This is valid + retVal = true; auto inactiveLocation = std::find(inactiveDTCList.begin(), inactiveDTCList.end(), dtc); if (inactiveDTCList.end() != inactiveLocation) @@ -256,6 +257,7 @@ namespace isobus if (inactiveDTCList.end() == inactiveLocation) { + retVal = true; auto activeLocation = std::find(activeDTCList.begin(), activeDTCList.end(), dtc); if (activeDTCList.end() != activeLocation) @@ -915,10 +917,10 @@ namespace isobus buffer[2] = 0xFF; buffer[3] = 0xFF; buffer[4] = 0xFF; - buffer[5] = (currentMessageData.suspectParameterNumber & 0xFF); - buffer[6] = ((currentMessageData.suspectParameterNumber >> 8) & 0xFF); - buffer[7] = (((currentMessageData.suspectParameterNumber >> 16) << 5) & 0xFF); - buffer[7] |= (currentMessageData.failureModeIdentifier & 0x07); + buffer[5] = static_cast(currentMessageData.suspectParameterNumber & 0xFF); + buffer[6] = static_cast((currentMessageData.suspectParameterNumber >> 8) & 0xFF); + buffer[7] = static_cast(((currentMessageData.suspectParameterNumber >> 16) << 5) & 0xE0); + buffer[7] |= (currentMessageData.failureModeIdentifier & 0x1F); retVal = CANNetworkManager::CANNetwork.send_can_message(static_cast(CANLibParameterGroupNumber::DiagnosticMessage22), buffer.data(), @@ -1006,7 +1008,7 @@ namespace isobus } } - if (0 != tempDM22Data.nackIndicator) + if (0 == tempDM22Data.nackIndicator) { // DTC is in neither list. NACK with the reason that we don't know anything about it tempDM22Data.nackIndicator = static_cast(DM22NegativeAcknowledgeIndicator::UnknownOrDoesNotExist); @@ -1050,7 +1052,7 @@ namespace isobus } } - if (0 != tempDM22Data.nackIndicator) + if (0 == tempDM22Data.nackIndicator) { // DTC is in neither list. NACK with the reason that we don't know anything about it tempDM22Data.nackIndicator = static_cast(DM22NegativeAcknowledgeIndicator::UnknownOrDoesNotExist); diff --git a/test/diagnostic_protocol_tests.cpp b/test/diagnostic_protocol_tests.cpp index 305b8bf0..387b7b67 100644 --- a/test/diagnostic_protocol_tests.cpp +++ b/test/diagnostic_protocol_tests.cpp @@ -707,10 +707,6 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) { // Test DM1 - isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC1(1234, isobus::DiagnosticProtocol::FailureModeIdentifier::ConditionExists, isobus::DiagnosticProtocol::LampStatus::None); - isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC2(567, isobus::DiagnosticProtocol::FailureModeIdentifier::DataErratic, isobus::DiagnosticProtocol::LampStatus::AmberWarningLampSlowFlash); - isobus::DiagnosticProtocol::DiagnosticTroubleCode testDTC3(8910, isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntelligentDevice, isobus::DiagnosticProtocol::LampStatus::RedStopLampSolid); - protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); // Use a PGN request to trigger sending it immediately @@ -971,7 +967,303 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) protocolUnderTest.update(); EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); } + + { + // Test DM22 + protocolUnderTest.suspend_broadcasts(2); // Since DM1 could be sent during the test, suspend broadcasts for now + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, true); + protocolUnderTest.update(); + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + testFrame.dataLength = 8; + testFrame.identifier = 0x18C3AAAB; + testFrame.data[0] = 17; // Request to clear/reset a specific active DTC 5.7.22.1 + testFrame.data[1] = 0xFF; // Control Byte Specific Indicator for Individual DTC Clear (N/A) + testFrame.data[2] = 0xFF; // Reserved + testFrame.data[3] = 0xFF; // Reserved + testFrame.data[4] = 0xFF; // Reserved + testFrame.data[5] = 0xD2; // SPN + testFrame.data[6] = 0x04; // SPN + testFrame.data[7] = 31; // FMI (5 bits) + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Check for a positive acknowledge that the DTC was cleared + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18C3ABAA, testFrame.identifier); + EXPECT_EQ(18, testFrame.data[0]); // Positive acknowledge of clear/reset of a specific active DTC + EXPECT_EQ(0xFF, testFrame.data[1]); // NA + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xD2, testFrame.data[5]); // SPN + EXPECT_EQ(0x04, testFrame.data[6]); // SPN + EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + + // Try and clear a non-existant active DTC (re-clear the one we just cleared) + testFrame.dataLength = 8; + testFrame.identifier = 0x18C3AAAB; + testFrame.data[0] = 17; // Request to clear/reset a specific active DTC 5.7.22.1 + testFrame.data[1] = 0xFF; // Control Byte Specific Indicator for Individual DTC Clear (N/A) + testFrame.data[2] = 0xFF; // Reserved + testFrame.data[3] = 0xFF; // Reserved + testFrame.data[4] = 0xFF; // Reserved + testFrame.data[5] = 0xD2; // SPN + testFrame.data[6] = 0x04; // SPN + testFrame.data[7] = 31; // FMI (5 bits) + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Check for a negative acknowledge that the DTC was cleared + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18C3ABAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(19, testFrame.data[0]); // Negative acknowledge of clear/reset of a specific active DTC + EXPECT_EQ(0x04, testFrame.data[1]); // Diagnostic trouble code no longer active + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xD2, testFrame.data[5]); // SPN + EXPECT_EQ(0x04, testFrame.data[6]); // SPN + EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + + // Try to clear the DTC from the inactive list + testFrame.dataLength = 8; + testFrame.identifier = 0x18C3AAAB; + testFrame.data[0] = 1; // Request to clear/reset a specific previously active DTC 5.7.22.1 + testFrame.data[1] = 0xFF; // Control Byte Specific Indicator for Individual DTC Clear (N/A) + testFrame.data[2] = 0xFF; // Reserved + testFrame.data[3] = 0xFF; // Reserved + testFrame.data[4] = 0xFF; // Reserved + testFrame.data[5] = 0xD2; // SPN + testFrame.data[6] = 0x04; // SPN + testFrame.data[7] = 31; // FMI (5 bits) + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Check for a positive acknowledge that the DTC was cleared + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18C3ABAA, testFrame.identifier); + EXPECT_EQ(2, testFrame.data[0]); // Positive acknowledge of clear/reset of a specific active DTC + EXPECT_EQ(0xFF, testFrame.data[1]); // NA + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xD2, testFrame.data[5]); // SPN + EXPECT_EQ(0x04, testFrame.data[6]); // SPN + EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + + // Try to clear the DTC again from the inactive list (which is not valid) + testFrame.dataLength = 8; + testFrame.identifier = 0x18C3AAAB; + testFrame.data[0] = 1; // Request to clear/reset a specific previously active DTC 5.7.22.1 + testFrame.data[1] = 0xFF; // Control Byte Specific Indicator for Individual DTC Clear (N/A) + testFrame.data[2] = 0xFF; // Reserved + testFrame.data[3] = 0xFF; // Reserved + testFrame.data[4] = 0xFF; // Reserved + testFrame.data[5] = 0xD2; // SPN + testFrame.data[6] = 0x04; // SPN + testFrame.data[7] = 31; // FMI (5 bits) + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Check for a negative acknowledge that the DTC was cleared + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18C3ABAA, testFrame.identifier); + EXPECT_EQ(3, testFrame.data[0]); // Positive acknowledge of clear/reset of a specific active DTC + EXPECT_EQ(0x02, testFrame.data[1]); // Since the DTC is not active, it is unknown to us. + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xD2, testFrame.data[5]); // SPN + EXPECT_EQ(0x04, testFrame.data[6]); // SPN + EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + + // Reset back to a known state + protocolUnderTest.clear_active_diagnostic_trouble_codes(); + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); + } + + { + // Test DM11 + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, true); + + // This tests that when DM1 is requested after a DM11 request is received, no DTCs are active + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xD3; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCA; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + protocolUnderTest.update(); + + // The stack will have sent an ACK since we sent it as destination specific + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18E8FFAA, testFrame.identifier); + EXPECT_EQ(0x00, testFrame.data[0]); // Positive Ack + EXPECT_EQ(0xFF, testFrame.data[1]); + EXPECT_EQ(0xFF, testFrame.data[2]); + EXPECT_EQ(0xFF, testFrame.data[3]); + EXPECT_EQ(0xAB, testFrame.data[4]); // Address + EXPECT_EQ(0xD3, testFrame.data[5]); // PGN + EXPECT_EQ(0xFE, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // Parse DM1 response + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECAAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0xFF, testFrame.data[0]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0x00, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x00, testFrame.data[3]); // SPN + EXPECT_EQ(0x00, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(0x00, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + // Reset back to a known state + protocolUnderTest.clear_active_diagnostic_trouble_codes(); + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); + } + + { + // Test DM3 + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, false); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, false); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, false); + + // Should have some DTCs in the inactive list now, as tested by a previous unit test + + // Send the DM3 request + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCC; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // The stack will have sent an ACK since we sent it as destination specific + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // Screen out DM1 + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18E8FFAA, testFrame.identifier); + EXPECT_EQ(0x00, testFrame.data[0]); // Positive Ack + EXPECT_EQ(0xFF, testFrame.data[1]); + EXPECT_EQ(0xFF, testFrame.data[2]); + EXPECT_EQ(0xFF, testFrame.data[3]); + EXPECT_EQ(0xAB, testFrame.data[4]); // Address + EXPECT_EQ(0xCC, testFrame.data[5]); // PGN + EXPECT_EQ(0xFE, testFrame.data[6]); // PGN + EXPECT_EQ(0x00, testFrame.data[7]); // PGN + + // Request DM2 to see if it has been cleared by our request for DM3 + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCB; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + // Parse DM2 response + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + // Screen out DM1 + if (((testFrame.identifier >> 8) & 0xFFFF) == 0xFECA) + { + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + } + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECBAA, testFrame.identifier); // BAM from address AA + EXPECT_EQ(0xFF, testFrame.data[0]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0xFF, testFrame.data[1]); // Lamp (unused in ISO11783 mode) + EXPECT_EQ(0x00, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x00, testFrame.data[3]); // SPN + EXPECT_EQ(0x00, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(0x00, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + // Reset back to a known state + protocolUnderTest.clear_active_diagnostic_trouble_codes(); + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); + } + + { + // Test DTC Getters and setters + EXPECT_TRUE(protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true)); + EXPECT_TRUE(protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, true)); + EXPECT_TRUE(protocolUnderTest.set_diagnostic_trouble_code_active(testDTC3, true)); + + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC1)); + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC2)); + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC3)); + + EXPECT_TRUE(protocolUnderTest.set_diagnostic_trouble_code_active(testDTC2, false)); + + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC1)); + EXPECT_FALSE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC2)); + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC3)); + + EXPECT_FALSE(protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true)); + + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC1)); + EXPECT_FALSE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC2)); + EXPECT_TRUE(protocolUnderTest.get_diagnostic_trouble_code_active(testDTC3)); + + EXPECT_EQ(1234, testDTC1.get_suspect_parameter_number()); + EXPECT_EQ(567, testDTC2.get_suspect_parameter_number()); + EXPECT_EQ(8910, testDTC3.get_suspect_parameter_number()); + + EXPECT_EQ(isobus::DiagnosticProtocol::FailureModeIdentifier::ConditionExists, testDTC1.get_failure_mode_identifier()); + EXPECT_EQ(isobus::DiagnosticProtocol::FailureModeIdentifier::DataErratic, testDTC2.get_failure_mode_identifier()); + EXPECT_EQ(isobus::DiagnosticProtocol::FailureModeIdentifier::BadIntelligentDevice, testDTC3.get_failure_mode_identifier()); + + // Reset back to a known state + protocolUnderTest.clear_active_diagnostic_trouble_codes(); + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); + } protocolUnderTest.terminate(); EXPECT_FALSE(protocolUnderTest.get_initialized()); CANHardwareInterface::stop(); + + TestInternalECU->destroy(); } From cdbbd25fc84564ef60c999dad952a518f3e67933 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Sun, 2 Jul 2023 10:38:59 -0600 Subject: [PATCH 7/7] [DP]: Fix DM13 command bit shift, additional tests Added some additional unit tests to diagnostic protocol to catch a few more cases with DM2, 22, and 13. Added a missing doxygen comment in ControlFunction's constructor. --- .../isobus/isobus/can_control_function.hpp | 1 + isobus/src/isobus_diagnostic_protocol.cpp | 9 +- test/diagnostic_protocol_tests.cpp | 91 ++++++++++++++++++- 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/isobus/include/isobus/isobus/can_control_function.hpp b/isobus/include/isobus/isobus/can_control_function.hpp index 5e5e45a6..2bb973f5 100644 --- a/isobus/include/isobus/isobus/can_control_function.hpp +++ b/isobus/include/isobus/isobus/can_control_function.hpp @@ -76,6 +76,7 @@ namespace isobus /// @param[in] NAMEValue The NAME of the control function /// @param[in] addressValue The current address of the control function /// @param[in] CANPort The CAN channel index that the control function communicates on + /// @param[in] type The 'Type' of control function to create ControlFunction(NAME NAMEValue, std::uint8_t addressValue, std::uint8_t CANPort, Type type = Type::External); friend class CANNetworkManager; diff --git a/isobus/src/isobus_diagnostic_protocol.cpp b/isobus/src/isobus_diagnostic_protocol.cpp index 13693b07..a44893ba 100644 --- a/isobus/src/isobus_diagnostic_protocol.cpp +++ b/isobus/src/isobus_diagnostic_protocol.cpp @@ -1093,7 +1093,7 @@ namespace isobus { const auto &messageData = message.get_data(); - auto command = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(networkType)))); + auto command = static_cast((messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(networkType)))) >> (DM13_BITS_PER_NETWORK * static_cast(networkType))); switch (command) { case StopStartCommand::StopBroadcast: @@ -1140,7 +1140,7 @@ namespace isobus } // Check current data link - command = static_cast(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(NetworkType::CurrentDataLink)))); + command = static_cast((messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast(NetworkType::CurrentDataLink)))) >> (DM13_BITS_PER_NETWORK * static_cast(NetworkType::CurrentDataLink))); switch (command) { case StopStartCommand::StopBroadcast: @@ -1179,8 +1179,11 @@ namespace isobus break; case StopStartCommand::StartBroadcast: + { broadcastState = true; - break; + customDM13SuspensionTime = 0; + } + break; default: break; diff --git a/test/diagnostic_protocol_tests.cpp b/test/diagnostic_protocol_tests.cpp index 387b7b67..bb009e76 100644 --- a/test/diagnostic_protocol_tests.cpp +++ b/test/diagnostic_protocol_tests.cpp @@ -908,6 +908,36 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) EXPECT_EQ(0x00, testFrame.data[5]); // Occurrence Count + Conversion Method EXPECT_EQ(0xFF, testFrame.data[6]); // Padding EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + // Try in J1939 Mode, make sure lamps are not reserved values + protocolUnderTest.set_j1939_mode(true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, false); + + testFrame.dataLength = 3; + testFrame.identifier = 0x18EAAAAB; + testFrame.data[0] = 0xCB; + testFrame.data[1] = 0xFE; + testFrame.data[2] = 0x00; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18FECBAA, testFrame.identifier); // BAM from address AA + EXPECT_NE(0xFF, testFrame.data[0]); // Lamp + EXPECT_EQ(0xFF, testFrame.data[1]); // Solid Lamp + EXPECT_EQ(0xD2, testFrame.data[2]); // SPN LSB + EXPECT_EQ(0x04, testFrame.data[3]); // SPN + EXPECT_EQ(31, testFrame.data[4]); // SPN + FMI + EXPECT_EQ(0x01, testFrame.data[5]); // Occurrence Count + Conversion Method + EXPECT_EQ(0xFF, testFrame.data[6]); // Padding + EXPECT_EQ(0xFF, testFrame.data[7]); // Padding + + protocolUnderTest.set_j1939_mode(false); + protocolUnderTest.clear_inactive_diagnostic_trouble_codes(); } { @@ -942,7 +972,7 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) testFrame.data[0] = 0xFC; testFrame.data[1] = 0xFF; testFrame.data[2] = 0xFF; - testFrame.data[3] = 0x00; + testFrame.data[3] = 0x03; testFrame.data[4] = 0x0A; testFrame.data[5] = 0x00; testFrame.data[6] = 0xFF; @@ -966,6 +996,36 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) CANNetworkManager::CANNetwork.update(); protocolUnderTest.update(); EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); + + // Test suspending the current data link + testFrame.dataLength = 8; + testFrame.data[0] = 0x3F; + testFrame.data[1] = 0xFF; + testFrame.data[2] = 0xFF; + testFrame.data[3] = 0x00; + testFrame.data[4] = 0x0A; + testFrame.data[5] = 0x00; + testFrame.data[6] = 0xFF; + testFrame.data[7] = 0xFF; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + EXPECT_FALSE(protocolUnderTest.get_broadcast_state()); + + // Restart broadcasts + testFrame.dataLength = 8; + testFrame.data[0] = 0x7F; + testFrame.data[1] = 0xFF; + testFrame.data[2] = 0xFF; + testFrame.data[3] = 0x00; + testFrame.data[4] = 0xFF; + testFrame.data[5] = 0xFF; + testFrame.data[6] = 0xFF; + testFrame.data[7] = 0xFF; + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + EXPECT_TRUE(protocolUnderTest.get_broadcast_state()); } { @@ -1087,6 +1147,35 @@ TEST(DIAGNOSTIC_PROTOCOL_TESTS, MessageEncoding) EXPECT_EQ(0x04, testFrame.data[6]); // SPN EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + protocolUnderTest.set_diagnostic_trouble_code_active(testDTC1, true); + + // Try clearing an inactive DTC that is in the active list to check the error code + testFrame.dataLength = 8; + testFrame.identifier = 0x18C3AAAB; + testFrame.data[0] = 1; // Request to clear/reset a specific previously active DTC 5.7.22.1 + testFrame.data[1] = 0xFF; // Control Byte Specific Indicator for Individual DTC Clear (N/A) + testFrame.data[2] = 0xFF; // Reserved + testFrame.data[3] = 0xFF; // Reserved + testFrame.data[4] = 0xFF; // Reserved + testFrame.data[5] = 0xD2; // SPN + testFrame.data[6] = 0x04; // SPN + testFrame.data[7] = 31; // FMI (5 bits) + CANNetworkManager::process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + protocolUnderTest.update(); + + EXPECT_TRUE(testPlugin.read_frame(testFrame)); + EXPECT_EQ(CAN_DATA_LENGTH, testFrame.dataLength); + EXPECT_EQ(0x18C3ABAA, testFrame.identifier); + EXPECT_EQ(3, testFrame.data[0]); // Positive acknowledge of clear/reset of a specific active DTC + EXPECT_EQ(0x03, testFrame.data[1]); // DTC is not inactive (because it's in the active list) + EXPECT_EQ(0xFF, testFrame.data[2]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[3]); // Reserved + EXPECT_EQ(0xFF, testFrame.data[4]); // Reserved + EXPECT_EQ(0xD2, testFrame.data[5]); // SPN + EXPECT_EQ(0x04, testFrame.data[6]); // SPN + EXPECT_EQ(31, testFrame.data[7]); // 5 bits of FMI + // Reset back to a known state protocolUnderTest.clear_active_diagnostic_trouble_codes(); protocolUnderTest.clear_inactive_diagnostic_trouble_codes();