Merge pull request #91 from openppg/bms-cell-low-temps

Handle bad BMS cell temperature probes

Author: zjwhitehead

inc/sp140/bms.h

@@ -3,13 +3,47 @@
 #include <Arduino.h>
 #include <SPI.h>
 #include <BMS_CAN.h>
+#include <math.h>
 
 #include "sp140/structs.h"
 
 // BMS-related constants
 #define MCP_CS 5        // MCP2515 CS pin
 #define MCP_BAUDRATE 250000
 
+// BMS cell probe disconnect policy.
+// The library returns NaN for disconnected probes. This app tolerates up to
+// BMS_MAX_IGNORED_DISCONNECTED_PROBES disconnected; beyond that the NaN is
+// replaced with the sentinel value so downstream temp monitors fire an alert.
+constexpr uint8_t BMS_CELL_PROBE_COUNT = 4;
+constexpr uint8_t BMS_MAX_IGNORED_DISCONNECTED_PROBES = 2;
+
+inline void sanitizeCellProbeTemps(
+    const float temps[BMS_CELL_PROBE_COUNT],
+    float out[BMS_CELL_PROBE_COUNT]) {
+  uint8_t disconnectedCount = 0;
+
+  // First pass: count disconnected probes (NaN from library)
+  for (uint8_t i = 0; i < BMS_CELL_PROBE_COUNT; i++) {
+    if (isnan(temps[i])) disconnectedCount++;
+  }
+
+  // Second pass: if within tolerance, pass NaN through (silently ignored).
+  // If too many are disconnected, replace excess NaN with the sentinel value
+  // so temperature monitors fire a CRIT_LOW alert.
+  uint8_t ignoredCount = 0;
+  for (uint8_t i = 0; i < BMS_CELL_PROBE_COUNT; i++) {
+    if (!isnan(temps[i])) {
+      out[i] = temps[i];
+    } else if (ignoredCount < BMS_MAX_IGNORED_DISCONNECTED_PROBES) {
+      out[i] = NAN;
+      ignoredCount++;
+    } else {
+      out[i] = BMS_CAN::TEMP_PROBE_DISCONNECTED;
+    }
+  }
+}
+
 // External declarations
 extern STR_BMS_TELEMETRY_140 bmsTelemetryData;
 extern BMS_CAN* bms_can;

inc/sp140/structs.h

@@ -76,8 +76,8 @@ typedef struct {
   float power;                  // Power (kW)
   float highest_cell_voltage;   // Highest individual cell voltage (V)
   float lowest_cell_voltage;    // Lowest individual cell voltage (V)
-  float highest_temperature;    // Highest temperature reading (°C)
-  float lowest_temperature;     // Lowest temperature reading (°C)
+  float highest_temperature;    // Highest valid temperature reading (°C), NaN if unavailable
+  float lowest_temperature;     // Lowest valid temperature reading (°C), NaN if unavailable
   float energy_cycle;          // Energy per cycle (kWh)
   uint32_t battery_cycle;      // Battery cycle count
   uint8_t battery_fail_level;  // Battery failure status
@@ -92,10 +92,10 @@ typedef struct {
   // Individual temperature sensors
   float mos_temperature;        // BMS MOSFET temperature (°C) - index 0
   float balance_temperature;    // BMS balance resistor temperature (°C) - index 1
-  float t1_temperature;         // T1 cell temperature sensor (°C) - index 2
-  float t2_temperature;         // T2 cell temperature sensor (°C) - index 3
-  float t3_temperature;         // T3 cell temperature sensor (°C) - index 4
-  float t4_temperature;         // T4 cell temperature sensor (°C) - index 5
+  float t1_temperature;         // T1 cell temperature sensor (°C), NaN if disconnected - index 2
+  float t2_temperature;         // T2 cell temperature sensor (°C), NaN if disconnected - index 3
+  float t3_temperature;         // T3 cell temperature sensor (°C), NaN if disconnected - index 4
+  float t4_temperature;         // T4 cell temperature sensor (°C), NaN if disconnected - index 5
 } STR_BMS_TELEMETRY_140;
 #pragma pack(pop)
 

platformio.ini

@@ -21,7 +21,7 @@ lib_ignore =
 
 
 [env:OpenPPG-CESP32S3-CAN-SP140]
-platform = espressif32@6.12.0
+platform = espressif32@6.13.0
 board = m5stack-stamps3
 framework = arduino
 src_folder = sp140
@@ -43,19 +43,19 @@ debug_tool = esp-builtin
 lib_deps =
 	Wire
 	SPI
-	ArduinoJson@7.3.1
+	ArduinoJson@7.4.3
 	Time@1.6.1
-	adafruit/Adafruit BusIO@1.17.2
+	adafruit/Adafruit BusIO@1.17.4
 	adafruit/Adafruit BMP3XX Library@2.1.6
 	adafruit/Adafruit ST7735 and ST7789 Library@1.11.0
-	adafruit/Adafruit NeoPixel@1.15.2
-	adafruit/Adafruit CAN@0.2.1
+	adafruit/Adafruit NeoPixel@1.15.4
+	adafruit/Adafruit CAN@0.2.3
 	adafruit/Adafruit MCP2515@0.2.1
 	https://github.com/rlogiacco/CircularBuffer@1.4.0
 	https://github.com/openppg/SINE-ESC-CAN#8caa93996b5d000fe10ca5265bd1c472dfdf885b
-	https://github.com/openppg/ANT-BMS-CAN#da685ce2a0e87e23df625f33ad751203ad6a4f8f
+	https://github.com/openppg/ANT-BMS-CAN#fd54852bc6f1c9608e37af9ca7c13ea4135c095b
 	lvgl/lvgl@^8.4.0
-	h2zero/NimBLE-Arduino@^2.3.5
+	h2zero/NimBLE-Arduino@^2.3.9
 lib_ignore =
 	Adafruit SleepyDog Library
 	${extra.lib_ignore}

src/sp140/bms.cpp

@@ -3,6 +3,34 @@
 #include "sp140/globals.h"
 #include "sp140/lvgl/lvgl_core.h"  // for spiBusMutex
 
+namespace {
+
+void logBmsCellProbeConnectionTransitions(const float sanitizedCellTemps[BMS_CELL_PROBE_COUNT]) {
+  static bool hasPreviousState = false;
+  static bool wasConnected[BMS_CELL_PROBE_COUNT] = {false, false, false, false};
+
+  for (uint8_t i = 0; i < BMS_CELL_PROBE_COUNT; i++) {
+    const bool connected = !isnan(sanitizedCellTemps[i]);
+
+    if (!hasPreviousState) {
+      wasConnected[i] = connected;
+      continue;
+    }
+
+    if (connected != wasConnected[i]) {
+      USBSerial.printf("[BMS] T%u sensor %s (sanitized=%.1fC)\n",
+                       i + 1,
+                       connected ? "reconnected" : "disconnected",
+                       sanitizedCellTemps[i]);
+      wasConnected[i] = connected;
+    }
+  }
+
+  hasPreviousState = true;
+}
+
+}  // namespace
+
 STR_BMS_TELEMETRY_140 bmsTelemetryData = {
   .bmsState = TelemetryState::NOT_CONNECTED
 };
@@ -51,10 +79,6 @@ void updateBMSData() {
   // Calculated highest cell minus lowest cell voltage
   bmsTelemetryData.voltage_differential = bms_can->getHighestCellVoltage() - bms_can->getLowestCellVoltage();
 
-  // Temperature readings
-  bmsTelemetryData.highest_temperature = bms_can->getHighestTemperature();
-  bmsTelemetryData.lowest_temperature = bms_can->getLowestTemperature();
-
   // Battery statistics
   bmsTelemetryData.battery_cycle = bms_can->getBatteryCycle();
   bmsTelemetryData.energy_cycle = bms_can->getEnergyCycle();
@@ -70,13 +94,31 @@ void updateBMSData() {
     bmsTelemetryData.cell_voltages[i] = bms_can->getCellVoltage(i);
   }
 
-  // Populate individual temperature sensors
+  // Populate temperature sensors (library returns NaN for disconnected probes)
   bmsTelemetryData.mos_temperature = bms_can->getTemperature(0);      // BMS MOSFET
   bmsTelemetryData.balance_temperature = bms_can->getTemperature(1);  // BMS Balance resistors
-  bmsTelemetryData.t1_temperature = bms_can->getTemperature(2);       // Cell probe 1
-  bmsTelemetryData.t2_temperature = bms_can->getTemperature(3);       // Cell probe 2
-  bmsTelemetryData.t3_temperature = bms_can->getTemperature(4);       // Cell probe 3
-  bmsTelemetryData.t4_temperature = bms_can->getTemperature(5);       // Cell probe 4
+
+  // Cell probes: library already returns NaN for disconnected. Apply app-level
+  // policy (tolerate up to N disconnected before alerting).
+  const float cellTemps[BMS_CELL_PROBE_COUNT] = {
+    bms_can->getTemperature(2),
+    bms_can->getTemperature(3),
+    bms_can->getTemperature(4),
+    bms_can->getTemperature(5)
+  };
+  float sanitizedCellTemps[BMS_CELL_PROBE_COUNT];
+  sanitizeCellProbeTemps(cellTemps, sanitizedCellTemps);
+  bmsTelemetryData.t1_temperature = sanitizedCellTemps[0];
+  bmsTelemetryData.t2_temperature = sanitizedCellTemps[1];
+  bmsTelemetryData.t3_temperature = sanitizedCellTemps[2];
+  bmsTelemetryData.t4_temperature = sanitizedCellTemps[3];
+
+  // Emit transition logs to help field-debug intermittent probe wiring issues.
+  logBmsCellProbeConnectionTransitions(sanitizedCellTemps);
+
+  // Library already excludes disconnected probes from high/low temps
+  bmsTelemetryData.highest_temperature = bms_can->getHighestTemperature();
+  bmsTelemetryData.lowest_temperature = bms_can->getLowestTemperature();
 
   bmsTelemetryData.lastUpdateMs = millis();
   unsigned long dur = bmsTelemetryData.lastUpdateMs - tStart;

src/sp140/lvgl/lvgl_updates.cpp

@@ -334,11 +334,25 @@ void updateLvglMainScreen(
   float batteryPercent = unifiedBatteryData.soc;
   float totalVolts = unifiedBatteryData.volts;
   float lowestCellV = bmsTelemetry.lowest_cell_voltage;
-  // Calculate highest cell temperature from T1-T4 only (excluding MOSFET and balance temps)
-  float batteryTemp = bmsTelemetry.t1_temperature;
-  if (bmsTelemetry.t2_temperature > batteryTemp) batteryTemp = bmsTelemetry.t2_temperature;
-  if (bmsTelemetry.t3_temperature > batteryTemp) batteryTemp = bmsTelemetry.t3_temperature;
-  if (bmsTelemetry.t4_temperature > batteryTemp) batteryTemp = bmsTelemetry.t4_temperature;
+  // Calculate battery temp from connected T1-T4 cell probes only.
+  const float cellTemps[] = {
+    bmsTelemetry.t1_temperature,
+    bmsTelemetry.t2_temperature,
+    bmsTelemetry.t3_temperature,
+    bmsTelemetry.t4_temperature
+  };
+  float batteryTemp = NAN;
+  bool hasValidBatteryTemp = false;
+  for (float cellTemp : cellTemps) {
+    if (isnan(cellTemp)) {
+      continue;
+    }
+
+    if (!hasValidBatteryTemp || cellTemp > batteryTemp) {
+      batteryTemp = cellTemp;
+      hasValidBatteryTemp = true;
+    }
+  }
   float escTemp = escTelemetry.cap_temp;
   float motorTemp = escTelemetry.motor_temp;
   // Check if BMS or ESC is connected
@@ -660,7 +674,7 @@ void updateLvglMainScreen(
     lv_obj_remove_style(batt_temp_bg, &style_warning, 0);
     lv_obj_remove_style(batt_temp_bg, &style_critical, 0);
 
-    if (bmsTelemetry.bmsState == TelemetryState::CONNECTED) {
+    if (bmsTelemetry.bmsState == TelemetryState::CONNECTED && hasValidBatteryTemp) {
       lv_label_set_text_fmt(batt_temp_label, "%d", static_cast<int>(batteryTemp));
       if (batteryTemp >= bmsCellTempThresholds.critHigh) {
         lv_obj_add_style(batt_temp_bg, &style_critical, 0);
@@ -672,6 +686,7 @@ void updateLvglMainScreen(
         lv_obj_add_flag(batt_temp_bg, LV_OBJ_FLAG_HIDDEN);
       }
     } else {
+      // No valid cell probe connected: show "-" instead of a fake low reading.
       lv_label_set_text(batt_temp_label, "-");
       lv_obj_add_flag(batt_temp_bg, LV_OBJ_FLAG_HIDDEN);
     }

test/native_stubs/BMS_CAN.h

@@ -0,0 +1,9 @@
+#pragma once
+
+#include <cmath>
+#include <cstdint>
+
+class BMS_CAN {
+public:
+  static constexpr float TEMP_PROBE_DISCONNECTED = -40.0f;
+};

test/native_stubs/SPI.h

@@ -0,0 +1,3 @@
+#pragma once
+
+class SPIClass {};