Skip to content

Commit

Permalink
was: enable WebSocket destroy on exit in was_init
Browse files Browse the repository at this point in the history
The WAS WebSocket client sometimes fails to reconnect after receiving a
WEBSOCKET_EVENT_CLOSED event due to it not being able to create the
WebSocket client task:

  I (13:15:28.958) WILLOW/WAS: WebSocket closed
  I (13:15:28.959) WILLOW/WAS: initializing WebSocket client (ws://was.dev.willow:8502/ws)
  E (13:15:28.961) websocket_client: Error create websocket task
  E (13:15:28.967) WILLOW/WAS: failed to start WebSocket client: ESP_FAIL

When this happens, Willow stops trying to reconnect to WAS, and the user
has to reset or power-cycle their device. As this is terrible UX, this
should be avoided.

The root cause is not freeing resources of the WebSocket client before
we reinitialize it in init_was. This eventually causes us to run out of
IRAM, causing the WebSocket client task creation to fail.

To fix this, enable WebSocket destroy on exit in was_init, rather than
in was_deinit, so it is always enabled. This is needed to avoid leaking
resources due to not calling esp_websocket_client_destroy before calling
esp_websocket_client_init. We don't want to call
esp_websocket_client_destroy, as that has proven to be very prone to
crashes in the past.

Similarly, we don't want to call esp_websocket_client_start when the WAS
WebSocket client receives a WEBSOCKET_EVENT_CLOSED event. This would
avoid the resource leak, but testing has shown this is also very prone
to crashes.

See the following commits for extra information:
985375c ("was: drop esp_websocket_client_destroy()")
1f3d298 ("was: drop esp_websocket_client_destroy() completely")
  • Loading branch information
stintel authored and kristiankielhofner committed Jan 17, 2024
1 parent e30ab82 commit 203942d
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions main/was.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,8 @@ static void IRAM_ATTR cb_ws_event(const void *arg_evh, const esp_event_base_t *b

void was_deinit_task(void *data)
{
esp_err_t ret = ESP_OK;
ESP_LOGI(TAG, "stopping WebSocket client");
esp_err_t ret = esp_websocket_client_destroy_on_exit(hdl_wc);
if (ret != ESP_OK) {
ESP_LOGW(TAG, "failed to enable destroy on exit");
}

ret = esp_websocket_client_close(hdl_wc, 5000 / portTICK_PERIOD_MS);
if (ret != ESP_OK) {
Expand Down Expand Up @@ -407,6 +404,12 @@ esp_err_t init_was(void)
ESP_LOGI(TAG, "initializing WebSocket client (%s)", was_url);

hdl_wc = esp_websocket_client_init(&cfg_wc);

err = esp_websocket_client_destroy_on_exit(hdl_wc);
if (err != ESP_OK) {
ESP_LOGW(TAG, "failed to enable destroy on exit: %s", esp_err_to_name(err));
}

esp_websocket_register_events(hdl_wc, WEBSOCKET_EVENT_ANY, (esp_event_handler_t)cb_ws_event, NULL);
err = esp_websocket_client_start(hdl_wc);

Expand Down

0 comments on commit 203942d

Please sign in to comment.