MQTT enhancements#427
Conversation
- Add tbs_cmd_stop/vol_limit_spk/vol_limit_hdp/led commands - Add API endpoint /api/box/cmd for remote box control - Add MQTT command entities (Stop, VolLimitSpk, VolLimitHdp, LED) per box - Wire MQTT callbacks to tbs_cmd functions - SSE events for command feedback - Fix Docker workflow: dynamic REGISTRY_IMAGE for fork compatibility - Lowercase REGISTRY_IMAGE for GHCR requirements - Dynamic git config in build_commit_web.yml
mqtt_sendEvent was calling mqttClientPublish directly on the shared mqtt_context from HTTP handler threads, while the MQTT thread could concurrently close the connection via mqttClientClose. This caused a heap-use-after-free detected by AddressSanitizer. Fix: use the thread-safe mqtt_publish buffer mechanism (protected by MUTEX_MQTT_TX_BUFFER) which queues messages for the MQTT thread to send, instead of directly accessing mqtt_context.
- Remove unused toniebox_state_cmd_t enum (never referenced) - Remove unused pending_cmd/pending_cmd_value struct fields (never read/written) - Replace osSprintf with osSnprintf in MQTT tx callbacks (buffer safety) - Increase buffer size from 8 to 12 chars for uint32_t formatting
- Add SlapEnabled (ha_switch) and SlapDirection (ha_switch) per box - Add tbs_cmd_set_slap_enabled() and tbs_cmd_set_slap_dir() functions - Add slapEnabled and slapDir commands to /api/box/cmd endpoint - All settings applied via freshness check (setTonieboxSettings)
… timer - Replace fixed 500ms retry with exponential backoff (5s-60s cap) - Remove permanent MQTT disable on broker unavailability - Add clear [MQTT] prefixed logging for connect/disconnect/retry events - Fix ha_loop static timer bug: use per-instance timer so all boxes get periodic 60s republish (was broken for multi-box setups) - Update MQTT_CONTROL.md with reconnect behavior and expanded limitations
include/handler_api.h
Outdated
|
|
||
| error_t handleApiPluginsGet(HttpConnection *connection, const char_t *uri, const char_t *queryString, client_ctx_t *client_ctx); | ||
|
|
||
| error_t handleApiBoxCommand(HttpConnection *connection, const char_t *uri, const char_t *queryString, client_ctx_t *client_ctx); |
There was a problem hiding this comment.
For me this block looks like duplication of the existing settings API. You can set settings for an overlay, which is what you are doing here
There was a problem hiding this comment.
Tanks for the review! You're right the /api/box/cmd endpoint was duplicating what the existing settings overlay API already provides. I've removed it entirely in commit fbd3fe5:
Removed handleApiBoxCommand from handler_api.c / handler_api.h
Removed the /api/box/cmd route from server.c
Removed the helper tbs_get_overlay_by_common_name (was only used by that endpoint)
The MQTT command entities still work they call the tbs_cmd_* functions directly without going through the HTTP API layer.
There was a problem hiding this comment.
This change and the other workflow yml related changes do not fit to this PR. It would be great to split it into a different PR.
There was a problem hiding this comment.
I've reverted both workflow files (.github/workflows/build_commit_web.yml and .github/workflows/publish_docker_matrix_base.yml) back to upstream in the same commit. I'll open a separate PR for those if needed.
Address PR toniebox-reverse-engineering#427 review feedback from SciLor: - Remove handleApiBoxCommand (duplicates existing settings overlay API) - Remove tbs_get_overlay_by_common_name (only used by removed endpoint) - Remove /api/box/cmd route from server.c - Revert .github/workflows changes (belong in separate PR) MQTT command entities remain functional via tbs_cmd_* functions.
Robust MQTT reconnect with exponential backoff, fix ha_loop multi-box timer
get periodic 60s republish (was broken for multi-box setups)
feat: add slap control entities via MQTT + API
fix: remove dead code, prevent buffer overflow in MQTT tx callbacks
fix: heap-use-after-free race condition in mqtt_sendEvent
mqtt_sendEvent was calling mqttClientPublish directly on the shared
mqtt_context from HTTP handler threads, while the MQTT thread could
concurrently close the connection via mqttClientClose. This caused
a heap-use-after-free detected by AddressSanitizer.
Fix: use the thread-safe mqtt_publish buffer mechanism (protected
by MUTEX_MQTT_TX_BUFFER) which queues messages for the MQTT thread
to send, instead of directly accessing mqtt_context.
feat: stream-based playback control