fix dual CAN init, update UI (#3719)

* CAN init sequence

* config & UI cleanup

* ui

* simplify

firmware/hw_layer/drivers/can/can_hw.cpp

@@ -120,10 +120,7 @@ static const CANConfig canConfig1000 = {
 };
 #endif
 
-static const CANConfig *can1Config = &canConfig500;
-static const CANConfig *can2Config = &canConfig500;
-
-class CanRead final : public ThreadController<UTILITY_THREAD_STACK_SIZE> {
+class CanRead final : protected ThreadController<UTILITY_THREAD_STACK_SIZE> {
 public:
 	CanRead(size_t index)
 		: ThreadController("CAN RX", PRIO_CAN_RX)
@@ -131,17 +128,18 @@ public:
 	{
 	}
 
-	void ThreadTask() override {
-		CANDriver* device = detectCanDevice(m_index);
+	void Start(CANDriver* device) {
+		m_device = device;
 
-		if (!device) {
-			warning(CUSTOM_ERR_CAN_CONFIGURATION, "Read: CAN configuration issue bus=%d", m_index);
-			return;
+		if (device) {
+			ThreadController::Start();
 		}
+	}
 
+	void ThreadTask() override {
 		while (true) {
 			// Block until we get a message
-			msg_t result = canReceiveTimeout(device, CAN_ANY_MAILBOX, &m_buffer, TIME_INFINITE);
+			msg_t result = canReceiveTimeout(m_device, CAN_ANY_MAILBOX, &m_buffer, TIME_INFINITE);
 
 			if (result != MSG_OK) {
 				continue;
@@ -157,6 +155,7 @@ public:
 private:
 	const size_t m_index;
 	CANRxFrame m_buffer;
+	CANDriver* m_device;
 };
 
 CCM_OPTIONAL static CanRead canRead1(0);
@@ -247,7 +246,7 @@ void startCanPins() {
 }
 
 static const CANConfig * findConfig(can_baudrate_e rate) {
-	switch (engineConfiguration->canBaudRate) {
+	switch (rate) {
 	case B100KBPS:
 		return &canConfig100;
 		break;
@@ -257,6 +256,7 @@ static const CANConfig * findConfig(can_baudrate_e rate) {
 	case B1MBPS:
 		return &canConfig1000;
 		break;
+	case B500KBPS:
 	default:
 		return &canConfig500;
 	}
@@ -267,35 +267,41 @@ void initCan(void) {
 
 	isCanEnabled = false;
 
-	bool isCanConfigGood =
-		(isBrainPinValid(engineConfiguration->canTxPin)) && // both pins are set...
-		(isBrainPinValid(engineConfiguration->canRxPin)) &&
-		(engineConfiguration->canWriteEnabled || engineConfiguration->canReadEnabled) ; // ...and either read or write is enabled
-
-	// nothing to do if we aren't enabled...
-	if (!isCanConfigGood) {
+	// No CAN features enabled, nothing more to do.
+	if (!engineConfiguration->canWriteEnabled && !engineConfiguration->canReadEnabled) {
 		return;
 	}
 
-	can1Config = findConfig(engineConfiguration->canBaudRate);
-	can2Config = findConfig(engineConfiguration->can2BaudRate);
+	// Determine physical CAN peripherals based on selected pins
+	auto device1 = detectCanDevice(engineConfiguration->canRxPin, engineConfiguration->canTxPin);
+	auto device2 = detectCanDevice(engineConfiguration->can2RxPin, engineConfiguration->can2TxPin);
 
-	// Initialize hardware
-#if STM32_CAN_USE_CAN2
-	// CAN1 is required for CAN2
-	canStart(&CAND1, can1Config);
-	canStart(&CAND2, can2Config);
-#else
-	canStart(&CAND1, can1Config);
-#endif /* STM32_CAN_USE_CAN2 */
-
-	if (detectCanDevice(0) == detectCanDevice(1)) {
+	// Devices can't be the same!
+	if (device1 == device2) {
 		firmwareError(OBD_PCM_Processor_Fault, "CAN pins must be set to different devices");
 		return;
 	}
 
-	// Plumb CAN device to tx system
-	CanTxMessage::setDevice(detectCanDevice(0), detectCanDevice(1));
+	// If both devices are null, a firmware error was already thrown by detectCanDevice, but we shouldn't continue
+	if (!device1 && !device2) {
+		return;
+	}
+
+	// Generate configs based on baud rate
+	auto config1 = findConfig(engineConfiguration->canBaudRate);
+	auto config2 = findConfig(engineConfiguration->can2BaudRate);
+
+	// Initialize peripherals
+	if (device1) {
+		canStart(device1, config1);
+	}
+
+	if (device2) {
+		canStart(device2, config2);
+	}
+
+	// Plumb CAN devices to tx system
+	CanTxMessage::setDevice(device1, device2);
 
 	// fire up threads, as necessary
 	if (engineConfiguration->canWriteEnabled) {
@@ -303,8 +309,8 @@ void initCan(void) {
 	}
 
 	if (engineConfiguration->canReadEnabled) {
-		canRead1.Start();
-		canRead2.Start();
+		canRead1.Start(device1);
+		canRead2.Start(device2);
 	}
 
 	isCanEnabled = true;
@@ -314,15 +320,4 @@ bool getIsCanEnabled(void) {
 	return isCanEnabled;
 }
 
-CANDriver* detectCanDevice(size_t logicalIndex) {
-	switch (logicalIndex) {
-	case 0:
-		return detectCanDeviceImpl(engineConfiguration->canRxPin, engineConfiguration->canTxPin);
-	case 1:
-		return detectCanDeviceImpl(engineConfiguration->can2RxPin, engineConfiguration->can2TxPin);
-	}
-
-	return nullptr;
-}
-
 #endif /* EFI_CAN_SUPPORT */

firmware/hw_layer/drivers/can/can_hw.h

@@ -14,7 +14,6 @@ void setCanType(int type);
 void setCanVss(int type);
 
 #if EFI_CAN_SUPPORT
-CANDriver* detectCanDevice(size_t logicalIndex);
 
 void stopCanPins();
 void startCanPins();

firmware/hw_layer/ports/cypress/mpu_util.cpp

@@ -224,7 +224,7 @@ bool isValidCanRxPin(brain_pin_e pin) {
    return isValidCan1RxPin(pin) || isValidCan2RxPin(pin);
 }
 
-CANDriver* detectCanDeviceImpl(brain_pin_e pinRx, brain_pin_e pinTx) {
+CANDriver* detectCanDevice(brain_pin_e pinRx, brain_pin_e pinTx) {
    if (isValidCan1RxPin(pinRx) && isValidCan1TxPin(pinTx))
       return &CAND1;
    if (isValidCan2RxPin(pinRx) && isValidCan2TxPin(pinTx))

firmware/hw_layer/ports/kinetis/mpu_util.cpp

@@ -216,7 +216,7 @@ bool isValidCanRxPin(brain_pin_e pin) {
    return isValidCan1RxPin(pin) || isValidCan2RxPin(pin);
 }
 
-CANDriver* detectCanDeviceImpl(brain_pin_e pinRx, brain_pin_e pinTx) {
+CANDriver* detectCanDevice(brain_pin_e pinRx, brain_pin_e pinTx) {
    if (isValidCan1RxPin(pinRx) && isValidCan1TxPin(pinTx))
       return &CAND1;
    if (isValidCan2RxPin(pinRx) && isValidCan2TxPin(pinTx))

firmware/hw_layer/ports/mpu_util.h

@@ -23,7 +23,7 @@ bool readSlowAnalogInputs(adcsample_t* convertedSamples);
 #if HAL_USE_CAN
 bool isValidCanTxPin(brain_pin_e pin);
 bool isValidCanRxPin(brain_pin_e pin);
-CANDriver* detectCanDeviceImpl(brain_pin_e pinRx, brain_pin_e pinTx);
+CANDriver* detectCanDevice(brain_pin_e pinRx, brain_pin_e pinTx);
 #endif // HAL_USE_CAN
 
 bool isValidSerialTxPin(brain_pin_e pin);

firmware/hw_layer/ports/stm32/stm32_common.cpp

@@ -778,7 +778,7 @@ bool isValidCanRxPin(brain_pin_e pin) {
    return isValidCan1RxPin(pin) || isValidCan2RxPin(pin);
 }
 
-CANDriver* detectCanDeviceImpl(brain_pin_e pinRx, brain_pin_e pinTx) {
+CANDriver* detectCanDevice(brain_pin_e pinRx, brain_pin_e pinTx) {
 	if (pinRx == GPIO_UNASSIGNED && pinTx == GPIO_UNASSIGNED) {
 		return nullptr;
 	}
@@ -790,7 +790,7 @@ CANDriver* detectCanDeviceImpl(brain_pin_e pinRx, brain_pin_e pinTx) {
    if (isValidCan2RxPin(pinRx) && isValidCan2TxPin(pinTx))
       return &CAND2;
 #endif
-   firmwareError(OBD_PCM_Processor_Fault, "invalid CAN pins %s", hwPortname(pinTx));
+   firmwareError(OBD_PCM_Processor_Fault, "invalid CAN pins tx %s and rx %s", hwPortname(pinTx), hwPortname(pinRx));
    return nullptr;
 }
 

firmware/integration/rusefi_config.txt

@@ -724,7 +724,7 @@ custom adc_channel_mode_e 4 bits, U32, @OFFSET@, [0:1], "Off", "Slow", "Fast"
 pin_input_mode_e throttlePedalUpPinMode;
 	uint8_t acIdleExtraOffset;+Additional idle % while A/C is active;"%", 1, 0, 0, 100, 0
 
-	int can2SleepPeriodMs;+CANbus thread period, ms;"ms", 1, 0, 0, 1000, 2
+	int unused720;;"ms", 1, 0, 0, 1000, 2
 	uint16_t wastegatePositionMin;+Voltage when the wastegate is closed.\nYou probably don't have one of these!;"mv", 1, 0, 0, 5000, 0
 	uint16_t wastegatePositionMax;+Voltage when the wastegate is fully open.\nYou probably don't have one of these!\n1 volt = 1000 units;"mv", 1, 0, 0, 5000, 0
 	uint16_t idlePositionMin;+Voltage when the idle valve is closed.\nYou probably don't have one of these!;"mv", 1, 0, 0, 5000, 0
@@ -1200,16 +1200,16 @@ int16_t tps2Max;Full throttle#2. tpsMax value as 10 bit ADC value. Not Voltage!\
 	custom afr_override_e 1 bits, U08, @OFFSET@, [0:2], @@afr_override_e_enum@@
 	afr_override_e afrOverrideMode;+Override the Y axis (load) value used for the AFR table.\nAdvanced users only: If you aren't sure you need this, you probably don't need this.
 
-	uint32_t verboseCan2BaseAddress;;"", 1, 0, 0, 536870911, 0
-	bit enableVerboseCan2Tx;+CAN broadcast using custom rusEFI protocol\nenable can_broadcast/disable can_broadcast
-	bit can2ReadEnabled;enable can_read/disable can_read
-	bit can2WriteEnabled;enable can_write/disable can_write
+	uint32_t unused1736;;"", 1, 0, 0, 536870911, 0
+	bit unused1740b0
+	bit unused1740b1
+	bit unused1740b2
 	bit stepperDcInvertedPins;+Enable if DC-motor driver (H-bridge) inverts the signals (eg. RZ7899 on Hellen boards)
 	bit unused1127
 	bit unused1128
 	bit unused1129
 	bit unused1130
-	can_nbc_e can2NbcType;set can_mode X
+	uint32_t unused1744;;"",1,0,0,0,0
 	brain_pin_e can2TxPin;set_can2_tx_pin X
 	brain_pin_e can2RxPin;set_can2_rx_pin X	
 	pin_output_mode_e starterControlPinMode;

firmware/tunerstudio/rusefi.input

@@ -3030,35 +3030,35 @@ cmd_set_engine_type_default					= "@@TS_IO_TEST_COMMAND_char@@\x00\x31\x00\x00"
 		field = "TX pin",                               binarySerialTxPin, {useSerialPort == 1}
 		field = "RX pin",                               binarySerialRxPin, {useSerialPort == 1}
 
+	dialog = canHw1, "Primary CAN"
+		field = "Bitrate",		canBaudRate
+		field = "RX pin",		canRxPin @@if_ts_show_can_pins
+		field = "TX pin",		canTxPin @@if_ts_show_can_pins
+
+	dialog = canHw2, "Secondary CAN"
+		field = "Bitrate",	can2BaudRate
+		field = "RX pin",		can2RxPin @@if_ts_show_can_pins
+		field = "TX pin",		can2TxPin @@if_ts_show_can_pins
+
 	dialog = canBus, "CAN Bus"
 		field = "CAN read enabled",						canReadEnabled
 		field = "CAN write enabled",					canWriteEnabled
-		field = "CAN bitrate",							canBaudRate
 		field = "CAN dash type",						canNbcType
 		field = "Verbose Can",                          verboseCan
 		field = "inertia measurement unit",             imuType
 		field = "Enable rusEFI CAN broadcast",			enableVerboseCanTx
-		field = "Which CAN channel to broadcast on",    canBroadcastUseChannelTwo
+		field = "rusEFI CAN data bus",    canBroadcastUseChannelTwo
 		field = "rusEFI CAN data base address",			verboseCanBaseAddress
 		field = "rusEFI CAN data address type",			rusefiVerbose29b
 		field = "rusEFI CAN data period",				canSleepPeriodMs
-		field = "RX pin",								canRxPin @@if_ts_show_can_pins
-		field = "TX pin",								canTxPin @@if_ts_show_can_pins
 
 	dialog = canBus2, "Secondary CAN Bus"
-		field = "CAN read enabled",						can2ReadEnabled
-		field = "CAN write enabled",					can2WriteEnabled
-		field = "CAN bitrate",							can2BaudRate
-		field = "CAN dash type",						can2NbcType
-		field = "Enable rusEFI CAN broadcast",			enableVerboseCan2Tx
-		field = "rusEFI CAN data base address",			verboseCan2BaseAddress
-		field = "rusEFI CAN data period",				can2SleepPeriodMs
-		field = "RX pin",								can2RxPin @@if_ts_show_can_pins
-		field = "TX pin",								can2TxPin @@if_ts_show_can_pins
 
 	dialog = canBusMain, "CAN Bus Communication", yAxis
 		panel = canBus
-		panel = canBus2							@@if_ts_show_can2
+
+		panel = canHw1
+		panel = canHw2	@@if_ts_show_can2
 
 	dialog = auxSerial, "AUX Sensor Serial"
 		field = "RX pin",								auxSerialRxPin @@if_ts_show_auxserial_pins