From 9410b8732ba9582e368ad22c0d057894e8f98560 Mon Sep 17 00:00:00 2001 From: mvgalen Date: Tue, 14 Jan 2025 22:26:31 +0100 Subject: [PATCH] Changes due to review comments - Split bms_status (system status) to real_bms_status (bms status) - Put #ifdef DEBUG_LOG around all log calls. - Do not update bms_status (system status) directly, fire event instead from automatic precharge control - Add Battery BMS status to webserver for MEB battery type only --- Software/Software.ino | 27 ++--------- Software/src/battery/MEB-BATTERY.cpp | 46 +++++++++++++------ Software/src/communication/can/obd.cpp | 2 + .../precharge_control/precharge_control.cpp | 28 +++++++++-- Software/src/datalayer/datalayer.h | 5 +- Software/src/devboard/safety/safety.cpp | 4 ++ Software/src/devboard/utils/events.cpp | 9 ++-- Software/src/devboard/utils/events.h | 1 + Software/src/devboard/utils/logging.cpp | 28 +++++++++++ Software/src/devboard/utils/logging.h | 2 + Software/src/devboard/utils/types.h | 1 + 11 files changed, 103 insertions(+), 50 deletions(-) diff --git a/Software/Software.ino b/Software/Software.ino index c1bebda9..50c5d95b 100644 --- a/Software/Software.ino +++ b/Software/Software.ino @@ -306,30 +306,9 @@ void core_loop(void* task_time_us) { if (check_pause_2s.elapsed()) { emulator_pause_state_transmit_can_battery(); } - static bms_status_enum previous_state = FAULT; - if (previous_state != datalayer.battery.status.bms_status) { - switch (datalayer.battery.status.bms_status) { - case ACTIVE: - logging.printf("BMS state changed to: OK\n"); - break; - case UPDATING: - logging.printf("BMS state changed to: UPDATING\n"); - break; - case FAULT: - logging.printf("BMS state changed to: FAULT\n"); - break; - case INACTIVE: - logging.printf("BMS state changed to: INACTIVE\n"); - break; - case STANDBY: - logging.printf("BMS state changed to: STANDBY\n"); - break; - default: - logging.printf("BMS state changed to: ??\n"); - break; - } - previous_state = datalayer.battery.status.bms_status; - } +#ifdef DEBUG_LOG + logging.log_bms_status(datalayer.battery.status.real_bms_status, 1); +#endif vTaskDelayUntil(&xLastWakeTime, xFrequency); } diff --git a/Software/src/battery/MEB-BATTERY.cpp b/Software/src/battery/MEB-BATTERY.cpp index 7e4d53ba..7ee4a89c 100644 --- a/Software/src/battery/MEB-BATTERY.cpp +++ b/Software/src/battery/MEB-BATTERY.cpp @@ -643,7 +643,9 @@ void update_values_battery() { //This function maps all the values fetched via void handle_incoming_can_frame_battery(CAN_frame rx_frame) { last_can_msg_timestamp = millis(); if (first_can_msg == 0) { +#ifdef DEBUG_LOG logging.printf("MEB: First CAN msg received\n"); +#endif first_can_msg = last_can_msg_timestamp; } switch (rx_frame.ID) { @@ -1013,31 +1015,39 @@ void handle_incoming_can_frame_battery(CAN_frame rx_frame) { case 3: // EXTERN CHARGING case 4: // AC_CHARGING case 6: // DC_CHARGING +#ifdef DEBUG_LOG if (!datalayer.system.status.battery_allows_contactor_closing) logging.printf("MEB Contactors closed\n"); - if (datalayer.battery.status.bms_status != FAULT) - datalayer.battery.status.bms_status = ACTIVE; +#endif + if (datalayer.battery.status.real_bms_status != BMS_FAULT) + datalayer.battery.status.real_bms_status = BMS_ACTIVE; datalayer.system.status.battery_allows_contactor_closing = true; break; case 5: // Error +#ifdef DEBUG_LOG if (datalayer.system.status.battery_allows_contactor_closing) logging.printf("MEB Contactors opened\n"); - datalayer.battery.status.bms_status = FAULT; +#endif + datalayer.battery.status.real_bms_status = BMS_FAULT; datalayer.system.status.battery_allows_contactor_closing = false; break; case 7: // Init +#ifdef DEBUG_LOG if (datalayer.system.status.battery_allows_contactor_closing) logging.printf("MEB Contactors opened\n"); - if (datalayer.battery.status.bms_status != FAULT) - datalayer.battery.status.bms_status = INACTIVE; +#endif + if (datalayer.battery.status.real_bms_status != BMS_FAULT) + datalayer.battery.status.real_bms_status = BMS_STANDBY; datalayer.system.status.battery_allows_contactor_closing = false; break; case 2: // BALANCING default: +#ifdef DEBUG_LOG if (datalayer.system.status.battery_allows_contactor_closing) logging.printf("MEB Contactors opened\n"); - if (datalayer.battery.status.bms_status != FAULT) - datalayer.battery.status.bms_status = STANDBY; +#endif + if (datalayer.battery.status.real_bms_status != BMS_FAULT) + datalayer.battery.status.real_bms_status = BMS_STANDBY; datalayer.system.status.battery_allows_contactor_closing = false; } BMS_HVIL_status = (rx_frame.data.u8[2] & 0x18) >> 3; @@ -1511,14 +1521,16 @@ void handle_incoming_can_frame_battery(CAN_frame rx_frame) { handle_obd_frame(rx_frame); break; default: +#ifdef DEBUG_LOG logging.printf("Unknown CAN frame received:\n"); +#endif dump_can_frame(rx_frame, MSG_RX); break; } datalayer.battery.status.CAN_battery_still_alive = CAN_STILL_ALIVE; if (can_msg_received == 0xFFFF && nof_cells_determined) { - if (datalayer.battery.status.bms_status == INACTIVE) - datalayer.battery.status.bms_status = STANDBY; + if (datalayer.battery.status.real_bms_status == BMS_DISCONNECTED) + datalayer.battery.status.real_bms_status = BMS_STANDBY; } } @@ -1526,12 +1538,16 @@ void transmit_can_battery() { unsigned long currentMillis = millis(); // Send 10ms CAN Message if (datalayer.system.settings.equipment_stop_active || currentMillis > last_can_msg_timestamp + 500) { +#ifdef DEBUG_LOG if (first_can_msg) logging.printf("MEB: No CAN msg received for 500ms\n"); +#endif can_msg_received = RX_DEFAULT; first_can_msg = 0; - if (datalayer.battery.status.bms_status != FAULT) - datalayer.battery.status.bms_status = INACTIVE; + if (datalayer.battery.status.real_bms_status != BMS_FAULT){ + datalayer.battery.status.real_bms_status = BMS_DISCONNECTED; + datalayer.system.status.battery_allows_contactor_closing = false; + } } if (currentMillis - previousMillis10ms >= INTERVAL_10_MS) { @@ -1601,15 +1617,17 @@ void transmit_can_battery() { //HV request and DC/DC control lies in 0x503 - if (datalayer.battery.status.bms_status != FAULT && /*first_can_msg > 0 && currentMillis > first_can_msg + 2000*/ - (datalayer.battery.status.bms_status == STANDBY || datalayer.battery.status.bms_status == ACTIVE) && + if (datalayer.battery.status.real_bms_status != BMS_FAULT && /*first_can_msg > 0 && currentMillis > first_can_msg + 2000*/ + (datalayer.battery.status.real_bms_status == BMS_STANDBY || datalayer.battery.status.real_bms_status == BMS_ACTIVE) && (labs(((int32_t)datalayer.battery.status.voltage_dV) - ((int32_t)datalayer_extended.meb.BMS_voltage_intermediate_dV)) < 200)) { +#ifdef DEBUG_LOG if (MEB_503.data.u8[3] == BMS_TARGET_HV_OFF) { logging.printf("MEB Requesting HV\n"); } +#endif MEB_503.data.u8[1] = - 0x30 | (datalayer.battery.status.bms_status == ACTIVE ? 0x00 : 0x80); // Disable precharing if ACTIVE + 0x30 | (datalayer.battery.status.real_bms_status == BMS_ACTIVE ? 0x00 : 0x80); // Disable precharing if ACTIVE MEB_503.data.u8[3] = BMS_TARGET_HV_ON; //TODO, should we try AC_2 or DC charging? MEB_503.data.u8[5] = 0x82; // Bordnetz Active MEB_503.data.u8[6] = 0xE0; // Request emergency shutdown HV system == 0, false diff --git a/Software/src/communication/can/obd.cpp b/Software/src/communication/can/obd.cpp index 1665d518..cf0f058f 100644 --- a/Software/src/communication/can/obd.cpp +++ b/Software/src/communication/can/obd.cpp @@ -23,6 +23,7 @@ void show_dtc(uint8_t byte0, uint8_t byte1) { } void handle_obd_frame(CAN_frame& rx_frame) { +#ifdef DEBUG_LOG if (rx_frame.data.u8[1] == 0x7F) { const char* error_str = "?"; switch (rx_frame.data.u8[3]) { // See https://automotive.wiki/index.php/ISO_14229 @@ -105,6 +106,7 @@ void handle_obd_frame(CAN_frame& rx_frame) { } } dump_can_frame(rx_frame, MSG_RX); +#endif } void transmit_obd_can_frame(unsigned int address, int interface) { diff --git a/Software/src/communication/precharge_control/precharge_control.cpp b/Software/src/communication/precharge_control/precharge_control.cpp index 46f091d1..8d7bbefe 100644 --- a/Software/src/communication/precharge_control/precharge_control.cpp +++ b/Software/src/communication/precharge_control/precharge_control.cpp @@ -27,9 +27,12 @@ static int32_t prev_external_voltage = 20000; void init_precharge_control() { // Setup PWM Channel Frequency and Resolution +#ifdef DEBUG_LOG logging.printf("Precharge control initialised\n"); +#endif pinMode(PRECHARGE_PIN, OUTPUT); digitalWrite(PRECHARGE_PIN, LOW); + digitalWrite(POSITIVE_CONTACTOR_PIN, LOW); } // Main functions @@ -44,8 +47,8 @@ void handle_precharge_control() { switch (prechargeStatus) { case PRECHARGE_IDLE: - if (datalayer.battery.status.bms_status == STANDBY && datalayer.system.status.inverter_allows_contactor_closing && - !datalayer.system.settings.equipment_stop_active) { + if (datalayer.battery.status.bms_status != FAULT && datalayer.battery.status.real_bms_status == BMS_STANDBY && + datalayer.system.status.inverter_allows_contactor_closing && !datalayer.system.settings.equipment_stop_active) { prechargeStatus = START_PRECHARGE; } break; @@ -56,7 +59,9 @@ void handle_precharge_control() { ledcWriteTone(PRECHARGE_PIN, freq); // Set frequency and set dutycycle to 50% prechargeStartTime = currentTime; prechargeStatus = PRECHARGE; +#ifdef DEBUG_LOG logging.printf("Precharge: Starting sequence\n"); +#endif digitalWrite(POSITIVE_CONTACTOR_PIN, HIGH); break; @@ -82,39 +87,50 @@ void handle_precharge_control() { freq = Precharge_max_PWM_Freq; if (freq < Precharge_min_PWM_Freq) freq = Precharge_min_PWM_Freq; +#ifdef DEBUG_LOG logging.printf("Precharge: Target: %d V Extern: %d V Frequency: %u\n", target_voltage / 10, external_voltage / 10, freq); +#endif ledcWriteTone(PRECHARGE_PIN, freq); } - if ((datalayer.battery.status.bms_status != STANDBY && datalayer.battery.status.bms_status != ACTIVE) || - datalayer.system.settings.equipment_stop_active) { + if ((datalayer.battery.status.real_bms_status != BMS_STANDBY && datalayer.battery.status.real_bms_status != BMS_ACTIVE) || + datalayer.battery.status.bms_status != ACTIVE || datalayer.system.settings.equipment_stop_active) { pinMode(PRECHARGE_PIN, OUTPUT); digitalWrite(PRECHARGE_PIN, LOW); digitalWrite(POSITIVE_CONTACTOR_PIN, LOW); prechargeStatus = PRECHARGE_IDLE; +#ifdef DEBUG_LOG logging.printf("Precharge: Disabling Precharge bms not standby/active or equipment stop\n"); +#endif } else if (currentTime - prechargeStartTime >= MAX_PRECHARGE_TIME_MS) { pinMode(PRECHARGE_PIN, OUTPUT); digitalWrite(PRECHARGE_PIN, LOW); digitalWrite(POSITIVE_CONTACTOR_PIN, LOW); prechargeStatus = PRECHARGE_OFF; - datalayer.battery.status.bms_status = FAULT; +#ifdef DEBUG_LOG logging.printf("Precharge: Disabled (timeout reached) -> PRECHARGE_OFF\n"); +#endif + set_event(EVENT_AUTOMATIC_PRECHARGE_FAILURE, 0); + // Add event } else if (datalayer.system.status.battery_allows_contactor_closing) { pinMode(PRECHARGE_PIN, OUTPUT); digitalWrite(PRECHARGE_PIN, LOW); digitalWrite(POSITIVE_CONTACTOR_PIN, LOW); prechargeStatus = COMPLETED; +#ifdef DEBUG_LOG logging.printf("Precharge: Disabled (contacts closed) -> COMPLETED\n"); +#endif } break; case COMPLETED: if (datalayer.system.settings.equipment_stop_active || datalayer.battery.status.bms_status != ACTIVE) { prechargeStatus = PRECHARGE_IDLE; +#ifdef DEBUG_LOG logging.printf("Precharge: equipment stop activated -> IDLE\n"); +#endif } break; @@ -125,7 +141,9 @@ void handle_precharge_control() { prechargeStatus = PRECHARGE_IDLE; pinMode(PRECHARGE_PIN, OUTPUT); digitalWrite(PRECHARGE_PIN, LOW); +#ifdef DEBUG_LOG logging.printf("Precharge: equipment stop activated -> IDLE\n"); +#endif } break; diff --git a/Software/src/datalayer/datalayer.h b/Software/src/datalayer/datalayer.h index d18bf87f..bd8e8a18 100644 --- a/Software/src/datalayer/datalayer.h +++ b/Software/src/datalayer/datalayer.h @@ -92,8 +92,11 @@ typedef struct { uint8_t CAN_battery_still_alive = CAN_STILL_ALIVE; /** Other */ - /** The current BMS status */ + /** The current system status, which for now still has the name bms_status */ bms_status_enum bms_status = ACTIVE; + + /** The current battery status, which for now has the name real_bms_status */ + real_bms_status_enum real_bms_status = BMS_DISCONNECTED; } DATALAYER_BATTERY_STATUS_TYPE; typedef struct { diff --git a/Software/src/devboard/safety/safety.cpp b/Software/src/devboard/safety/safety.cpp index 2cea48aa..b5393645 100644 --- a/Software/src/devboard/safety/safety.cpp +++ b/Software/src/devboard/safety/safety.cpp @@ -343,12 +343,16 @@ void emulator_pause_state_transmit_can_battery() { allowed_to_send_CAN = (!emulator_pause_CAN_send_ON || emulator_pause_status == NORMAL); if (previous_allowed_to_send_CAN && !allowed_to_send_CAN) { +#ifdef DEBUG_LOG logging.printf("Safety: Pausing CAN sending"); +#endif //completely force stop the CAN communication ESP32Can.CANStop(); } else if (!previous_allowed_to_send_CAN && allowed_to_send_CAN) { //resume CAN communication +#ifdef DEBUG_LOG logging.printf("Safety: Resuming CAN sending"); +#endif ESP32Can.CANInit(); } } diff --git a/Software/src/devboard/utils/events.cpp b/Software/src/devboard/utils/events.cpp index 1b9d6e90..3adc269d 100644 --- a/Software/src/devboard/utils/events.cpp +++ b/Software/src/devboard/utils/events.cpp @@ -168,6 +168,7 @@ void init_events(void) { events.entries[EVENT_SOH_LOW].level = EVENT_LEVEL_ERROR; events.entries[EVENT_HVIL_FAILURE].level = EVENT_LEVEL_ERROR; events.entries[EVENT_PRECHARGE_FAILURE].level = EVENT_LEVEL_INFO; + events.entries[EVENT_AUTOMATIC_PRECHARGE_FAILURE].level = EVENT_LEVEL_ERROR; events.entries[EVENT_INTERNAL_OPEN_FAULT].level = EVENT_LEVEL_ERROR; events.entries[EVENT_INVERTER_OPEN_CONTACTOR].level = EVENT_LEVEL_INFO; events.entries[EVENT_INTERFACE_MISSING].level = EVENT_LEVEL_INFO; @@ -340,6 +341,8 @@ const char* get_event_message_string(EVENTS_ENUM_TYPE event) { "Battery will be disabled!"; case EVENT_PRECHARGE_FAILURE: return "Battery failed to precharge. Check that capacitor is seated on high voltage output."; + case EVENT_AUTOMATIC_PRECHARGE_FAILURE: + return "Automatic precharge failed to reach target voltae."; case EVENT_INTERNAL_OPEN_FAULT: return "High voltage cable removed while battery running. Opening contactors!"; case EVENT_INVERTER_OPEN_CONTACTOR: @@ -501,15 +504,9 @@ static void update_bms_status(void) { break; case EVENT_LEVEL_UPDATE: datalayer.battery.status.bms_status = UPDATING; -#ifdef DOUBLE_BATTERY - datalayer.battery2.status.bms_status = UPDATING; -#endif break; case EVENT_LEVEL_ERROR: datalayer.battery.status.bms_status = FAULT; -#ifdef DOUBLE_BATTERY - datalayer.battery2.status.bms_status = FAULT; -#endif break; default: break; diff --git a/Software/src/devboard/utils/events.h b/Software/src/devboard/utils/events.h index 2e116581..3b7ed6d4 100644 --- a/Software/src/devboard/utils/events.h +++ b/Software/src/devboard/utils/events.h @@ -112,6 +112,7 @@ XX(EVENT_MQTT_CONNECT) \ XX(EVENT_MQTT_DISCONNECT) \ XX(EVENT_EQUIPMENT_STOP) \ + XX(EVENT_AUTOMATIC_PRECHARGE_FAILURE) \ XX(EVENT_NOF_EVENTS) typedef enum { EVENTS_ENUM_TYPE(GENERATE_ENUM) } EVENTS_ENUM_TYPE; diff --git a/Software/src/devboard/utils/logging.cpp b/Software/src/devboard/utils/logging.cpp index c6811697..5b9968ec 100644 --- a/Software/src/devboard/utils/logging.cpp +++ b/Software/src/devboard/utils/logging.cpp @@ -134,3 +134,31 @@ void Logging::printf(const char* fmt, ...) { previous_message_was_newline = message_buffer[size - 1] == '\n'; #endif // DEBUG_LOG } + +void Logging::log_bms_status(real_bms_status_enum bms_status, int battery_id){ + static real_bms_status_enum previous_state = BMS_FAULT; + const char *id = ""; + if (battery_id == 2){ + id = "2"; + } + if (previous_state != bms_status) { + switch (bms_status) { + case BMS_ACTIVE: + logging.printf("Battery%s BMS state changed to: OK\n", id); + break; + case BMS_DISCONNECTED: + logging.printf("Battery%s BMS state changed to: DISCONNECTED\n"); + break; + case BMS_FAULT: + logging.printf("Battery%s BMS state changed to: FAULT\n"); + break; + case BMS_STANDBY: + logging.printf("Battery%s BMS state changed to: STANDBY\n"); + break; + default: + logging.printf("Battery%s BMS state changed to: ??\n"); + break; + } + previous_state = bms_status; + } +} \ No newline at end of file diff --git a/Software/src/devboard/utils/logging.h b/Software/src/devboard/utils/logging.h index 84fc9a97..41369965 100644 --- a/Software/src/devboard/utils/logging.h +++ b/Software/src/devboard/utils/logging.h @@ -3,6 +3,7 @@ #include #include "Print.h" +#include "types.h" class Logging : public Print { void add_timestamp(size_t size); @@ -11,6 +12,7 @@ class Logging : public Print { virtual size_t write(const uint8_t* buffer, size_t size); virtual size_t write(uint8_t) { return 0; } void printf(const char* fmt, ...); + void log_bms_status(real_bms_status_enum bms_status, int battery_id); Logging() {} }; diff --git a/Software/src/devboard/utils/types.h b/Software/src/devboard/utils/types.h index 45a44394..629fea04 100644 --- a/Software/src/devboard/utils/types.h +++ b/Software/src/devboard/utils/types.h @@ -4,6 +4,7 @@ #include enum bms_status_enum { STANDBY = 0, INACTIVE = 1, DARKSTART = 2, ACTIVE = 3, FAULT = 4, UPDATING = 5 }; +enum real_bms_status_enum { BMS_DISCONNECTED = 0, BMS_STANDBY = 1, BMS_ACTIVE = 2, BMS_FAULT = 3 }; enum battery_chemistry_enum { NCA, NMC, LFP }; enum led_color { GREEN, YELLOW, RED, BLUE };