Skip to content

Return the configured payload format from MQTT handlers - #1267

Open
Allram wants to merge 1 commit into
UtilitechAS:mainfrom
Allram:pr/mqtt-handler-format-mismatch
Open

Allram wants to merge 1 commit into
UtilitechAS:mainfrom
Allram:pr/mqtt-handler-format-mismatch

Conversation

@Allram

@Allram Allram commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Problem

MQTT_connect() compares mqttHandler->getFormat() against mqttConfig.payloadFormat and replaces the handler when they differ, but two handlers never report the configured value:

Handler Returns Serves formats
RawMqttHandler::getFormat() full ? 3 : 2 1 (minimal), 2 (full)
JsonMqttHandler::getFormat() 0 0, 5, 6

So 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, which AmsMqttHandler::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. 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 on env:esp8266.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant