Skip to content

Commit

Permalink
[DP]: Fix DM13 command bit shift, additional tests
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ad3154
ad3154 committed Jul 2, 2023
1 parent b707108 commit ec4e583
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 4 deletions.
1 change: 1 addition & 0 deletions isobus/include/isobus/isobus/can_control_function.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions isobus/src/isobus_diagnostic_protocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ namespace isobus
{
const auto &messageData = message.get_data();

auto command = static_cast<StopStartCommand>(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(networkType))));
auto command = static_cast<StopStartCommand>((messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(networkType)))) >> (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(networkType)));
switch (command)
{
case StopStartCommand::StopBroadcast:
Expand Down Expand Up @@ -1140,7 +1140,7 @@ namespace isobus
}

// Check current data link
command = static_cast<StopStartCommand>(messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(NetworkType::CurrentDataLink))));
command = static_cast<StopStartCommand>((messageData[0] & (DM13_NETWORK_BITMASK << (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(NetworkType::CurrentDataLink)))) >> (DM13_BITS_PER_NETWORK * static_cast<std::uint8_t>(NetworkType::CurrentDataLink)));
switch (command)
{
case StopStartCommand::StopBroadcast:
Expand Down Expand Up @@ -1179,8 +1179,11 @@ namespace isobus
break;

case StopStartCommand::StartBroadcast:
{
broadcastState = true;
break;
customDM13SuspensionTime = 0;
}
break;

default:
break;
Expand Down
91 changes: 90 additions & 1 deletion test/diagnostic_protocol_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

{
Expand Down Expand Up @@ -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;
Expand All @@ -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());
}

{
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit ec4e583

Please sign in to comment.