Conversation
MQTT_connect() compares mqttHandler->getFormat() against
mqttConfig.payloadFormat and replaces the handler when they differ, but two
handlers never report the configured value:
RawMqttHandler::getFormat() returns full ? 3 : 2, while the raw formats
are 1 (minimal) and 2 (full)
JsonMqttHandler::getFormat() always returns 0, while the JSON handler also
serves formats 5 and 6
For payload formats 1, 2, 5 and 6 the comparison therefore never matches and
the handler is deleted and reconstructed on every reconnect.
Besides the heap churn, this breaks the reboot-on-MQTT-loss feature: a fresh
handler has lastSuccessfulLoop = 0, which loop() reads as 'last success at
boot'. Once uptime exceeds rebootMinutes, the first failed connection
reboots the device immediately instead of after the configured grace period.
Reproduced on an ESP8266 with raw minimal payload and rebootMinutes = 10:
after 16 hours of uptime a single 'Failed to connect to MQTT: -9' rebooted
the device five seconds later with REBOOT_CAUSE_MQTT_DISCONNECTED.
Report the configured format from both handlers, and start the grace period
at the first failed loop when no successful loop has happened yet, so a
handler that has never connected cannot trip the timer on its first attempt.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MQTT_connect()comparesmqttHandler->getFormat()againstmqttConfig.payloadFormatand replaces the handler when they differ, but two handlers never report the configured value:RawMqttHandler::getFormat()full ? 3 : 2JsonMqttHandler::getFormat()0So for payload formats 1, 2, 5 and 6 the comparison never matches and the handler is deleted and reconstructed on every reconnect.
Besides the heap churn, this breaks the reboot-on-MQTT-loss feature. A fresh handler has
lastSuccessfulLoop = 0, whichAmsMqttHandler::loop()reads as "last success at boot". Once uptime exceedsrebootMinutes, the first failed connection reboots the device immediately instead of after the configured grace period.Reproduced on an ESP8266 with raw minimal payload and
rebootMinutes = 10: after 16 hours of uptime, a singleFailed to connect to MQTT: -9rebooted the device five seconds later withREBOOT_CAUSE_MQTT_DISCONNECTED. Two more unexplained reboots the night before matched the same pattern.Fix
Report the configured format from both handlers: raw returns 1 or 2, json returns the format stored at construction. Domoticz (3), Home Assistant (4) and Passthrough (255) already reported their configured value.
Also start the reboot grace period at the first failed loop when no successful loop has happened yet, so a handler that has never connected cannot trip the timer on its first attempt.
Note on the shape of the fix: dropping the two overrides entirely and letting the base class return a stored format is tidier, but it moves each class's key function, so the vtable and its inline virtuals get emitted in every translation unit. That cost 25 616 bytes of flash and pushed the esp8266 target over its partition. Keeping the out-of-line definitions costs 48 bytes.
Tested
ESP8266, raw minimal payload,
rebootMinutes = 10. Before: reboot within seconds of any MQTT connect failure. After: the connection is retried and the handler survives; flash 1 024 431 -> 1 024 479 bytes onenv:esp8266.