From cc285d63318d25e107a1bf104c0e1fd1d396a1ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simen=20S=2E=20R=C3=B8stad?= Date: Fri, 26 Sep 2025 13:37:22 +0200 Subject: [PATCH] cloud: fix fallback provisioning network state handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallback provisioning logic incorrectly determined the next state when provisioning failed. The network_connected variable was not being updated properly, causing incorrect state transitions. This fix ensures the network connection status is checked accurately before determining whether to retry provisioning or transition to an offline state. Added corresponding unit tests to verify the correct behavior of the fallback provisioning logic under different network conditions. Signed-off-by: Simen S. Røstad --- app/src/modules/cloud/cloud.c | 25 +++- tests/module/cloud/src/cloud_module_test.c | 108 ++++++++++++++++-- .../state_machines/source_of_truth/cloud.puml | 42 +++---- 3 files changed, 139 insertions(+), 36 deletions(-) diff --git a/app/src/modules/cloud/cloud.c b/app/src/modules/cloud/cloud.c index d06356ec..e3476e1d 100644 --- a/app/src/modules/cloud/cloud.c +++ b/app/src/modules/cloud/cloud.c @@ -160,8 +160,8 @@ struct cloud_state_object { /* Last received message */ uint8_t msg_buf[MAX_MSG_SIZE]; - /* Network status */ - enum network_msg_type nw_status; + /* Last network connection status */ + bool network_connected; /* Provisioning ongoing flag */ bool provisioning_ongoing; @@ -877,9 +877,13 @@ static void state_connecting_provisioning_run(void *obj) smf_set_state(SMF_CTX(state_object), &states[STATE_PROVISIONED]); return; - } else if (msg == CLOUD_PROVISIONING_FAILED) { + } else if (msg == CLOUD_PROVISIONING_FAILED && state_object->network_connected) { smf_set_state(SMF_CTX(state_object), &states[STATE_CONNECTING_BACKOFF]); + return; + } else if (msg == CLOUD_PROVISIONING_FAILED && !state_object->network_connected) { + smf_set_state(SMF_CTX(state_object), &states[STATE_DISCONNECTED]); + return; } } @@ -1483,6 +1487,19 @@ static void state_connected_paused_run(void *obj) } } +static void network_connection_status_retain(struct cloud_state_object *state_object) +{ + if (state_object->chan == &NETWORK_CHAN) { + struct network_msg msg = MSG_TO_NETWORK_MSG(state_object->msg_buf); + + if (msg.type == NETWORK_DISCONNECTED || msg.type == NETWORK_CONNECTED) { + /* Update network status to retain the last connection status */ + state_object->network_connected = + (msg.type == NETWORK_CONNECTED) ? true : false; + } + } +} + static void cloud_module_thread(void) { int err; @@ -1525,6 +1542,8 @@ static void cloud_module_thread(void) return; } + network_connection_status_retain(&cloud_state); + err = smf_run_state(SMF_CTX(&cloud_state)); if (err) { LOG_ERR("smf_run_state(), error: %d", err); diff --git a/tests/module/cloud/src/cloud_module_test.c b/tests/module/cloud/src/cloud_module_test.c index d52d1d6f..f3ed9336 100644 --- a/tests/module/cloud/src/cloud_module_test.c +++ b/tests/module/cloud/src/cloud_module_test.c @@ -211,7 +211,7 @@ static void connect_cloud(void) err = zbus_chan_pub(&NETWORK_CHAN, &nw, K_NO_WAIT); TEST_ASSERT_EQUAL(0, err); - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); } static void dummy_cb(const struct zbus_channel *chan) @@ -516,7 +516,7 @@ void test_connected_disconnected_to_connected_send_payload_disconnect(void) TEST_ASSERT_EQUAL(0, err); /* Transport module needs CPU to run state machine */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); err = k_sem_take(&cloud_connected, K_SECONDS(1)); TEST_ASSERT_EQUAL(0, err); @@ -525,7 +525,7 @@ void test_connected_disconnected_to_connected_send_payload_disconnect(void) TEST_ASSERT_EQUAL(0, err); /* Transport module needs CPU to run state machine */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); TEST_ASSERT_EQUAL(1, nrf_cloud_coap_json_message_send_fake.call_count); TEST_ASSERT_EQUAL(0, strncmp(nrf_cloud_coap_json_message_send_fake.arg0_val, @@ -539,7 +539,7 @@ void test_connected_disconnected_to_connected_send_payload_disconnect(void) TEST_ASSERT_EQUAL(0, err); /* Transport module needs CPU to run state machine */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); err = k_sem_take(&cloud_disconnected, K_SECONDS(1)); TEST_ASSERT_EQUAL(0, err); @@ -597,7 +597,7 @@ void test_gnss_location_data_handling(void) TEST_ASSERT_EQUAL(0, err); /* Give the module time to process */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); /* Verify that GNSS location data was sent to nRF Cloud */ TEST_ASSERT_EQUAL(1, nrf_cloud_coap_location_send_fake.call_count); @@ -629,7 +629,7 @@ void test_storage_data_battery_sent_to_cloud(void) TEST_ASSERT_EQUAL(0, err); /* Allow processing */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); /* One successful read + one -EAGAIN drain */ TEST_ASSERT_EQUAL(2, storage_batch_read_fake.call_count); @@ -657,7 +657,7 @@ void test_storage_data_environmental_sent_to_cloud(void) ARG_UNUSED(err); /* Allow processing */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); /* One successful read + one -EAGAIN drain */ TEST_ASSERT_EQUAL(2, storage_batch_read_fake.call_count); @@ -685,7 +685,7 @@ void test_storage_data_network_conn_eval_sent_to_cloud(void) ARG_UNUSED(err); /* Allow processing */ - k_sleep(K_MSEC(100)); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); /* One successful read + one -EAGAIN drain */ TEST_ASSERT_EQUAL(2, storage_batch_read_fake.call_count); @@ -693,6 +693,98 @@ void test_storage_data_network_conn_eval_sent_to_cloud(void) TEST_ASSERT_EQUAL(2, nrf_cloud_coap_sensor_send_fake.call_count); } +void test_provisioning_failed_with_network_connected_should_go_to_backoff(void) +{ + int err; + struct network_msg network_msg = { + .type = NETWORK_CONNECTED + }; + struct cloud_msg cloud_msg = { + .type = CLOUD_PROVISIONING_REQUEST + }; + struct nrf_provisioning_callback_data event = { + .type = NRF_PROVISIONING_EVENT_FAILED + }; + + /* Start with a connected network state */ + zbus_chan_pub(&NETWORK_CHAN, &network_msg, K_NO_WAIT); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); + + /* Should now be connected */ + err = k_sem_take(&cloud_connected, K_SECONDS(WAIT_TIMEOUT)); + TEST_ASSERT_EQUAL(0, err); + + /* Trigger provisioning request */ + zbus_chan_pub(&CLOUD_CHAN, &cloud_msg, K_NO_WAIT); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); + + /* Should now be disconnected (entering provisioning state) */ + err = k_sem_take(&cloud_disconnected, K_SECONDS(WAIT_TIMEOUT)); + TEST_ASSERT_EQUAL(0, err); + + TEST_ASSERT_EQUAL(1, nrf_provisioning_trigger_manually_fake.call_count); + + /* Simulate provisioning failure while network is still connected */ + handler(&event); + + /* Allow time for state machine to process the failure and enter backoff state */ + k_sleep(K_SECONDS(INITIAL_PROVISIONING_RETRY_SEC + 1)); + + /* Verify that provisioning is retried after backoff + * (indicating we went to backoff state) + */ + TEST_ASSERT_EQUAL(2, nrf_provisioning_trigger_manually_fake.call_count); + + /* Exit provisioning by simulating failure */ + handler(&event); +} + +void test_provisioning_failed_with_network_disconnected_should_go_to_disconnected(void) +{ + int err; + struct network_msg network_msg = { + .type = NETWORK_CONNECTED + }; + struct cloud_msg cloud_msg = { + .type = CLOUD_PROVISIONING_REQUEST + }; + struct nrf_provisioning_callback_data event = { + .type = NRF_PROVISIONING_EVENT_FAILED + }; + + /* Start with a connected network state */ + zbus_chan_pub(&NETWORK_CHAN, &network_msg, K_NO_WAIT); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); + + /* Should now be connected */ + err = k_sem_take(&cloud_connected, K_SECONDS(WAIT_TIMEOUT)); + TEST_ASSERT_EQUAL(0, err); + + /* Trigger provisioning request */ + zbus_chan_pub(&CLOUD_CHAN, &cloud_msg, K_NO_WAIT); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); + + /* Should now be disconnected (entering provisioning state) */ + err = k_sem_take(&cloud_disconnected, K_SECONDS(WAIT_TIMEOUT)); + TEST_ASSERT_EQUAL(0, err); + + TEST_ASSERT_EQUAL(1, nrf_provisioning_trigger_manually_fake.call_count); + + /* Simulate network disconnect while in provisioning state */ + network_msg.type = NETWORK_DISCONNECTED; + zbus_chan_pub(&NETWORK_CHAN, &network_msg, K_NO_WAIT); + k_sleep(K_MSEC(PROCESSING_DELAY_MS)); + + /* Simulate provisioning failure while network is disconnected */ + handler(&event); + + /* Allow time for state machine to process the failure and enter disconnected state */ + k_sleep(K_SECONDS(INITIAL_PROVISIONING_RETRY_SEC + 1)); + + /* Verify that provisioning is NOT retried (indicating we went to disconnected state) */ + TEST_ASSERT_EQUAL(1, nrf_provisioning_trigger_manually_fake.call_count); +} + /* This is required to be added to each test. That is because unity's * main may return nonzero, while zephyr's main currently must * return 0 in all cases (other values are reserved). diff --git a/tests/state_machines/source_of_truth/cloud.puml b/tests/state_machines/source_of_truth/cloud.puml index a02e4114..3a15cbca 100644 --- a/tests/state_machines/source_of_truth/cloud.puml +++ b/tests/state_machines/source_of_truth/cloud.puml @@ -1,6 +1,4 @@ @startuml -[*] --> STATE_RUNNING - state STATE_RUNNING { [*] --> STATE_DISCONNECTED @@ -8,42 +6,36 @@ state STATE_RUNNING { state STATE_CONNECTING state STATE_CONNECTED - STATE_DISCONNECTED --> STATE_CONNECTING: NETWORK_CONNECTED - STATE_CONNECTING --> STATE_DISCONNECTED: NETWORK_DISCONNECTED + STATE_DISCONNECTED --> STATE_CONNECTING : NETWORK_CONNECTED state STATE_CONNECTING { [*] --> STATE_CONNECTING_ATTEMPT state STATE_CONNECTING_ATTEMPT { [*] --> STATE_PROVISIONED - state STATE_PROVISIONED - state STATE_PROVISIONING - STATE_PROVISIONED --> STATE_PROVISIONING: CLOUD_NOT_AUTHENTICATED - STATE_PROVISIONED --> STATE_CONNECTED: CLOUD_CONNECTION_SUCCESS - STATE_PROVISIONED --> STATE_CONNECTING_BACKOFF: CLOUD_CONNECTION_FAILED - - STATE_PROVISIONING --> STATE_PROVISIONED: CLOUD_PROVISIONING_FINISHED - STATE_PROVISIONING --> STATE_CONNECTING_BACKOFF: CLOUD_PROVISIONING_FAILED + STATE_PROVISIONED --> STATE_PROVISIONING : CLOUD_NOT_AUTHENTICATED + STATE_PROVISIONED --> STATE_CONNECTED : CLOUD_CONNECTION_SUCCESS + STATE_PROVISIONED --> STATE_CONNECTING_BACKOFF : CLOUD_CONNECTION_FAILED } - state STATE_CONNECTING_BACKOFF - - STATE_CONNECTING_BACKOFF --> STATE_PROVISIONED: CLOUD_BACKOFF_EXPIRED && !provisioning_ongoing - STATE_CONNECTING_BACKOFF --> STATE_PROVISIONING: CLOUD_BACKOFF_EXPIRED && provisioning_ongoing + STATE_CONNECTING --> STATE_DISCONNECTED : NETWORK_DISCONNECTED + STATE_PROVISIONING --> STATE_PROVISIONED : CLOUD_PROVISIONING_FINISHED + STATE_PROVISIONING --> STATE_CONNECTING_BACKOFF : CLOUD_PROVISIONING_FAILED && network_connected + STATE_PROVISIONING --> STATE_DISCONNECTED : CLOUD_PROVISIONING_FAILED && !network_connected + STATE_CONNECTING_BACKOFF --> STATE_PROVISIONED : CLOUD_BACKOFF_EXPIRED && !provisioning_ongoing + STATE_CONNECTING_BACKOFF --> STATE_PROVISIONING : CLOUD_BACKOFF_EXPIRED && provisioning_ongoing } state STATE_CONNECTED { [*] --> STATE_CONNECTED_READY - state STATE_CONNECTED_READY - state STATE_CONNECTED_PAUSED - - STATE_CONNECTED_READY --> STATE_CONNECTED_PAUSED: NETWORK_DISCONNECTED - STATE_CONNECTED_PAUSED --> STATE_CONNECTED_READY: NETWORK_CONNECTED - - STATE_CONNECTED_READY --> STATE_CONNECTING: CLOUD_SEND_REQUEST_FAILED - STATE_CONNECTED_READY --> STATE_PROVISIONING: CLOUD_PROVISIONING_REQUEST + STATE_CONNECTED_READY --> STATE_CONNECTED_PAUSED : NETWORK_DISCONNECTED + STATE_CONNECTED_READY --> STATE_CONNECTING : CLOUD_SEND_REQUEST_FAILED + STATE_CONNECTED_READY --> STATE_PROVISIONING : CLOUD_PROVISIONING_REQUEST + STATE_CONNECTED_PAUSED --> STATE_CONNECTED_READY : NETWORK_CONNECTED } } -@enduml \ No newline at end of file + +[*] --> STATE_RUNNING +@enduml