From 6b6fd5e6e8da8de620acbd98bd9e207389133d05 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Fri, 24 Dec 2021 20:33:54 -0800 Subject: [PATCH] fix dual CAN init, update UI (#3719) * CAN init sequence * config & UI cleanup * ui * simplify --- firmware/hw_layer/drivers/can/can_hw.cpp | 85 +++++++++---------- firmware/hw_layer/drivers/can/can_hw.h | 1 - firmware/hw_layer/ports/cypress/mpu_util.cpp | 2 +- firmware/hw_layer/ports/kinetis/mpu_util.cpp | 2 +- firmware/hw_layer/ports/mpu_util.h | 2 +- .../hw_layer/ports/stm32/stm32_common.cpp | 4 +- firmware/integration/rusefi_config.txt | 12 +-- firmware/tunerstudio/rusefi.input | 28 +++--- 8 files changed, 65 insertions(+), 71 deletions(-) diff --git a/firmware/hw_layer/drivers/can/can_hw.cpp b/firmware/hw_layer/drivers/can/can_hw.cpp index 8b56dbacdd..8456e58b0e 100644 --- a/firmware/hw_layer/drivers/can/can_hw.cpp +++ b/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 { +class CanRead final : protected ThreadController { 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 */ diff --git a/firmware/hw_layer/drivers/can/can_hw.h b/firmware/hw_layer/drivers/can/can_hw.h index 5042254f43..2c6ee89c14 100644 --- a/firmware/hw_layer/drivers/can/can_hw.h +++ b/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(); diff --git a/firmware/hw_layer/ports/cypress/mpu_util.cpp b/firmware/hw_layer/ports/cypress/mpu_util.cpp index dc368899a7..89404389a5 100644 --- a/firmware/hw_layer/ports/cypress/mpu_util.cpp +++ b/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)) diff --git a/firmware/hw_layer/ports/kinetis/mpu_util.cpp b/firmware/hw_layer/ports/kinetis/mpu_util.cpp index 222902d4fb..1e3b9e671a 100644 --- a/firmware/hw_layer/ports/kinetis/mpu_util.cpp +++ b/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)) diff --git a/firmware/hw_layer/ports/mpu_util.h b/firmware/hw_layer/ports/mpu_util.h index f7b78b8263..9783d8d518 100644 --- a/firmware/hw_layer/ports/mpu_util.h +++ b/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); diff --git a/firmware/hw_layer/ports/stm32/stm32_common.cpp b/firmware/hw_layer/ports/stm32/stm32_common.cpp index d4ae72783a..7c7ca5ad32 100644 --- a/firmware/hw_layer/ports/stm32/stm32_common.cpp +++ b/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; } diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index eb6ccd3903..1151614c61 100644 --- a/firmware/integration/rusefi_config.txt +++ b/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; diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index 2376bc6a5b..3a721d5cd2 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/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