Skip to content

Commit 950806c

Browse files
cbrown350claude
cbrown350
and
claude
committed
fix: MQTT publish rate limiting, NaN state handling, threshold validation
- Add setState() with change detection to avoid publishing multiple times per second; only publishes on meaningful state change or every 30s - Fix NaN temperatures producing invalid JSON (use null instead) - Add per-sensor availability templates for proper unavailable state in HA - Enforce temp_threshold_on <= temp_threshold_off in MQTT command handler - Fix configuration_url using MAC instead of hostname - Increase MQTT buffer to 2048 for large discovery payloads - Number entities use box/slider mode as appropriate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6f79c25 commit 950806c

3 files changed

Lines changed: 97 additions & 21 deletions

File tree

‎lib/MQTTManager/MQTTManager.cpp‎

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "MQTTManager.h"
22
#include "Logger.h"
3+
#include <math.h>
34

45
// ============================================================================
56
// Public API
@@ -10,7 +11,7 @@ void MQTTManager::begin(const MQTTConfig& config) {
1011
#ifdef ESP32
1112
mqttClient_.setClient(wifiClient_);
1213
mqttClient_.setServer(config_.server.c_str(), config_.port);
13-
mqttClient_.setBufferSize(1024);
14+
mqttClient_.setBufferSize(2048);
1415
mqttClient_.setCallback([this](char* topic, byte* payload, unsigned int length) {
1516
String topicStr(topic);
1617
String payloadStr;
@@ -52,7 +53,7 @@ void MQTTManager::update() {
5253

5354
mqttClient_.loop();
5455

55-
// Periodic state republish
56+
// Publish state on meaningful change or periodic interval
5657
unsigned long now = millis();
5758
if (state_changed_ || (now - last_state_publish_ >= STATE_PUBLISH_INTERVAL)) {
5859
publishState(last_state_);
@@ -61,17 +62,54 @@ void MQTTManager::update() {
6162
#endif
6263
}
6364

65+
void MQTTManager::setState(const MQTTStateData& state) {
66+
if (!has_published_ || hasMeaningfulChange(last_state_, state)) {
67+
state_changed_ = true;
68+
}
69+
last_state_ = state;
70+
}
71+
72+
bool MQTTManager::hasMeaningfulChange(const MQTTStateData& a, const MQTTStateData& b) const {
73+
// Compare all fields except heap, uptime, and rssi (these change constantly)
74+
if (a.sensor1_connected != b.sensor1_connected) return true;
75+
if (a.sensor2_connected != b.sensor2_connected) return true;
76+
if (a.pump_running != b.pump_running) return true;
77+
if (a.pump_auto_mode != b.pump_auto_mode) return true;
78+
if (a.water_flow_error != b.water_flow_error) return true;
79+
if (a.door_open != b.door_open) return true;
80+
if (a.door_closed != b.door_closed) return true;
81+
if (a.door_auto_mode != b.door_auto_mode) return true;
82+
if (a.light_on != b.light_on) return true;
83+
if (a.light_brightness != b.light_brightness) return true;
84+
if (a.light_auto_mode != b.light_auto_mode) return true;
85+
// Use threshold for float comparisons (0.1 degree)
86+
if (fabsf(a.sensor1_temp_f - b.sensor1_temp_f) > 0.1f) return true;
87+
if (fabsf(a.sensor2_temp_f - b.sensor2_temp_f) > 0.1f) return true;
88+
if (fabsf(a.water_flow_rate - b.water_flow_rate) > 0.01f) return true;
89+
if (fabsf(a.water_total_gallons - b.water_total_gallons) > 0.01f) return true;
90+
if (fabsf(a.temp_threshold_on - b.temp_threshold_on) > 0.01f) return true;
91+
if (fabsf(a.temp_threshold_off - b.temp_threshold_off) > 0.01f) return true;
92+
// Handle NaN transitions
93+
if (isnan(a.sensor1_temp_f) != isnan(b.sensor1_temp_f)) return true;
94+
if (isnan(a.sensor2_temp_f) != isnan(b.sensor2_temp_f)) return true;
95+
return false;
96+
}
97+
6498
void MQTTManager::publishState(const MQTTStateData& state) {
6599
last_state_ = state;
100+
has_published_ = true;
66101

67102
#ifdef ESP32
68103
if (!mqttClient_.connected()) return;
69104

70105
// Publish main state JSON
71106
{
72107
JsonDocument doc;
73-
doc["sensor1_temp_f"] = serialized(String(state.sensor1_temp_f, 1));
74-
doc["sensor2_temp_f"] = serialized(String(state.sensor2_temp_f, 1));
108+
// Use null for NaN values to keep JSON valid
109+
if (isnan(state.sensor1_temp_f)) doc["sensor1_temp_f"] = nullptr;
110+
else doc["sensor1_temp_f"] = serialized(String(state.sensor1_temp_f, 1));
111+
if (isnan(state.sensor2_temp_f)) doc["sensor2_temp_f"] = nullptr;
112+
else doc["sensor2_temp_f"] = serialized(String(state.sensor2_temp_f, 1));
75113
doc["sensor1_connected"] = state.sensor1_connected;
76114
doc["sensor2_connected"] = state.sensor2_connected;
77115
doc["water_flow_rate"] = serialized(String(state.water_flow_rate, 2));
@@ -240,7 +278,7 @@ void MQTTManager::addDeviceInfo(JsonObject& doc) const {
240278
device["manufacturer"] = "DIY";
241279
device["model"] = "Coop Controller";
242280
device["sw_version"] = config_.fw_version;
243-
device["configuration_url"] = "http://" + config_.device_id + ".local";
281+
device["configuration_url"] = "http://" + config_.hostname + ".local";
244282
}
245283

246284
void MQTTManager::addOriginInfo(JsonObject& doc) const {
@@ -284,12 +322,14 @@ void MQTTManager::publishDiscovery() {
284322

285323
// --- Sensors ---
286324
publishSensorDiscovery("sensor_1_temperature_f", "Sensor 1 Temperature",
287-
"{{ value_json.sensor1_temp_f }}",
288-
"temperature", "\u00b0F", "measurement");
325+
"{{ value_json.sensor1_temp_f | default('') }}",
326+
"temperature", "\u00b0F", "measurement", "",
327+
"", "{{ value_json.sensor1_temp_f is not none }}");
289328

290329
publishSensorDiscovery("sensor_2_temperature_f", "Sensor 2 Temperature",
291-
"{{ value_json.sensor2_temp_f }}",
292-
"temperature", "\u00b0F", "measurement");
330+
"{{ value_json.sensor2_temp_f | default('') }}",
331+
"temperature", "\u00b0F", "measurement", "",
332+
"", "{{ value_json.sensor2_temp_f is not none }}");
293333

294334
publishSensorDiscovery("water_flow_rate", "Water Flow Rate",
295335
"{{ value_json.water_flow_rate }}",
@@ -365,15 +405,15 @@ void MQTTManager::publishDiscovery() {
365405
// --- Numbers ---
366406
publishNumberDiscovery("temp_threshold_on", "Temp Threshold ON",
367407
"{{ value_json.temp_threshold_on }}",
368-
0, 120, 1, "\u00b0F", "mdi:thermometer-high");
408+
-10, 80, 0.5f, "\u00b0F", "mdi:thermometer-high", "box");
369409

370410
publishNumberDiscovery("temp_threshold_off", "Temp Threshold OFF",
371411
"{{ value_json.temp_threshold_off }}",
372-
0, 120, 1, "\u00b0F", "mdi:thermometer-low");
412+
-10, 80, 0.5f, "\u00b0F", "mdi:thermometer-low", "box");
373413

374414
publishNumberDiscovery("light_brightness", "Light Brightness",
375415
"{{ value_json.light_brightness }}",
376-
0, 100, 1, "%", "mdi:brightness-percent");
416+
0, 100, 1, "%", "mdi:brightness-percent", "slider");
377417

378418
logger.logInfo("MQTT discovery configs published");
379419
}
@@ -388,15 +428,15 @@ void MQTTManager::publishSensorDiscovery(const String& objectId, const String& n
388428
const String& unit,
389429
const String& stateClass,
390430
const String& entityCategory,
391-
const String& icon) {
431+
const String& icon,
432+
const String& availabilityTemplate) {
392433
JsonDocument doc;
393434
JsonObject root = doc.to<JsonObject>();
394435

395436
root["name"] = name;
396437
root["unique_id"] = config_.device_id + "_" + objectId;
397438
root["object_id"] = config_.device_id + "_" + objectId;
398439
root["state_topic"] = getBaseTopic() + "/state";
399-
root["availability_topic"] = getBaseTopic() + "/availability";
400440
root["value_template"] = valueTemplate;
401441

402442
if (deviceClass.length() > 0) root["device_class"] = deviceClass;
@@ -405,6 +445,21 @@ void MQTTManager::publishSensorDiscovery(const String& objectId, const String& n
405445
if (entityCategory.length() > 0) root["entity_category"] = entityCategory;
406446
if (icon.length() > 0) root["icon"] = icon;
407447

448+
// Use availability list to support both LWT and per-value availability
449+
if (availabilityTemplate.length() > 0) {
450+
JsonArray avail = root["availability"].to<JsonArray>();
451+
JsonObject lwt = avail.add<JsonObject>();
452+
lwt["topic"] = getBaseTopic() + "/availability";
453+
JsonObject val = avail.add<JsonObject>();
454+
val["topic"] = getBaseTopic() + "/state";
455+
val["value_template"] = availabilityTemplate;
456+
val["payload_available"] = "True";
457+
val["payload_not_available"] = "False";
458+
root["availability_mode"] = "all";
459+
} else {
460+
root["availability_topic"] = getBaseTopic() + "/availability";
461+
}
462+
408463
addDeviceInfo(root);
409464
addOriginInfo(root);
410465

@@ -512,7 +567,8 @@ void MQTTManager::publishNumberDiscovery(const String& objectId, const String& n
512567
const String& valueTemplate,
513568
float min, float max, float step,
514569
const String& unit,
515-
const String& icon) {
570+
const String& icon,
571+
const String& mode) {
516572
JsonDocument doc;
517573
JsonObject root = doc.to<JsonObject>();
518574

@@ -529,6 +585,7 @@ void MQTTManager::publishNumberDiscovery(const String& objectId, const String& n
529585

530586
if (unit.length() > 0) root["unit_of_measurement"] = unit;
531587
if (icon.length() > 0) root["icon"] = icon;
588+
if (mode.length() > 0) root["mode"] = mode;
532589

533590
addDeviceInfo(root);
534591
addOriginInfo(root);

‎lib/MQTTManager/MQTTManager.h‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct MQTTConfig {
2222
String password;
2323
String device_id; // Unique device identifier (from MAC or hostname)
2424
String device_name; // Human-readable device name
25+
String hostname; // Device hostname for configuration_url
2526
String fw_version; // Firmware version for device info
2627
};
2728

@@ -70,7 +71,10 @@ class MQTTManager {
7071
// Call in main loop - handles connection, reconnection, and message processing
7172
void update();
7273

73-
// Publish current state (call when state changes or periodically)
74+
// Update state data - detects meaningful changes and publishes when appropriate
75+
void setState(const MQTTStateData& state);
76+
77+
// Force-publish current state immediately (used internally and after connect)
7478
void publishState(const MQTTStateData& state);
7579

7680
// Register callback for when commands are received
@@ -110,6 +114,10 @@ class MQTTManager {
110114
static constexpr unsigned long STATE_PUBLISH_INTERVAL = 30000; // Publish state every 30s
111115
MQTTStateData last_state_;
112116
bool state_changed_ = false;
117+
bool has_published_ = false; // True after first publish
118+
119+
// Check if meaningful state fields changed (excludes heap/uptime/rssi)
120+
bool hasMeaningfulChange(const MQTTStateData& a, const MQTTStateData& b) const;
113121

114122
// Statistics
115123
unsigned int messages_published_ = 0;
@@ -136,7 +144,8 @@ class MQTTManager {
136144
const String& unit = "",
137145
const String& stateClass = "",
138146
const String& entityCategory = "",
139-
const String& icon = "");
147+
const String& icon = "",
148+
const String& availabilityTemplate = "");
140149
void publishBinarySensorDiscovery(const String& objectId, const String& name,
141150
const String& valueTemplate,
142151
const String& deviceClass = "",
@@ -151,7 +160,8 @@ class MQTTManager {
151160
const String& valueTemplate,
152161
float min, float max, float step,
153162
const String& unit = "",
154-
const String& icon = "");
163+
const String& icon = "",
164+
const String& mode = "auto");
155165

156166
// Build common device JSON
157167
void addDeviceInfo(JsonObject& doc) const;

‎src/main.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ void setup() // NOSONAR - complexity ok
441441
mac.toLowerCase();
442442
mqttConfig.device_id = mac;
443443
mqttConfig.device_name = settingsManager.getHostname();
444+
mqttConfig.hostname = settingsManager.getHostname();
444445
mqttConfig.fw_version = firmwareVersion;
445446
mqttManager.begin(mqttConfig);
446447
mqttManager.setEnabled(settingsManager.getMqttEnabled());
@@ -487,10 +488,18 @@ void setup() // NOSONAR - complexity ok
487488
// Number commands (numeric values)
488489
else if (entityId == "temp_threshold_on") {
489490
float val = payload.toFloat();
491+
// Ensure on threshold <= off threshold
492+
if (val > settingsManager.getTempThresholdOffF()) {
493+
settingsManager.setTempThresholdOffF(val);
494+
}
490495
settingsManager.setTempThresholdOnF(val);
491496
settingsManager.save();
492497
} else if (entityId == "temp_threshold_off") {
493498
float val = payload.toFloat();
499+
// Ensure off threshold >= on threshold
500+
if (val < settingsManager.getTempThresholdOnF()) {
501+
settingsManager.setTempThresholdOnF(val);
502+
}
494503
settingsManager.setTempThresholdOffF(val);
495504
settingsManager.save();
496505
} else if (entityId == "light_brightness") {
@@ -811,8 +820,8 @@ void setup() // NOSONAR - complexity ok
811820
// Update MQTT manager (connection, message processing, state publishing)
812821
mqttManager.update();
813822

814-
// Publish MQTT state periodically (every sensor update cycle)
815-
if (mqttManager.isConnected() && currentTime - lastSensorUpdate < SENSOR_UPDATE_INTERVAL + 100) {
823+
// Update MQTT state data (MQTTManager handles change detection and publish timing)
824+
if (mqttManager.isEnabled()) {
816825
MQTTStateData mqttState;
817826
mqttState.sensor1_temp_f = sensorManager.getTemperature1F();
818827
mqttState.sensor2_temp_f = sensorManager.getTemperature2F();
@@ -835,7 +844,7 @@ void setup() // NOSONAR - complexity ok
835844
mqttState.wifi_rssi = hal.wifiGetRSSI();
836845
mqttState.free_heap = hal.getFreeHeap();
837846
mqttState.uptime_seconds = currentTime / 1000;
838-
mqttManager.publishState(mqttState);
847+
mqttManager.setState(mqttState);
839848
}
840849

841850
delay(10);

0 commit comments

Comments
 (0)