OBD2 CAN broken due to math base mismatch (#7053)

* updated can_msg_tx/obd2 code for unit tests, fixed supported pid error, WIP unit tests on obd2

* fix txCanBuffer not cleared after a tx can test

* simulator needs can header, add settings to respect code format on vscode, added more tests

.editorconfig

@@ -11,6 +11,7 @@ trim_trailing_whitespace = true
 insert_final_newline = true
 indent_style = space
 indent_size = 2
+cpp_new_line_before_open_brace_block = same_line
 
 # 4 space indentation
 [*.java]

.vscode/settings.json

@@ -1,17 +1,15 @@
 {
     "files.associations": {
-		"xstring": "cpp",
+		"*.input": "ini",
-		"ios": "cpp",
+		"array": "cpp",
-		"xlocale": "cpp",
+		"*.tcc": "cpp",
-		"xtr1common": "cpp",
+		"memory": "cpp",
-		"xiosbase": "cpp",
+		"istream": "cpp",
-		"deque": "cpp",
+		"functional": "cpp",
-		"forward_list": "cpp",
+		"tuple": "cpp",
-		"list": "cpp",
+		"utility": "cpp",
-		"system_error": "cpp",
+		"variant": "cpp",
-		"vector": "cpp",
+		"format": "cpp"
-		"xhash": "cpp",
-		"xtree": "cpp"
 	},
 	"editor.detectIndentation": false,
 	"editor.indentSize": 4,
@@ -36,4 +34,5 @@
 	"files.associations": {
 		"*.input": "ini"
 	},
+	"C_Cpp.formatting": "vcFormat",
 }

firmware/controllers/can/obd2.cpp

@@ -23,7 +23,7 @@
 
 #include "pch.h"
 
-#if EFI_CAN_SUPPORT
+#if EFI_CAN_SUPPORT || EFI_UNIT_TEST
 
 #include "obd2.h"
 #include "can.h"
@@ -56,7 +56,7 @@ static const int16_t supportedPids4160[] = {
 	-1
 };
 
-static void obdSendPacket(int mode, int PID, int numBytes, uint32_t iValue, size_t busIndex) {
+void obdSendPacket(int mode, int PID, int numBytes, uint32_t iValue, size_t busIndex) {
 	CanTxMessage resp(CanCategory::OBD, OBD_TEST_RESPONSE);
 
 	// Respond on the same bus we got the request from
@@ -84,9 +84,9 @@ static void obdSendValue(int mode, int PID, int numBytes, float value, size_t bu
 }
 
 
-//#define MOCK_SUPPORTED_PIDS 0xffffffff
+// #define MOCK_SUPPORTED_PIDS 0xffffffff
 
-static void obdWriteSupportedPids(int PID, int bitOffset, const int16_t *supportedPids, size_t busIndex) {
+void obdWriteSupportedPids(int PID, int bitOffset, const int16_t *supportedPids, size_t busIndex) {
 	uint32_t value = 0;
 	// gather all 32 bit fields
 	for (int i = 0; i < 32 && supportedPids[i] > 0; i++)
@@ -100,17 +100,17 @@ static void obdWriteSupportedPids(int PID, int bitOffset, const int16_t *support
 	obdSendPacket(1, PID, 4, value, busIndex);
 }
 
-static void handleGetDataRequest(const CANRxFrame& rx, size_t busIndex) {
+void handleGetDataRequest(const CANRxFrame& rx, size_t busIndex) {
 	int pid = rx.data8[2];
 	switch (pid) {
 	case PID_SUPPORTED_PIDS_REQUEST_01_20:
 		obdWriteSupportedPids(pid, 1, supportedPids0120, busIndex);
 		break;
 	case PID_SUPPORTED_PIDS_REQUEST_21_40:
-		obdWriteSupportedPids(pid, 21, supportedPids2140, busIndex);
+		obdWriteSupportedPids(pid, 0x21, supportedPids2140, busIndex);
 		break;
 	case PID_SUPPORTED_PIDS_REQUEST_41_60:
-		obdWriteSupportedPids(pid, 41, supportedPids4160, busIndex);
+		obdWriteSupportedPids(pid, 0x41, supportedPids4160, busIndex);
 		break;
 	case PID_MONITOR_STATUS:
 		obdSendPacket(1, pid, 4, 0, busIndex);	// todo: add statuses
@@ -188,7 +188,7 @@ static void handleDtcRequest(int numCodes, ObdCode* dtcCode) {
 	// }
 }
 
-#if HAL_USE_CAN
+#if HAS_CAN_FRAME
 void obdOnCanPacketRx(const CANRxFrame& rx, size_t busIndex) {
 	if (CAN_SID(rx) != OBD_TEST_REQUEST) {
 		return;
@@ -204,6 +204,6 @@ void obdOnCanPacketRx(const CANRxFrame& rx, size_t busIndex) {
 		handleDtcRequest(1, &engine->engineState.warnings.lastErrorCode);
 	}
 }
-#endif /* HAL_USE_CAN */
+#endif /* HAS_CAN_FRAME */
 
 #endif /* EFI_CAN_SUPPORT */

firmware/controllers/can/obd2.h

@@ -6,6 +6,7 @@
  */
 
 #pragma once
+#include "can.h"
 
 #define OBD_TEST_REQUEST 0x7DF
 
@@ -45,9 +46,16 @@
 #define PID_FUEL_RATE 0x5E
 //todo#define PID_TURBO_RPM 0x74
 
-#if HAL_USE_CAN
+#if HAS_CAN_FRAME
+void obdSendPacket(int mode, int PID, int numBytes, uint32_t iValue, size_t busIndex);
+void obdWriteSupportedPids(int PID, int bitOffset, const int16_t *supportedPids, size_t busIndex);
 void obdOnCanPacketRx(const CANRxFrame& rx, size_t busIndex);
-#endif /* HAL_USE_CAN */
+void handleGetDataRequest(const CANRxFrame& rx, size_t busIndex);
+#endif /* HAS_CAN_FRAME */
+
+#if EFI_UNIT_TEST
+#include "can_msg_tx.h"
+#endif
 
 #define ODB_RPM_MULT 4
 #define ODB_TEMP_EXTRA 40

firmware/hw_layer/drivers/can/can_msg_tx.cpp

@@ -14,7 +14,7 @@
 
 #include "can.h"
 
-#if EFI_SIMULATOR
+#if EFI_SIMULATOR || EFI_UNIT_TEST
 #include "fifo_buffer.h"
 fifo_buffer<CANTxFrame, 1024> txCanBuffer;
 #endif // EFI_SIMULATOR
@@ -60,8 +60,24 @@ CanTxMessage::CanTxMessage(CanCategory p_category, uint32_t eid, uint8_t dlc, si
 }
 
 CanTxMessage::~CanTxMessage() {
-#if EFI_SIMULATOR
+#if EFI_SIMULATOR || EFI_UNIT_TEST
 	txCanBuffer.put(m_frame);
+
+#if EFI_UNIT_TEST
+	printf("%s Sending CAN bus%d message: ID=%x/l=%x %x %x %x %x %x %x %x %x \n",
+		   getCanCategory(category),
+		   busIndex,
+#ifndef STM32H7XX
+		   (unsigned int)((m_frame.IDE == CAN_IDE_EXT) ? CAN_EID(m_frame) : CAN_SID(m_frame)),
+#else
+		   (unsigned int)(m_frame.common.XTD ? CAN_EID(m_frame) : CAN_SID(m_frame)),
+#endif
+		   m_frame.DLC,
+		   m_frame.data8[0], m_frame.data8[1],
+		   m_frame.data8[2], m_frame.data8[3],
+		   m_frame.data8[4], m_frame.data8[5],
+		   m_frame.data8[6], m_frame.data8[7]);
+#endif
 #endif // EFI_SIMULATOR
 
 #if EFI_CAN_SUPPORT

firmware/hw_layer/drivers/can/can_msg_tx.h

@@ -15,6 +15,11 @@
 #include "can_category.h"
 #include "can.h"
 
+#if EFI_SIMULATOR || EFI_UNIT_TEST
+#include "fifo_buffer.h"
+extern fifo_buffer<CANTxFrame, 1024> txCanBuffer;
+#endif // EFI_SIMULATOR
+
 /**
  * Represent a message to be transmitted over CAN.
  *

unit_tests/tests/controllers/can/test_can_serial.cpp

@@ -114,6 +114,9 @@ public:
 		// check the FIFO buf size
 		EXPECT_EQ(0, rxFifoBuf.getCount());
 
+		// clear shared buffer
+		txCanBuffer.clear();
+		EXPECT_FALSE(txCanBuffer.getCount());
 	}
 
 protected:

unit_tests/tests/controllers/can/test_obd2.cpp

@@ -0,0 +1,218 @@
+#include "pch.h"
+
+#include "can_msg_tx.h"
+#include "can.h"
+#include "can_listener.h"
+#include "obd2.h"
+
+
+TEST(CanObd2, obdSendPacket)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    obdSendPacket(0, PID_RPM, 1, 0xff, 0);
+    EXPECT_TRUE(txCanBuffer.getCount());
+
+    CANTxFrame frame = txCanBuffer.get();
+
+    EXPECT_EQ(frame.data8[0], 3);		// correct data size
+    EXPECT_EQ(frame.data8[1], 0X40);	// correct header
+    EXPECT_EQ(frame.data8[2], PID_RPM); // correct PID
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_SUPPORTED_PIDS_01_20)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_SUPPORTED_PIDS_REQUEST_01_20;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 6);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0X41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_SUPPORTED_PIDS_REQUEST_01_20); // correct PID
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_SUPPORTED_PIDS_21_40)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_SUPPORTED_PIDS_REQUEST_21_40;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 6);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_SUPPORTED_PIDS_REQUEST_21_40); // correct PID
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_SUPPORTED_PIDS_41_60)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_SUPPORTED_PIDS_REQUEST_41_60;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 6);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_SUPPORTED_PIDS_REQUEST_41_60); // correct PID
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+
+TEST(CanObd2, handleGetDataRequest_SUPPORTED_PID_ENGINE_LOAD)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_ENGINE_LOAD;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_ENGINE_LOAD); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], getFuelingLoad() * ODB_TPS_BYTE_PERCENT); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+
+TEST(CanObd2, handleGetDataRequest_PID_COOLANT_TEMP)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_COOLANT_TEMP;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_COOLANT_TEMP); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], Sensor::getOrZero(SensorType::Clt) + ODB_TEMP_EXTRA); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_PID_STFT_BANK1_2)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_STFT_BANK1;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_STFT_BANK1); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], 128 * engine->engineState.stftCorrection[0]); // correct value
+
+    frame.data8[2] = PID_STFT_BANK2;
+
+    handleGetDataRequest(frame, 0);
+    rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_STFT_BANK2); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], 128 * engine->engineState.stftCorrection[0]); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_PID_INTAKE_MAP)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_INTAKE_MAP;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_INTAKE_MAP); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], Sensor::getOrZero(SensorType::Map)); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_PID_RPM)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_RPM;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 4);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_RPM); // correct PID
+
+    int rpm = Sensor::getOrZero(SensorType::Rpm) * ODB_RPM_MULT;
+    
+    EXPECT_EQ(rxFrame.data8[3], (rpm >> 0) & 0xff); // correct value
+    EXPECT_EQ(rxFrame.data8[3], (rpm >> 8) & 0xff); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}
+
+TEST(CanObd2, handleGetDataRequest_PID_SPEED)
+{
+    EngineTestHelper eth(engine_type_e::TEST_ENGINE);
+
+    CANRxFrame frame;
+    frame.data8[2] = PID_SPEED;
+
+    handleGetDataRequest(frame, 0);
+    CANTxFrame rxFrame = txCanBuffer.get();
+
+    EXPECT_EQ(rxFrame.data8[0], 3);		// correct data size
+    EXPECT_EQ(rxFrame.data8[1], 0x41);	// correct header
+    EXPECT_EQ(rxFrame.data8[2], PID_SPEED); // correct PID
+    EXPECT_EQ(rxFrame.data8[3], Sensor::getOrZero(SensorType::VehicleSpeed)); // correct value
+
+    // clear shared buffer
+    txCanBuffer.clear();
+    EXPECT_FALSE(txCanBuffer.getCount());
+}

unit_tests/tests/tests.mk

@@ -139,9 +139,6 @@ TESTS_SRC_CPP = \
 	tests/test_dynoview.cpp \
 	tests/test_gpio.cpp \
 	tests/test_limp.cpp \
-	tests/test_can_rx.cpp \
-	tests/test_can_serial.cpp \
-	tests/test_can_wideband.cpp \
 	tests/test_hellen_board_id.cpp \
 	tests/sensor/test_frequency_sensor.cpp \
 	tests/sensor/test_turbocharger_speed_converter.cpp \
@@ -164,3 +161,7 @@ TESTS_SRC_CPP = \
 	tests/actuators/boost/boost_test_base.cpp \
 	tests/actuators/boost/test_open_loop_multipliers.cpp \
 	tests/actuators/boost/test_closed_loop_adders.cpp \
+	tests/controllers/can/test_can_rx.cpp \
+	tests/controllers/can/test_can_serial.cpp \
+	tests/controllers/can/test_can_wideband.cpp \
+	tests/controllers/can/test_obd2.cpp \