Surface live settings action updates#98
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| handles->mqtt = esp_mqtt_client_init(&mqtt_config); | ||
| if (handles->mqtt) | ||
| { | ||
| esp_mqtt_client_start(handles->mqtt); | ||
| esp_err_t mqtt_err = esp_mqtt_client_start(handles->mqtt); | ||
| if (mqtt_err != ESP_OK) | ||
| { | ||
| ESP_LOGE(TAG, "MQTT start failed: 0x%x", (unsigned int)mqtt_err); | ||
| emit_diag_event(callback, user_data, DIAG_EVENT_ERROR, mqtt_err); | ||
| return mqtt_err; |
There was a problem hiding this comment.
[P1] Clean up HTTP server when diagnostics MQTT start fails
When esp_mqtt_client_start fails, the new callback code returns the error immediately, but the HTTP diagnostics server started just above remains running and handles->httpd stays set. Callers see a failure and leave diag_running_ false, so the HTTP server and MQTT handle are never stopped, leaking resources and blocking subsequent starts because port 80 is still bound. The function should tear down any partially initialized resources before returning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Ensured
diag_startdestroys the MQTT client and stops the diagnostics HTTP server whenesp_mqtt_client_startfails so partially initialized resources are cleaned up before returning. components/diag/src/diag.cL104-L119
Testing
⚠️ idf.py set-target esp32p4(command unavailable in container)⚠️ idf.py build(command unavailable in container)⚠️ idf.py lint(command unavailable in container)
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68cd5a4436508324b276ccfa3e2e9a52