Fix T-Beam 1W fan races and remove PWM controls

variants/lilygo_tbeam_1w/TBeam1WBoard.cpp

@@ -5,8 +5,6 @@
 #include <math.h>
 #include <stdlib.h>
 
-static const int FAN_PWM_MAX = (1 << FAN_PWM_RES_BITS) - 1;
-
 void TBeam1WBoard::begin() {
   ESP32Board::begin();
 
@@ -30,8 +28,8 @@ void TBeam1WBoard::begin() {
   // Fan: auto/onoff. Thermal on at 36C / off below 30C; TX still forces a cooldown.
   pinMode(FAN_CTRL_PIN, OUTPUT);
   digitalWrite(FAN_CTRL_PIN, HIGH);
+  _fan_on = true;
   _temp_c = readNtcTempC();
-  applyDuty(100);
   startFanTask();
 }
 
@@ -43,26 +41,30 @@ void TBeam1WBoard::startFanTask() {
 void TBeam1WBoard::fanTaskThunk(void* arg) {
   auto* self = static_cast<TBeam1WBoard*>(arg);
   for (;;) {
-    if (!self->_stopped) {
+    self->updateFan();
-      self->updateFan();
-    }
     vTaskDelay(pdMS_TO_TICKS(1000));
   }
 }
 
 void TBeam1WBoard::onBeforeTransmit() {
   digitalWrite(LED_PIN, HIGH);  // TX LED on
+  portENTER_CRITICAL(&_fan_mux);
   _tx_active = true;
-  if (_mode == FAN_AUTO && _manual_duty < 0 && !_stopped) {
+  if (_mode == FAN_AUTO && !_stopped) {
-    applyDuty(FAN_TX_FLOOR_PCT);
+    setFanOutputLocked(true);
   }
+  portEXIT_CRITICAL(&_fan_mux);
 }
 
 void TBeam1WBoard::onAfterTransmit() {
   digitalWrite(LED_PIN, LOW);   // TX LED off
-  _tx_until_ms = millis() + FAN_TX_COOLDOWN_MS;
+  portENTER_CRITICAL(&_fan_mux);
-  _tx_cooldown_active = true;
+  if (!_stopped) {
+    _tx_until_ms = millis() + FAN_TX_COOLDOWN_MS;
+    _tx_cooldown_active = true;
+  }
   _tx_active = false;
+  portEXIT_CRITICAL(&_fan_mux);
 }
 
 uint16_t TBeam1WBoard::getBattMilliVolts() {
@@ -81,8 +83,12 @@ const char* TBeam1WBoard::getManufacturerName() const {
 }
 
 void TBeam1WBoard::powerOff() {
+  portENTER_CRITICAL(&_fan_mux);
   _stopped = true;
-  applyDuty(0);
+  _tx_active = false;
+  _tx_cooldown_active = false;
+  setFanOutputLocked(false);
+  portEXIT_CRITICAL(&_fan_mux);
 
   // Turn off radio LNA (CTRL pin must be LOW when not receiving)
   digitalWrite(SX126X_RXEN, LOW);
@@ -97,11 +103,16 @@ void TBeam1WBoard::powerOff() {
 }
 
 void TBeam1WBoard::setFanEnabled(bool enabled) {
-  applyDuty(enabled ? 100 : 0);
+  portENTER_CRITICAL(&_fan_mux);
+  setFanOutputLocked(enabled && !_stopped);
+  portEXIT_CRITICAL(&_fan_mux);
 }
 
 bool TBeam1WBoard::isFanEnabled() const {
-  return _duty_pct > 0;
+  portENTER_CRITICAL(&_fan_mux);
+  bool enabled = _fan_on;
+  portEXIT_CRITICAL(&_fan_mux);
+  return enabled;
 }
 
 float TBeam1WBoard::readNtcTempC() {
@@ -129,16 +140,7 @@ bool TBeam1WBoard::ntcImplausible(float temp_c) const {
   return isnan(temp_c) || temp_c < -20.0f || temp_c > 120.0f;
 }
 
-int TBeam1WBoard::rampDuty(float temp_c) const {
+bool TBeam1WBoard::isTxCoolingLocked(uint32_t now) {
-  if (temp_c < (float)_lo_c) return 0;
-  if (temp_c >= (float)_hi_c) return 100;
-  float span = (float)(_hi_c - _lo_c);
-  if (span <= 0.0f) return 100;
-  float t = (temp_c - (float)_lo_c) / span;
-  return FAN_MIN_DUTY_PCT + (int)((100 - FAN_MIN_DUTY_PCT) * t + 0.5f);
-}
-
-bool TBeam1WBoard::isTxCooling(uint32_t now) {
   if (_tx_active) return true;
   if (!_tx_cooldown_active) return false;
   if ((int32_t)(now - _tx_until_ms) >= 0) {
@@ -148,7 +150,7 @@ bool TBeam1WBoard::isTxCooling(uint32_t now) {
   return true;
 }
 
-int TBeam1WBoard::cooldownSecs() {
+int TBeam1WBoard::cooldownSecsLocked() {
   if (_tx_active) return (FAN_TX_COOLDOWN_MS + 999) / 1000;
   if (!_tx_cooldown_active) return 0;
   int32_t remain_ms = (int32_t)(_tx_until_ms - millis());
@@ -159,59 +161,41 @@ int TBeam1WBoard::cooldownSecs() {
   return (remain_ms + 999) / 1000;
 }
 
-void TBeam1WBoard::applyDuty(int pct) {
+void TBeam1WBoard::setFanOutputLocked(bool enabled) {
-  if (pct < 0) pct = 0;
+  _fan_on = enabled;
-  if (pct > 100) pct = 100;
+  digitalWrite(FAN_CTRL_PIN, enabled ? HIGH : LOW);
-  _duty_pct = pct;
-
-  if (_drive == FAN_DRIVE_PWM) {
-    if (!_pwm_attached) {
-      ledcSetup(FAN_PWM_CHANNEL, FAN_PWM_FREQ_HZ, FAN_PWM_RES_BITS);
-      ledcAttachPin(FAN_CTRL_PIN, FAN_PWM_CHANNEL);
-      _pwm_attached = true;
-    }
-    uint32_t ticks = ((uint32_t)pct * FAN_PWM_MAX + 50) / 100;
-    ledcWrite(FAN_PWM_CHANNEL, ticks);
-  } else {
-    if (_pwm_attached) {
-      ledcDetachPin(FAN_CTRL_PIN);
-      _pwm_attached = false;
-      pinMode(FAN_CTRL_PIN, OUTPUT);
-    }
-    digitalWrite(FAN_CTRL_PIN, pct > 0 ? HIGH : LOW);
-  }
 }
 
 void TBeam1WBoard::updateFan() {
   float t = readNtcTempC();
-  _temp_c = t;
+  uint32_t now = millis();
-  bool tx_cooling = isTxCooling(millis());
 
-  int duty;
+  portENTER_CRITICAL(&_fan_mux);
-  if (_manual_duty >= 0) {
+  if (_stopped) {
-    duty = _manual_duty;
+    portEXIT_CRITICAL(&_fan_mux);
-  } else if (_mode == FAN_ON) {
+    return;
-    duty = 100;
+  }
+
+  _temp_c = t;
+  bool tx_cooling = isTxCoolingLocked(now);
+
+  bool enabled;
+  if (_mode == FAN_ON) {
+    enabled = true;
   } else if (_mode == FAN_OFF) {
-    duty = 0;
+    enabled = false;
   } else if (ntcImplausible(t)) {
-    duty = 100;  // fail-safe: treat bad NTC as hot
+    enabled = true;  // fail-safe: treat bad NTC as hot
-  } else if (_drive == FAN_DRIVE_PWM) {
-    duty = rampDuty(t);
   } else {
-    // Thermal hysteresis only: on at hi, off below lo. TX cooldown is applied
+    // TX cooldown must not latch _thermal_on, or a TX at 30C keeps the fan
-    // after this and must not latch _thermal_on, or a TX at 30C keeps the fan
+    // running until the temperature dips under lo.
-    // running until temp dips under lo.
     if (t >= (float)_hi_c) _thermal_on = true;
     else if (t < (float)_lo_c) _thermal_on = false;
-    duty = _thermal_on ? 100 : 0;
+    enabled = _thermal_on || tx_cooling;
   }
 
-  if (_mode == FAN_AUTO && _manual_duty < 0 && !ntcImplausible(t)) {
+  setFanOutputLocked(enabled);
-    if (tx_cooling && duty < FAN_TX_FLOOR_PCT) duty = FAN_TX_FLOOR_PCT;
+  portEXIT_CRITICAL(&_fan_mux);
-  }
-
-  applyDuty(duty);
 }
 
 bool TBeam1WBoard::persistKey(const char* key, const char* value) {
@@ -233,32 +217,28 @@ bool TBeam1WBoard::parseIntArg(const char* text, int& value) {
 void TBeam1WBoard::loadFanPrefs() {
   if (!_prefs) return;
 
+  FanMode mode = FAN_AUTO;
   char buf[12];
   buf[0] = 0;
   if (_prefs->getByKey("fan", buf, 11)) {
-    if (strcmp(buf, "auto") == 0) _mode = FAN_AUTO;
+    if (strcmp(buf, "off") == 0) mode = FAN_OFF;
-    else if (strcmp(buf, "off") == 0) _mode = FAN_OFF;
+    else if (strcmp(buf, "on") == 0) mode = FAN_ON;
-    else if (strcmp(buf, "on") == 0) _mode = FAN_ON;
   }
 
-  buf[0] = 0;
+  int lo = FAN_DEFAULT_LO_C;
-  if (_prefs->getByKey("fan_drv", buf, 11)) {
+  int hi = FAN_DEFAULT_HI_C;
-    if (strcmp(buf, "onoff") == 0) _drive = FAN_DRIVE_ONOFF;
-    else if (strcmp(buf, "pwm") == 0) _drive = FAN_DRIVE_PWM;
-  }
-
-  int lo = _lo_c;
-  int hi = _hi_c;
   buf[0] = 0;
   if (_prefs->getByKey("fan_lo", buf, 11)) lo = atoi(buf);
   buf[0] = 0;
   if (_prefs->getByKey("fan_hi", buf, 11)) hi = atoi(buf);
+
+  portENTER_CRITICAL(&_fan_mux);
+  _mode = mode;
   if (lo >= 0 && hi <= 120 && lo < hi) {
     _lo_c = lo;
     _hi_c = hi;
   }
-
+  portEXIT_CRITICAL(&_fan_mux);
-  _manual_duty = -1;
 }
 
 void TBeam1WBoard::attachDynamicPrefs(KeyValueStore* prefs) {
@@ -267,40 +247,44 @@ void TBeam1WBoard::attachDynamicPrefs(KeyValueStore* prefs) {
   updateFan();
 }
 
-const char* TBeam1WBoard::modeName() const {
+const char* TBeam1WBoard::modeNameLocked() const {
-  if (_manual_duty >= 0) return "manual";
   if (_mode == FAN_AUTO) return "auto";
   if (_mode == FAN_OFF) return "off";
   return "on";
 }
 
-const char* TBeam1WBoard::driveName() const {
-  return _drive == FAN_DRIVE_ONOFF ? "onoff" : "pwm";
-}
-
 bool TBeam1WBoard::handleCommand(const char* command, uint32_t sender_timestamp, char* reply) {
   (void)sender_timestamp;
 
   if (strcmp(command, "get fan") == 0) {
-    int cd = cooldownSecs();
+    portENTER_CRITICAL(&_fan_mux);
-    if (ntcImplausible(_temp_c)) {
+    int cd = cooldownSecsLocked();
-      sprintf(reply, "> %s n/a duty=%d%% %s cd=%ds",
+    float temp_c = _temp_c;
-              modeName(), (int)_duty_pct, driveName(), cd);
+    bool enabled = _fan_on;
+    const char* mode = modeNameLocked();
+    portEXIT_CRITICAL(&_fan_mux);
+    if (ntcImplausible(temp_c)) {
+      sprintf(reply, "> %s n/a fan=%s cd=%ds", mode, enabled ? "on" : "off", cd);
     } else {
-      sprintf(reply, "> %s %.1fC duty=%d%% %s cd=%ds",
+      sprintf(reply, "> %s %.1fC fan=%s cd=%ds",
-              modeName(), (double)_temp_c, (int)_duty_pct, driveName(), cd);
+              mode, (double)temp_c, enabled ? "on" : "off", cd);
     }
     return true;
   }
 
   if (strncmp(command, "set fan.lo ", 11) == 0) {
     int lo;
-    if (!parseIntArg(&command[11], lo) || lo < 0 || lo >= _hi_c || lo > 100) {
+    portENTER_CRITICAL(&_fan_mux);
+    int hi_limit = _hi_c;
+    portEXIT_CRITICAL(&_fan_mux);
+    if (!parseIntArg(&command[11], lo) || lo < 0 || lo >= hi_limit || lo > 100) {
       strcpy(reply, "Error: fan.lo must be 0..100 and < fan.hi");
     } else if (!persistKey("fan_lo", &command[11])) {
       strcpy(reply, "Error: failed to save fan.lo");
     } else {
+      portENTER_CRITICAL(&_fan_mux);
       _lo_c = lo;
+      portEXIT_CRITICAL(&_fan_mux);
       sprintf(reply, "OK - fan.lo %d", lo);
     }
     return true;
@@ -308,79 +292,51 @@ bool TBeam1WBoard::handleCommand(const char* command, uint32_t sender_timestamp,
 
   if (strncmp(command, "set fan.hi ", 11) == 0) {
     int hi;
-    if (!parseIntArg(&command[11], hi) || hi <= _lo_c || hi > 120) {
+    portENTER_CRITICAL(&_fan_mux);
+    int lo_limit = _lo_c;
+    portEXIT_CRITICAL(&_fan_mux);
+    if (!parseIntArg(&command[11], hi) || hi <= lo_limit || hi > 120) {
       strcpy(reply, "Error: fan.hi must be > fan.lo and <= 120");
     } else if (!persistKey("fan_hi", &command[11])) {
       strcpy(reply, "Error: failed to save fan.hi");
     } else {
+      portENTER_CRITICAL(&_fan_mux);
       _hi_c = hi;
+      portEXIT_CRITICAL(&_fan_mux);
       sprintf(reply, "OK - fan.hi %d", hi);
     }
     return true;
   }
 
-  if (strncmp(command, "set fan.drive ", 14) == 0) {
-    const char* arg = &command[14];
-    if (strcmp(arg, "pwm") == 0) {
-      if (!persistKey("fan_drv", "pwm")) {
-        strcpy(reply, "Error: failed to save fan.drive");
-      } else {
-        _drive = FAN_DRIVE_PWM;
-        applyDuty(_duty_pct);
-        strcpy(reply, "OK - fan.drive pwm");
-      }
-    } else if (strcmp(arg, "onoff") == 0) {
-      if (!persistKey("fan_drv", "onoff")) {
-        strcpy(reply, "Error: failed to save fan.drive");
-      } else {
-        _drive = FAN_DRIVE_ONOFF;
-        applyDuty(_duty_pct);
-        strcpy(reply, "OK - fan.drive onoff");
-      }
-    } else {
-      strcpy(reply, "Error: fan.drive must be pwm or onoff");
-    }
-    return true;
-  }
-
-  if (strncmp(command, "set fan.duty ", 13) == 0) {
-    int duty;
-    if (!parseIntArg(&command[13], duty) || duty < 0 || duty > 100) {
-      strcpy(reply, "Error: fan.duty must be 0-100");
-    } else {
-      _manual_duty = duty;
-      applyDuty(duty);
-      sprintf(reply, "OK - fan.duty %d (not saved)", duty);
-    }
-    return true;
-  }
-
   if (strncmp(command, "set fan ", 8) == 0) {
     const char* arg = &command[8];
     if (strcmp(arg, "on") == 0) {
       if (!persistKey("fan", "on")) {
         strcpy(reply, "Error: failed to save fan mode");
       } else {
+        portENTER_CRITICAL(&_fan_mux);
         _mode = FAN_ON;
-        _manual_duty = -1;
+        if (!_stopped) setFanOutputLocked(true);
-        applyDuty(100);
+        portEXIT_CRITICAL(&_fan_mux);
         strcpy(reply, "OK - fan on");
       }
     } else if (strcmp(arg, "off") == 0) {
       if (!persistKey("fan", "off")) {
         strcpy(reply, "Error: failed to save fan mode");
       } else {
+        portENTER_CRITICAL(&_fan_mux);
         _mode = FAN_OFF;
-        _manual_duty = -1;
+        setFanOutputLocked(false);
-        applyDuty(0);
+        portEXIT_CRITICAL(&_fan_mux);
         strcpy(reply, "OK - fan off");
       }
     } else if (strcmp(arg, "auto") == 0) {
       if (!persistKey("fan", "auto")) {
         strcpy(reply, "Error: failed to save fan mode");
       } else {
+        portENTER_CRITICAL(&_fan_mux);
         _mode = FAN_AUTO;
-        _manual_duty = -1;
+        portEXIT_CRITICAL(&_fan_mux);
         updateFan();
         strcpy(reply, "OK - fan auto");
       }

variants/lilygo_tbeam_1w/TBeam1WBoard.h

@@ -32,39 +32,34 @@
 class TBeam1WBoard : public ESP32Board {
 public:
   enum FanMode { FAN_ON, FAN_OFF, FAN_AUTO };
-  enum FanDrive { FAN_DRIVE_PWM, FAN_DRIVE_ONOFF };
 
 private:
   bool radio_powered = false;
   bool _stopped = false;
-  bool _pwm_attached = false;
   KeyValueStore* _prefs = nullptr;
   FanMode _mode = FAN_AUTO;
-  FanDrive _drive = FAN_DRIVE_ONOFF;
   int _lo_c = FAN_DEFAULT_LO_C;
   int _hi_c = FAN_DEFAULT_HI_C;
-  int _manual_duty = -1;   // -1 = follow mode; 0..100 = CLI override
   bool _thermal_on = false;  // onoff hysteresis; TX boost must not latch this
-  volatile float _temp_c = NAN;
+  bool _fan_on = true;
-  volatile int _duty_pct = 100;
+  float _temp_c = NAN;
-  volatile bool _tx_active = false;
+  bool _tx_active = false;
-  volatile bool _tx_cooldown_active = false;
+  bool _tx_cooldown_active = false;
-  volatile uint32_t _tx_until_ms = 0;
+  uint32_t _tx_until_ms = 0;
   TaskHandle_t _fan_task = nullptr;
+  mutable portMUX_TYPE _fan_mux = portMUX_INITIALIZER_UNLOCKED;
 
   void startFanTask();
   void updateFan();
-  void applyDuty(int pct);
+  void setFanOutputLocked(bool enabled);
   float readNtcTempC();
-  int rampDuty(float temp_c) const;
+  int cooldownSecsLocked();
-  int cooldownSecs();
+  bool isTxCoolingLocked(uint32_t now);
-  bool isTxCooling(uint32_t now);
   bool ntcImplausible(float temp_c) const;
   bool persistKey(const char* key, const char* value);
   static bool parseIntArg(const char* text, int& value);
   void loadFanPrefs();
-  const char* modeName() const;
+  const char* modeNameLocked() const;
-  const char* driveName() const;
   static void fanTaskThunk(void* arg);
 
 public:

variants/lilygo_tbeam_1w/variant.h

@@ -85,16 +85,11 @@
 #define NTC_R_FIXED 10000.0f
 #define NTC_VCC_MV 3300.0f
 
-// Fan control (GPIO41). Default auto + on/off: NTC is PA-adjacent PCB temp,
+// Fan control (GPIO41). NTC is PA-adjacent PCB temp, not die temp, so trip
-// not die temp, so trip well below the SX1262/ESP32 85C operating limit.
+// well below the SX1262/ESP32 85C operating limit. Hardware testing confirmed
-// This fan/MOSFET path does not respond to PWM below 100% duty.
+// that this fan/MOSFET path is on/off; PWM below 100% does not spin the fan.
 #define FAN_CTRL_PIN 41
-#define FAN_PWM_CHANNEL 4
-#define FAN_PWM_FREQ_HZ 25000
-#define FAN_PWM_RES_BITS 8
-#define FAN_MIN_DUTY_PCT 40
 #define FAN_TX_COOLDOWN_MS 15000
-#define FAN_TX_FLOOR_PCT 100
 #define FAN_DEFAULT_LO_C 30   // off below typical indoor idle (~86F)
 #define FAN_DEFAULT_HI_C 36   // on at ~97F PCB; still far below 85C chip ratings