Merge branch 'bugfix/claiming' into 'master'

claim: Fixes for assisted claiming

See merge request app-frameworks/esp-rainmaker!608

Author: shahpiyushv

components/esp_rainmaker/CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 1.8.7
+
+### Bug Fixes
+- If assisted claiming was already performed before calling `esp_rmaker_start()`,
+  the firmware was stalling with an incorrect print saying that the Node connected to network
+  without claiming.
+
 ## 1.8.6
 
 ### New Feature

components/esp_rainmaker/idf_component.yml

@@ -1,5 +1,5 @@
 ## IDF Component Manager Manifest File
-version: "1.8.6"
+version: "1.8.7"
 description: ESP RainMaker firmware agent
 url: https://github.com/espressif/esp-rainmaker/tree/master/components/esp_rainmaker
 repository: https://github.com/espressif/esp-rainmaker.git

components/esp_rainmaker/src/core/esp_rmaker_claim.c

@@ -486,14 +486,19 @@ esp_err_t esp_rmaker_self_claim_perform(esp_rmaker_claim_data_t *claim_data)
         ESP_LOGE(TAG, "Self claiming not initialised.");
         return ESP_ERR_INVALID_STATE;
     }
+    esp_rmaker_post_event(RMAKER_EVENT_CLAIM_STARTED, NULL, 0);
     esp_err_t err = esp_rmaker_claim_perform_init(claim_data);
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "Claim Init Sequence Failed.");
+        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
         return err;
     }
     err = esp_rmaker_claim_perform_verify(claim_data);
     if (err == ESP_OK) {
         ESP_LOGI(TAG, "Self Claiming was successful. Certificate received.");
+        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_SUCCESSFUL, NULL, 0);
+    } else {
+        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
     }
     esp_rmaker_claim_data_free(claim_data);
     return err;
@@ -601,6 +606,7 @@ esp_err_t esp_rmaker_assisted_claim_handle_start(RmakerClaim__RMakerClaimPayload
     response->resppayload->status = RMAKER_CLAIM__RMAKER_CLAIM_STATUS__Success;
     claim_data->state = RMAKER_CLAIM_STATE_INIT;
     ESP_LOGI(TAG, "Assisted Claiming Started.");
+    esp_rmaker_post_event(RMAKER_EVENT_CLAIM_STARTED, NULL, 0);
     return ESP_OK;
 }
 
@@ -729,11 +735,16 @@ esp_err_t esp_rmaker_assisted_claim_handle_verify(RmakerClaim__RMakerClaimPayloa
         if (handle_claim_verify_response(claim_data) == ESP_OK) {
             ESP_LOGI(TAG,"Assisted Claiming was Successful.");
             claim_data->state = RMAKER_CLAIM_STATE_VERIFY_DONE;
+            esp_rmaker_post_event(RMAKER_EVENT_CLAIM_SUCCESSFUL, NULL, 0);
             if (claim_event_group) {
                 xEventGroupSetBits(claim_event_group, CLAIM_TASK_BIT);
             }
         } else {
+            esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
             response->resppayload->status = RMAKER_CLAIM__RMAKER_CLAIM_STATUS__InvalidParam;
+            if (claim_event_group) {
+                xEventGroupSetBits(claim_event_group, CLAIM_TASK_BIT);
+            }
             return ESP_OK;
         }
     }
@@ -792,6 +803,10 @@ esp_err_t esp_rmaker_claiming_handler(uint32_t session_id, const uint8_t *inbuf,
             claim_data->state = RMAKER_CLAIM_STATE_PK_GENERATED;
             resppayload.status = RMAKER_CLAIM__RMAKER_CLAIM_STATUS__Success;
             ESP_LOGW(TAG, "Assisted Claiming Aborted.");
+            esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
+            if (claim_event_group) {
+                xEventGroupSetBits(claim_event_group, CLAIM_TASK_BIT);
+            }
             break;
         default:
             break;
@@ -921,10 +936,13 @@ static esp_rmaker_claim_data_t *esp_rmaker_claim_init(void)
         ESP_LOGE(TAG, "Claim already initialised");
         return NULL;
     }
-    claim_event_group = xEventGroupCreate();
+    /* Create event group if it doesn't exist (for assisted claiming, it may already exist) */
     if (!claim_event_group) {
-        ESP_LOGE(TAG, "Couldn't create event group");
-        return NULL;
+        claim_event_group = xEventGroupCreate();
+        if (!claim_event_group) {
+            ESP_LOGE(TAG, "Couldn't create event group");
+            return NULL;
+        }
     }
     esp_rmaker_claim_data_t *claim_data = NULL;
 
@@ -935,14 +953,23 @@ static esp_rmaker_claim_data_t *esp_rmaker_claim_init(void)
     if (xTaskCreate(&esp_rmaker_claim_task, "claim_task", ESP_RMAKER_CLAIM_TASK_STACK_SIZE,
                 &claim_data, tskIDLE_PRIORITY, NULL) != pdPASS) {
         ESP_LOGE(TAG, "Couldn't create Claim task");
+        /* On failure, delete event group and reset handle */
         vEventGroupDelete(claim_event_group);
+        claim_event_group = NULL;
         return NULL;
     }
 
     /* Wait for claim init to complete */
     xEventGroupWaitBits(claim_event_group, CLAIM_TASK_BIT, false, true, portMAX_DELAY);
+#ifdef CONFIG_ESP_RMAKER_SELF_CLAIM
+    /* For self-claiming, delete event group after PK generation */
     vEventGroupDelete(claim_event_group);
     claim_event_group = NULL;
+#else
+    /* For assisted claiming, clear the bit as it would be re-used later*/
+    xEventGroupClearBits(claim_event_group, CLAIM_TASK_BIT);
+#endif
+    /* For assisted claiming, keep event group for claiming completion signaling */
     return claim_data;
 }
 
@@ -960,12 +987,11 @@ esp_err_t esp_rmaker_assisted_claim_perform(esp_rmaker_claim_data_t *claim_data)
         ESP_LOGE(TAG, "Assisted claiming not initialised.");
         return ESP_ERR_INVALID_STATE;
     }
-    claim_event_group = xEventGroupCreate();
     if (!claim_event_group) {
-        ESP_LOGE(TAG, "Couldn't create event group");
-        return ESP_ERR_NO_MEM;
+        ESP_LOGE(TAG, "Claim event group not created.");
+        return ESP_ERR_INVALID_STATE;
     }
-    /* Wait for assisted claim to complete */
+    /* Wait for assisted claim to complete (returns immediately if bit already set) */
     ESP_LOGI(TAG, "Waiting for assisted claim to finish.");
     xEventGroupWaitBits(claim_event_group, CLAIM_TASK_BIT, false, true, portMAX_DELAY);
     esp_err_t err = ESP_FAIL;
@@ -976,16 +1002,20 @@ esp_err_t esp_rmaker_assisted_claim_perform(esp_rmaker_claim_data_t *claim_data)
     esp_event_handler_unregister(NETWORK_PROV_EVENT, NETWORK_PROV_START, &event_handler);
     esp_rmaker_claim_data_free(claim_data);
     vEventGroupDelete(claim_event_group);
+    claim_event_group = NULL;
     return err;
 }
 esp_rmaker_claim_data_t *esp_rmaker_assisted_claim_init(void)
 {
     ESP_LOGI(TAG, "Initialising Assisted Claiming. This may take time.");
+    /* esp_rmaker_claim_init() will create the event group, and for assisted claiming,
+     * it will be kept after PK generation for claiming completion signaling */
     esp_rmaker_claim_data_t *claim_data = esp_rmaker_claim_init();
     if (claim_data) {
         esp_event_handler_register(NETWORK_PROV_EVENT, NETWORK_PROV_INIT, &event_handler, claim_data);
         esp_event_handler_register(NETWORK_PROV_EVENT, NETWORK_PROV_START, &event_handler, claim_data);
     }
+    /* If claim init failed, event group is already cleaned up in esp_rmaker_claim_init() */
     return claim_data;
 }
 #endif

components/esp_rainmaker/src/core/esp_rmaker_core.c

@@ -84,6 +84,7 @@ typedef struct {
     esp_rmaker_mqtt_conn_params_t *mqtt_conn_params;
 #ifdef ESP_RMAKER_CLAIM_ENABLED
     bool need_claim;
+    bool claim_done;
     esp_rmaker_claim_data_t *claim_data;
 #endif /* ESP_RMAKER_CLAIM_ENABLED */
     /* Use Thread partition ID as the Thread network identifier. */
@@ -165,6 +166,27 @@ static void reset_event_handler(void* arg, esp_event_base_t event_base,
         }
 }
 
+#ifdef ESP_RMAKER_CLAIM_ENABLED
+static void claim_event_handler(void* arg, esp_event_base_t event_base,
+                          int32_t event_id, void* event_data)
+{
+    if (!esp_rmaker_priv_data) {
+        return;
+    }
+    if (event_id == RMAKER_EVENT_CLAIM_SUCCESSFUL) {
+        esp_rmaker_priv_data->need_claim = false;
+        esp_rmaker_priv_data->claim_done = true;
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_STARTED, &claim_event_handler);
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_SUCCESSFUL, &claim_event_handler);
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_FAILED, &claim_event_handler);
+    } else if (event_id == RMAKER_EVENT_CLAIM_FAILED) {
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_STARTED, &claim_event_handler);
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_SUCCESSFUL, &claim_event_handler);
+        esp_event_handler_unregister(RMAKER_EVENT, RMAKER_EVENT_CLAIM_FAILED, &claim_event_handler);
+    }
+}
+#endif /* ESP_RMAKER_CLAIM_ENABLED */
+
 static void esp_rmaker_mqtt_event_handler(void* arg, esp_event_base_t event_base,
                           int32_t event_id, void* event_data)
 {
@@ -459,11 +481,9 @@ static void esp_rmaker_task(void *data)
             err = ESP_FAIL;
             goto rmaker_end;
         }
-        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_STARTED, NULL, 0);
         err = esp_rmaker_assisted_claim_perform(esp_rmaker_priv_data->claim_data);
         if (err != ESP_OK) {
-            esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
-            ESP_LOGE(TAG, "esp_rmaker_self_claim_perform() returned %d. Aborting", err);
+            ESP_LOGE(TAG, "esp_rmaker_assisted_claim_perform() returned %d. Aborting", err);
 #if defined(CONFIG_ESP_RMAKER_NETWORK_OVER_WIFI)
             esp_event_handler_unregister(IP_EVENT, IP_EVENT_STA_GOT_IP, &esp_rmaker_event_handler);
 #elif defined(CONFIG_ESP_RMAKER_NETWORK_OVER_THREAD)
@@ -473,7 +493,6 @@ static void esp_rmaker_task(void *data)
             goto rmaker_end;
         }
         esp_rmaker_priv_data->claim_data = NULL;
-        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_SUCCESSFUL, NULL, 0);
     }
 #endif /* CONFIG_ESP_RMAKER_ASSISTED_CLAIM */
 #if defined(CONFIG_ESP_RMAKER_NETWORK_OVER_WIFI)
@@ -504,19 +523,17 @@ static void esp_rmaker_task(void *data)
     /* Self claiming can be done only after Wi-Fi connection */
 #ifdef CONFIG_ESP_RMAKER_SELF_CLAIM
     if (esp_rmaker_priv_data->need_claim) {
-        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_STARTED, NULL, 0);
         err = esp_rmaker_self_claim_perform(esp_rmaker_priv_data->claim_data);
         if (err != ESP_OK) {
-            esp_rmaker_post_event(RMAKER_EVENT_CLAIM_FAILED, NULL, 0);
             ESP_LOGE(TAG, "esp_rmaker_self_claim_perform() returned %d. Aborting", err);
             goto rmaker_end;
         }
         esp_rmaker_priv_data->claim_data = NULL;
-        esp_rmaker_post_event(RMAKER_EVENT_CLAIM_SUCCESSFUL, NULL, 0);
     }
 #endif
 #ifdef ESP_RMAKER_CLAIM_ENABLED
-    if (esp_rmaker_priv_data->need_claim) {
+    /* Initialize MQTT if claiming was needed (either still in progress or just completed) */
+    if (esp_rmaker_priv_data->need_claim || esp_rmaker_priv_data->claim_done) {
         esp_rmaker_priv_data->mqtt_conn_params = esp_rmaker_get_mqtt_conn_params();
         if (!esp_rmaker_priv_data->mqtt_conn_params) {
             ESP_LOGE(TAG, "Failed to initialise MQTT Config after claiming. Aborting");
@@ -529,6 +546,7 @@ static void esp_rmaker_task(void *data)
             goto rmaker_end;
         }
         esp_rmaker_priv_data->need_claim = false;
+        esp_rmaker_priv_data->claim_done = false;
     }
 #endif /* ESP_RMAKER_CLAIM_ENABLED */
 #ifdef CONFIG_ESP_RMAKER_CMD_RESP_ENABLE
@@ -643,6 +661,7 @@ static esp_err_t esp_rmaker_mqtt_conn_params_init(esp_rmaker_priv_data_t *rmaker
             return ESP_FAIL;
         } else {
             rmaker_priv_data->need_claim = true;
+            rmaker_priv_data->claim_done = false;
             return ESP_OK;
         }
     }
@@ -737,6 +756,13 @@ static esp_err_t esp_rmaker_init(const esp_rmaker_config_t *config, bool use_cla
 #endif /* CONFIG_ESP_RMAKER_ENABLE_CHALLENGE_RESPONSE */
 
     esp_rmaker_priv_data->enable_time_sync = config->enable_time_sync;
+#ifdef ESP_RMAKER_CLAIM_ENABLED
+    if (esp_rmaker_priv_data->need_claim) {
+        esp_event_handler_register(RMAKER_EVENT, RMAKER_EVENT_CLAIM_STARTED, &claim_event_handler, NULL);
+        esp_event_handler_register(RMAKER_EVENT, RMAKER_EVENT_CLAIM_SUCCESSFUL, &claim_event_handler, NULL);
+        esp_event_handler_register(RMAKER_EVENT, RMAKER_EVENT_CLAIM_FAILED, &claim_event_handler, NULL);
+    }
+#endif /* ESP_RMAKER_CLAIM_ENABLED */
     esp_rmaker_post_event(RMAKER_EVENT_INIT_DONE, NULL, 0);
     esp_rmaker_priv_data->state = ESP_RMAKER_STATE_INIT_DONE;
 

components/esp_rainmaker/src/mqtt/esp_rmaker_mqtt.c

@@ -92,7 +92,7 @@ esp_err_t esp_rmaker_mqtt_disconnect(void)
 
 esp_err_t esp_rmaker_mqtt_subscribe(const char *topic, esp_rmaker_mqtt_subscribe_cb_t cb, uint8_t qos, void *priv_data)
 {
-    ESP_LOGI(TAG, "Subscribing to topic: %s", topic);
+    ESP_LOGD(TAG, "Subscribing to topic: %s", topic);
     if (g_mqtt_config.subscribe) {
         return g_mqtt_config.subscribe(topic, cb, qos, priv_data);
     }
@@ -111,7 +111,7 @@ esp_err_t esp_rmaker_mqtt_unsubscribe(const char *topic)
 
 esp_err_t esp_rmaker_mqtt_publish(const char *topic, void *data, size_t data_len, uint8_t qos, int *msg_id)
 {
-    ESP_LOGI(TAG, "Publishing to topic: %s", topic);
+    ESP_LOGD(TAG, "Publishing to topic: %s", topic);
     if (esp_rmaker_mqtt_is_budget_available() != true) {
         ESP_LOGE(TAG, "Out of MQTT Budget. Dropping publish message.");
         return ESP_FAIL;