Stop client before destroying

port/freertos/golioth_sys_freertos.c

@@ -14,38 +14,45 @@
  * Time
  *------------------------------------------------*/
 
-uint64_t golioth_sys_now_ms() {
+uint64_t golioth_sys_now_ms()
+{
     return xTaskGetTickCount() * portTICK_PERIOD_MS;
 }
 
-void golioth_sys_msleep(uint32_t ms) {
+void golioth_sys_msleep(uint32_t ms)
+{
     vTaskDelay(ms / portTICK_PERIOD_MS);
 }
 
 /*--------------------------------------------------
  * Semaphores
  *------------------------------------------------*/
 
-golioth_sys_sem_t golioth_sys_sem_create(uint32_t sem_max_count, uint32_t sem_initial_count) {
+golioth_sys_sem_t golioth_sys_sem_create(uint32_t sem_max_count, uint32_t sem_initial_count)
+{
     SemaphoreHandle_t sem = xSemaphoreCreateCounting(sem_max_count, sem_initial_count);
-    return (golioth_sys_sem_t)sem;
+    return (golioth_sys_sem_t) sem;
 }
 
-bool golioth_sys_sem_take(golioth_sys_sem_t sem, int32_t ms_to_wait) {
+bool golioth_sys_sem_take(golioth_sys_sem_t sem, int32_t ms_to_wait)
+{
     TickType_t ticks_to_wait =
-            (ms_to_wait < 0 ? portMAX_DELAY : (uint32_t)ms_to_wait / portTICK_PERIOD_MS);
-    return (pdTRUE == xSemaphoreTake((SemaphoreHandle_t)sem, ticks_to_wait));
+        (ms_to_wait < 0 ? portMAX_DELAY : (uint32_t) ms_to_wait / portTICK_PERIOD_MS);
+    return (pdTRUE == xSemaphoreTake((SemaphoreHandle_t) sem, ticks_to_wait));
 }
 
-bool golioth_sys_sem_give(golioth_sys_sem_t sem) {
-    return (pdTRUE == xSemaphoreGive((SemaphoreHandle_t)sem));
+bool golioth_sys_sem_give(golioth_sys_sem_t sem)
+{
+    return (pdTRUE == xSemaphoreGive((SemaphoreHandle_t) sem));
 }
 
-void golioth_sys_sem_destroy(golioth_sys_sem_t sem) {
-    vSemaphoreDelete((SemaphoreHandle_t)sem);
+void golioth_sys_sem_destroy(golioth_sys_sem_t sem)
+{
+    vSemaphoreDelete((SemaphoreHandle_t) sem);
 }
 
-int golioth_sys_sem_get_fd(golioth_sys_sem_t sem) {
+int golioth_sys_sem_get_fd(golioth_sys_sem_t sem)
+{
     return -1;
 }
 
@@ -58,41 +65,46 @@ int golioth_sys_sem_get_fd(golioth_sys_sem_t sem) {
 // embedded inside of the TimerHandle_t as the "TimerID").
 //
 // This wrapped timer is the underlying type of opaque golioth_sys_timer_t.
-typedef struct {
+typedef struct
+{
     TimerHandle_t timer;
     golioth_sys_timer_fn_t fn;
-    void* user_arg;
+    void *user_arg;
 } wrapped_timer_t;
 
 // Same callback used for all timers. We extract the user arg from
 // the underlying freertos timer and call the callback with it.
-static void freertos_timer_callback(TimerHandle_t xTimer) {
-    wrapped_timer_t* wt = (wrapped_timer_t*)pvTimerGetTimerID(xTimer);
+static void freertos_timer_callback(TimerHandle_t xTimer)
+{
+    wrapped_timer_t *wt = (wrapped_timer_t *) pvTimerGetTimerID(xTimer);
     wt->fn(wt->timer, wt->user_arg);
 }
 
-static TickType_t ms_to_ticks(uint32_t ms) {
+static TickType_t ms_to_ticks(uint32_t ms)
+{
     // Round to the nearest multiple of the tick period
     uint32_t remainder = ms % portTICK_PERIOD_MS;
     uint32_t rounded_ms = ms;
-    if (remainder != 0) {
+    if (remainder != 0)
+    {
         rounded_ms = ms + portTICK_PERIOD_MS - remainder;
     }
     return (rounded_ms / portTICK_PERIOD_MS);
 }
 
-golioth_sys_timer_t golioth_sys_timer_create(const struct golioth_timer_config *config) {
+golioth_sys_timer_t golioth_sys_timer_create(const struct golioth_timer_config *config)
+{
     assert(config->fn);  // timer callback function is required
 
-    wrapped_timer_t* wrapped_timer = (wrapped_timer_t*)golioth_sys_malloc(sizeof(wrapped_timer_t));
+    wrapped_timer_t *wrapped_timer =
+        (wrapped_timer_t *) golioth_sys_malloc(sizeof(wrapped_timer_t));
     memset(wrapped_timer, 0, sizeof(wrapped_timer_t));
 
-    TimerHandle_t timer = xTimerCreate(
-            config->name,
-            ms_to_ticks(config->expiration_ms),
-            pdTRUE,  // periodic
-            wrapped_timer,
-            freertos_timer_callback);
+    TimerHandle_t timer = xTimerCreate(config->name,
+                                       ms_to_ticks(config->expiration_ms),
+                                       pdTRUE,  // periodic
+                                       wrapped_timer,
+                                       freertos_timer_callback);
 
     wrapped_timer->timer = timer;
     wrapped_timer->fn = config->fn;
@@ -101,25 +113,31 @@ golioth_sys_timer_t golioth_sys_timer_create(const struct golioth_timer_config *
     return wrapped_timer;
 }
 
-bool golioth_sys_timer_start(golioth_sys_timer_t timer) {
-    wrapped_timer_t* wt = (wrapped_timer_t*)timer;
-    if (!wt) {
+bool golioth_sys_timer_start(golioth_sys_timer_t timer)
+{
+    wrapped_timer_t *wt = (wrapped_timer_t *) timer;
+    if (!wt)
+    {
         return false;
     }
-    return (pdPASS == xTimerStart((TimerHandle_t)wt->timer, 0));  // non-blocking
+    return (pdPASS == xTimerStart((TimerHandle_t) wt->timer, 0));  // non-blocking
 }
 
-bool golioth_sys_timer_reset(golioth_sys_timer_t timer) {
-    wrapped_timer_t* wt = (wrapped_timer_t*)timer;
-    if (!wt) {
+bool golioth_sys_timer_reset(golioth_sys_timer_t timer)
+{
+    wrapped_timer_t *wt = (wrapped_timer_t *) timer;
+    if (!wt)
+    {
         return false;
     }
-    return (pdPASS == xTimerReset((TimerHandle_t)wt->timer, 0));  // non-blocking
+    return (pdPASS == xTimerReset((TimerHandle_t) wt->timer, 0));  // non-blocking
 }
 
-void golioth_sys_timer_destroy(golioth_sys_timer_t timer) {
-    wrapped_timer_t* wt = (wrapped_timer_t*)timer;
-    if (!wt) {
+void golioth_sys_timer_destroy(golioth_sys_timer_t timer)
+{
+    wrapped_timer_t *wt = (wrapped_timer_t *) timer;
+    if (!wt)
+    {
         return;
     }
     xTimerDelete(wt->timer, 0);  // non-blocking
@@ -130,25 +148,36 @@ void golioth_sys_timer_destroy(golioth_sys_timer_t timer) {
  * Threads
  *------------------------------------------------*/
 
-golioth_sys_thread_t golioth_sys_thread_create(const struct golioth_thread_config *config) {
+golioth_sys_thread_t golioth_sys_thread_create(const struct golioth_thread_config *config)
+{
     TaskHandle_t task_handle = NULL;
     xTaskCreate(config->fn,
                 config->name,
                 config->stack_size,
                 config->user_arg,
                 config->prio,
                 &task_handle);
-    return (golioth_sys_thread_t)task_handle;
+    return (golioth_sys_thread_t) task_handle;
 }
 
-void golioth_sys_thread_destroy(golioth_sys_thread_t thread) {
-    vTaskDelete((TaskHandle_t)thread);
+void golioth_sys_thread_destroy(golioth_sys_thread_t thread)
+{
+    vTaskDelete((TaskHandle_t) thread);
 }
 
 /*--------------------------------------------------
  * Misc
  *------------------------------------------------*/
 
-void golioth_sys_client_connected(void* client) {}
+void golioth_sys_client_connected(void *client)
+{
+    if (CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD != 0)
+    {
+        golioth_debug_set_cloud_log_enabled(true);
+    }
+}
 
-void golioth_sys_client_disconnected(void* client) {}
+void golioth_sys_client_disconnected(void *client)
+{
+    golioth_debug_set_cloud_log_enabled(false);
+}

src/coap_client_libcoap.c

@@ -1241,6 +1241,7 @@ static void golioth_coap_client_thread(void *arg)
     cleanup:
         GLTH_LOGI(TAG, "Ending session");
 
+        golioth_sys_client_disconnected(client);
         if (client->event_callback && client->session_connected)
         {
             client->event_callback(client,
@@ -1374,6 +1375,11 @@ static void purge_request_mbox(golioth_mbox_t request_mbox)
             // free dynamically allocated user payload copy
             golioth_sys_free(request_msg.post.payload);
         }
+        else if (request_msg.type == GOLIOTH_COAP_REQUEST_POST_BLOCK)
+        {
+            // free dynamically allocated user payload copy
+            golioth_sys_free(request_msg.post_block.payload);
+        }
     }
 }
 
@@ -1383,6 +1389,10 @@ void golioth_client_destroy(struct golioth_client *client)
     {
         return;
     }
+    if (client->is_running)
+    {
+        golioth_client_stop(client);
+    }
     if (client->keepalive_timer)
     {
         golioth_sys_timer_destroy(client->keepalive_timer);

src/coap_client_zephyr.c

@@ -1564,6 +1564,11 @@ static void purge_request_mbox(golioth_mbox_t request_mbox)
             // free dynamically allocated user payload copy
             golioth_sys_free(request_msg.post.payload);
         }
+        else if (request_msg.type == GOLIOTH_COAP_REQUEST_POST_BLOCK)
+        {
+            // free dynamically allocated user payload copy
+            golioth_sys_free(request_msg.post_block.payload);
+        }
     }
 }
 
@@ -1573,6 +1578,10 @@ void golioth_client_destroy(struct golioth_client *client)
     {
         return;
     }
+    if (client->is_running)
+    {
+        golioth_client_stop(client);
+    }
     if (client->keepalive_timer)
     {
         golioth_sys_timer_destroy(client->keepalive_timer);

tests/hil/tests/connection/test.c

@@ -3,22 +3,57 @@
 
 LOG_TAG_DEFINE(test_connect);
 
+static golioth_sys_sem_t _connected_sem = NULL;
+
+static void on_client_event(struct golioth_client *client,
+                            enum golioth_client_event event,
+                            void *arg)
+{
+    bool is_connected = (event == GOLIOTH_CLIENT_EVENT_CONNECTED);
+    if (is_connected)
+    {
+        golioth_sys_sem_give(_connected_sem);
+    }
+    GLTH_LOGI(TAG, "Golioth client %s", is_connected ? "connected" : "disconnected");
+}
+
 void hil_test_entry(const struct golioth_client_config *config)
 {
+    _connected_sem = golioth_sys_sem_create(1, 0);
+
     struct golioth_client *client = golioth_client_create(config);
 
-    (void) client;
+    golioth_client_register_event_callback(client, on_client_event, NULL);
+    golioth_sys_sem_take(_connected_sem, GOLIOTH_SYS_WAIT_FOREVER);
 
-    while (1)
-    {
-        golioth_sys_msleep(10 * 1000);
+    /* Pause to ensure we don't have out-of-order logs */
+    golioth_sys_msleep(1 * 1000);
 
-        GLTH_LOGI(TAG, "Stopping client");
-        golioth_client_stop(client);
+    GLTH_LOGI(TAG, "Stopping client");
+    golioth_client_stop(client);
 
-        golioth_sys_msleep(10 * 1000);
+    golioth_sys_msleep(10 * 1000);
 
-        GLTH_LOGI(TAG, "Starting client");
-        golioth_client_start(client);
-    }
+    GLTH_LOGI(TAG, "Starting client");
+    golioth_client_start(client);
+
+    golioth_sys_sem_take(_connected_sem, GOLIOTH_SYS_WAIT_FOREVER);
+
+    /* Pause to ensure we don't have out-of-order logs */
+    golioth_sys_msleep(1 * 1000);
+
+    GLTH_LOGI(TAG, "Destroying client");
+    golioth_client_destroy(client);
+    client = NULL;
+
+    golioth_sys_msleep(10 * 1000);
+
+    GLTH_LOGI(TAG, "Starting client");
+    client = golioth_client_create(config);
+    golioth_client_start(client);
+
+    golioth_sys_sem_take(_connected_sem, GOLIOTH_SYS_WAIT_FOREVER);
+
+    /* Pause to ensure logs are show */
+    golioth_sys_msleep(1 * 1000);
 }

tests/hil/tests/connection/test_connection.py

@@ -10,7 +10,12 @@ async def test_connect(board, device):
     # Confirm connection to Golioth
     assert None != board.wait_for_regex_in_line('Golioth CoAP client connected', timeout_s=120)
 
-    # Wait for reconnection
-    assert None != board.wait_for_regex_in_line('Stopping client', timeout_s=10)
+    # Wait for reconnection after golioth_client_stop();
+    assert None != board.wait_for_regex_in_line('Stopping client', timeout_s=15)
+    assert None != board.wait_for_regex_in_line('Starting client', timeout_s=120)
+    assert None != board.wait_for_regex_in_line('Golioth CoAP client connected', timeout_s=120)
+
+    # Wait for reconnection after golioth_client_destroy();
+    assert None != board.wait_for_regex_in_line('Destroying client', timeout_s=15)
     assert None != board.wait_for_regex_in_line('Starting client', timeout_s=120)
     assert None != board.wait_for_regex_in_line('Golioth CoAP client connected', timeout_s=120)