Merge pull request #6251 from deepakvenugopal/master

Fixed Thread Commissioning issue in CoAP service

Author: adbridge

features/nanostack/FEATURE_NANOSTACK/coap-service/source/coap_connection_handler.c

@@ -537,7 +537,7 @@ static int timer_status(int8_t timer_id)
     return TIMER_STATE_CANCELLED;
 }
 
-static int read_data(socket_callback_t *sckt_data, internal_socket_t *sock, ns_address_t *src_address, uint8_t dst_address[static 16], int8_t *interface)
+static int read_data(socket_callback_t *sckt_data, internal_socket_t *sock, ns_address_t *src_address, uint8_t dst_address[static 16])
 {
     sock->data_len = 0;
     if (sckt_data->event_type == SOCKET_DATA && sckt_data->d_len > 0) {
@@ -584,7 +584,6 @@ static int read_data(socket_callback_t *sckt_data, internal_socket_t *sock, ns_a
             }
             if (pkt) {
                 memcpy(dst_address, pkt->ipi6_addr, 16);
-                *interface = pkt->ipi6_ifindex;
             } else {
                 goto return_failure;
             }
@@ -614,9 +613,8 @@ static void secure_recv_sckt_msg(void *cb_res)
     ns_address_t src_address;
     uint8_t dst_address[16] = {0};
     memset(&src_address, 0, sizeof(ns_address_t));
-    int8_t interface_id = -1;
 
-    if (sock && read_data(sckt_data, sock, &src_address, dst_address, &interface_id) == 0) {
+    if (sock && read_data(sckt_data, sock, &src_address, dst_address) == 0) {
         /* If received from multicast address, reject */
         if (*(dst_address) == 0xFF) {
             return;
@@ -685,7 +683,7 @@ static void secure_recv_sckt_msg(void *cb_res)
                     ns_dyn_mem_free(data);
                 } else {
                     if (sock->parent->_recv_cb) {
-                        sock->parent->_recv_cb(sock->socket, interface_id, src_address.address, src_address.identifier, dst_address, data, len);
+                        sock->parent->_recv_cb(sock->socket, src_address.address, src_address.identifier, dst_address, data, len);
                     }
                     ns_dyn_mem_free(data);
                 }
@@ -701,11 +699,10 @@ static void recv_sckt_msg(void *cb_res)
     internal_socket_t *sock = int_socket_find_by_socket_id(sckt_data->socket_id);
     ns_address_t src_address;
     uint8_t dst_address[16];
-    int8_t interface_id = -1;
 
-    if (sock && read_data(sckt_data, sock, &src_address, dst_address, &interface_id) == 0) {
+    if (sock && read_data(sckt_data, sock, &src_address, dst_address) == 0) {
         if (sock->parent && sock->parent->_recv_cb) {
-            sock->parent->_recv_cb(sock->socket, interface_id, src_address.address, src_address.identifier, dst_address, sock->data, sock->data_len);
+            sock->parent->_recv_cb(sock->socket, src_address.address, src_address.identifier, dst_address, sock->data, sock->data_len);
         }
         ns_dyn_mem_free(sock->data);
         sock->data = NULL;
@@ -714,8 +711,6 @@ static void recv_sckt_msg(void *cb_res)
 
 int coap_connection_handler_virtual_recv(coap_conn_handler_t *handler, uint8_t address[static 16], uint16_t port, uint8_t *data_ptr, uint16_t data_len)
 {
-    int8_t interface_id = -1;
-
     if(!handler || !handler->socket) {
         return -1;
     }
@@ -792,7 +787,7 @@ int coap_connection_handler_virtual_recv(coap_conn_handler_t *handler, uint8_t a
                     return 0;
                 } else {
                     if (sock->parent->_recv_cb) {
-                        sock->parent->_recv_cb(sock->socket, interface_id, address, port, ns_in6addr_any, data, len);
+                        sock->parent->_recv_cb(sock->socket, address, port, ns_in6addr_any, data, len);
                     }
                     ns_dyn_mem_free(data);
                     data = NULL;
@@ -803,7 +798,7 @@ int coap_connection_handler_virtual_recv(coap_conn_handler_t *handler, uint8_t a
     } else {
         /* unsecure*/
         if (sock->parent->_recv_cb) {
-            sock->parent->_recv_cb(sock->socket, interface_id, address, port, ns_in6addr_any, sock->data, sock->data_len);
+            sock->parent->_recv_cb(sock->socket, address, port, ns_in6addr_any, sock->data, sock->data_len);
         }
         if (sock->data) {
             ns_dyn_mem_free(sock->data);

features/nanostack/FEATURE_NANOSTACK/coap-service/source/coap_message_handler.c

@@ -257,8 +257,8 @@ coap_transaction_t *coap_message_handler_find_transaction(uint8_t *address_ptr,
     return transaction_find_by_address( address_ptr, port );
 }
 
-int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t socket_id, int8_t interface_id, const uint8_t source_addr_ptr[static 16], uint16_t port, const uint8_t dst_addr_ptr[static 16],
-                                      uint8_t *data_ptr, uint16_t data_len, int16_t (cb)(int8_t, int8_t, sn_coap_hdr_s *, coap_transaction_t *))
+int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t socket_id, const uint8_t source_addr_ptr[static 16], uint16_t port, const uint8_t dst_addr_ptr[static 16],
+                                      uint8_t *data_ptr, uint16_t data_len, int16_t (cb)(int8_t, sn_coap_hdr_s *, coap_transaction_t *))
 {
     sn_nsdl_addr_s src_addr;
     sn_coap_hdr_s *coap_message;
@@ -303,7 +303,7 @@ int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t
                 transaction_ptr->token_len = coap_message->token_len;
             }
             transaction_ptr->remote_port = port;
-            if (cb(socket_id, interface_id, coap_message, transaction_ptr) < 0) {
+            if (cb(socket_id, coap_message, transaction_ptr) < 0) {
                 // negative return value = message ignored -> delete transaction
                 transaction_delete(transaction_ptr);
             }
@@ -389,7 +389,20 @@ uint16_t coap_message_handler_request_send(coap_msg_handler_t *handle, int8_t se
         transaction_delete(transaction_ptr);
         return 0;
     }
-    sn_coap_protocol_build(handle->coap, &dst_addr, data_ptr, &request, transaction_ptr);
+    int16_t sn_coap_ret = sn_coap_protocol_build(handle->coap, &dst_addr, data_ptr, &request, transaction_ptr);
+    if (sn_coap_ret == -4) {
+        /*
+         * Not able to add message to resend queue, adjust message lifetime to one resending
+         */
+        transaction_ptr->valid_until = coap_service_get_internal_timer_ticks() + COAP_RESENDING_INTERVAL;
+    } else if (sn_coap_ret < 0) {
+        /*
+         * Failed to build message, set transaction validity time to minimum to get this transaction cleared
+         * immediately and callback called.
+         */
+        transaction_ptr->valid_until = coap_service_get_internal_timer_ticks();
+    }
+
     transaction_ptr->msg_id = request.msg_id;
     handle->sn_coap_tx_callback(data_ptr, data_len, &dst_addr, transaction_ptr);
 

features/nanostack/FEATURE_NANOSTACK/coap-service/source/coap_service_api.c

@@ -36,7 +36,7 @@
 #include "coap_message_handler.h"
 #include "mbed-coap/sn_coap_protocol.h"
 
-static int16_t coap_msg_process_callback(int8_t socket_id, int8_t interface_id, sn_coap_hdr_s *coap_message, coap_transaction_t *transaction_ptr);
+static int16_t coap_msg_process_callback(int8_t socket_id, sn_coap_hdr_s *coap_message, coap_transaction_t *transaction_ptr);
 
 typedef struct uri_registration {
     char *uri_ptr;
@@ -210,7 +210,7 @@ static void service_event_handler(arm_event_s *event)
     eventOS_event_timer_request((uint8_t)COAP_TICK_TIMER, ARM_LIB_SYSTEM_TIMER_EVENT, tasklet_id, 1000);
 }
 
-static int16_t coap_msg_process_callback(int8_t socket_id, int8_t interface_id, sn_coap_hdr_s *coap_message, coap_transaction_t *transaction_ptr)
+static int16_t coap_msg_process_callback(int8_t socket_id, sn_coap_hdr_s *coap_message, coap_transaction_t *transaction_ptr)
 {
     coap_service_t *this;
     if (!coap_message || !transaction_ptr) {
@@ -229,11 +229,6 @@ static int16_t coap_msg_process_callback(int8_t socket_id, int8_t interface_id,
         return -1;
     }
 
-    if ((interface_id != -1) && (this->interface_id != interface_id)) {
-        tr_debug("uri %.*s not registered to interface %d", coap_message->uri_path_len, coap_message->uri_path_ptr, interface_id);
-        return 0;
-    }
-
     uri_registration_t *uri_reg_ptr = uri_registration_find(this, coap_message->uri_path_ptr, coap_message->uri_path_len);
     if (uri_reg_ptr && uri_reg_ptr->request_recv_cb) {
         tr_debug("Service %d, call request recv cb uri %.*s", this->service_id, coap_message->uri_path_len, coap_message->uri_path_ptr);
@@ -249,7 +244,7 @@ static int16_t coap_msg_process_callback(int8_t socket_id, int8_t interface_id,
     return -1;
 }
 
-static int recv_cb(int8_t socket_id, int8_t interface_id, uint8_t src_address[static 16], uint16_t port, const uint8_t dst_address[static 16], unsigned char *data, int len)
+static int recv_cb(int8_t socket_id, uint8_t src_address[static 16], uint16_t port, const uint8_t dst_address[static 16], unsigned char *data, int len)
 {
     uint8_t *data_ptr = NULL;
     uint16_t data_len = 0;
@@ -268,7 +263,7 @@ static int recv_cb(int8_t socket_id, int8_t interface_id, uint8_t src_address[st
     tr_debug("service recv socket data len %d ", data_len);
 
     //parse coap message what CoAP to use
-    int ret = coap_message_handler_coap_msg_process(coap_service_handle, socket_id, interface_id, src_address, port, dst_address, data_ptr, data_len, &coap_msg_process_callback);
+    int ret = coap_message_handler_coap_msg_process(coap_service_handle, socket_id, src_address, port, dst_address, data_ptr, data_len, &coap_msg_process_callback);
     own_free(data_ptr);
     return ret;
 }

features/nanostack/FEATURE_NANOSTACK/coap-service/source/include/coap_connection_handler.h

@@ -35,7 +35,7 @@
 struct internal_socket_s;
 
 typedef int send_to_socket_cb(int8_t socket_id, const uint8_t address[static 16], uint16_t port, const void *, int);
-typedef int receive_from_socket_cb(int8_t socket_id, int8_t interface_id, uint8_t src_address[static 16], uint16_t port, const uint8_t dst_address[static 16], unsigned char *, int);
+typedef int receive_from_socket_cb(int8_t socket_id, uint8_t src_address[static 16], uint16_t port, const uint8_t dst_address[static 16], unsigned char *, int);
 typedef int get_pw_cb(int8_t socket_id, uint8_t address[static 16], uint16_t port, coap_security_keys_t *security_ptr);
 typedef void security_done_cb(int8_t socket_id, uint8_t address[static 16], uint16_t port, uint8_t keyblock[static 40]);
 

features/nanostack/FEATURE_NANOSTACK/coap-service/source/include/coap_message_handler.h

@@ -80,8 +80,8 @@ extern coap_transaction_t *coap_message_handler_transaction_valid(coap_transacti
 
 extern coap_transaction_t *coap_message_handler_find_transaction(uint8_t *address_ptr, uint16_t port);
 
-extern int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t socket_id, int8_t interface_id, const uint8_t source_addr_ptr[static 16], uint16_t port, const uint8_t dst_addr_ptr[static 16],
-                                                         uint8_t *data_ptr, uint16_t data_len, int16_t (cb)(int8_t, int8_t, sn_coap_hdr_s *, coap_transaction_t *));
+extern int16_t coap_message_handler_coap_msg_process(coap_msg_handler_t *handle, int8_t socket_id, const uint8_t source_addr_ptr[static 16], uint16_t port, const uint8_t dst_addr_ptr[static 16],
+                                                         uint8_t *data_ptr, uint16_t data_len, int16_t (cb)(int8_t, sn_coap_hdr_s *, coap_transaction_t *));
 
 extern uint16_t coap_message_handler_request_send(coap_msg_handler_t *handle, int8_t service_id, uint8_t options, const uint8_t destination_addr[static 16],
                                                       uint16_t destination_port, sn_coap_msg_type_e msg_type, sn_coap_msg_code_e msg_code, const char *uri, sn_coap_content_format_e cont_type,

features/nanostack/FEATURE_NANOSTACK/coap-service/test/coap-service/unittest/coap_message_handler/test_coap_message_handler.c

@@ -141,7 +141,7 @@ bool test_coap_message_handler_coap_msg_process()
     uint8_t buf[16];
     memset(&buf, 1, 16);
     /*Handler is null*/
-    if( -1 != coap_message_handler_coap_msg_process(NULL, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, NULL))
+    if( -1 != coap_message_handler_coap_msg_process(NULL, 0, buf, 22, ns_in6addr_any, NULL, 0, NULL))
         return false;
 
     retCounter = 1;
@@ -151,14 +151,14 @@ bool test_coap_message_handler_coap_msg_process()
 
     sn_coap_protocol_stub.expectedHeader = NULL;
     /* Coap parse returns null */
-    if( -1 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( -1 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
     sn_coap_protocol_stub.expectedHeader = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
     memset(sn_coap_protocol_stub.expectedHeader, 0, sizeof(sn_coap_hdr_s));
     sn_coap_protocol_stub.expectedHeader->coap_status = 66;
     /* Coap library responds */
-    if( -1 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( -1 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
     sn_coap_protocol_stub.expectedHeader = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
@@ -167,7 +167,7 @@ bool test_coap_message_handler_coap_msg_process()
     sn_coap_protocol_stub.expectedHeader->msg_code = 1;
     retValue = 0;
     /* request received */
-    if( 0 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( 0 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
     sn_coap_protocol_stub.expectedHeader = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
@@ -176,15 +176,15 @@ bool test_coap_message_handler_coap_msg_process()
     sn_coap_protocol_stub.expectedHeader->msg_code = 1;
     nsdynmemlib_stub.returnCounter = 1;
     retValue = -1;
-    if( 0 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( 0 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
     sn_coap_protocol_stub.expectedHeader = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
     memset(sn_coap_protocol_stub.expectedHeader, 0, sizeof(sn_coap_hdr_s));
     sn_coap_protocol_stub.expectedHeader->coap_status = COAP_STATUS_OK;
     sn_coap_protocol_stub.expectedHeader->msg_code = 333;
 
-    if( -1 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( -1 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
     sn_coap_protocol_stub.expectedHeader = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
@@ -205,7 +205,7 @@ bool test_coap_message_handler_coap_msg_process()
     sn_coap_protocol_stub.expectedHeader->msg_id = 2;
 //    sn_coap_protocol_stub.expectedHeader->token_ptr = (uint8_t*)malloc(4);
 //    memset(sn_coap_protocol_stub.expectedHeader->token_ptr, 1, 4);
-    if( -1 != coap_message_handler_coap_msg_process(handle, 0, -1, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
+    if( -1 != coap_message_handler_coap_msg_process(handle, 0, buf, 22, ns_in6addr_any, NULL, 0, process_cb))
         return false;
 
 //    free(sn_coap_protocol_stub.expectedHeader->token_ptr);
@@ -252,6 +252,36 @@ bool test_coap_message_handler_request_send()
     if( 2 != coap_message_handler_request_send(handle, 3, 0, buf, 24, 1, 2, &uri, 4, NULL, 0, &resp_recv))
         return false;
 
+    /* Clear all transactions */
+    if( 0 != coap_message_handler_exec(handle, 0xffffffff))
+        return false;
+
+    sn_coap_protocol_stub.expectedInt16 = -4;
+    nsdynmemlib_stub.returnCounter = 3;
+    if( 2 != coap_message_handler_request_send(handle, 3, 0, buf, 24, 1, 2, &uri, 4, NULL, 0, &transaction_recv_cb))
+        return false;
+
+    transaction_cb = 0;
+    sn_coap_protocol_stub.expectedInt8 = 0;
+    if( 0 != coap_message_handler_exec(handle, 12))
+        return false;
+
+    if (transaction_cb != 1)
+        return false;
+
+    sn_coap_protocol_stub.expectedInt16 = -2;
+    nsdynmemlib_stub.returnCounter = 3;
+    if( 2 != coap_message_handler_request_send(handle, 3, 0, buf, 24, 1, 2, &uri, 4, NULL, 0, &transaction_recv_cb))
+        return false;
+
+    transaction_cb = 0;
+    if( 0 != coap_message_handler_exec(handle, 2)) {
+        return false;
+    }
+    if (transaction_cb != 1)
+        return false;
+
+
     free(sn_coap_protocol_stub.expectedCoap);
     sn_coap_protocol_stub.expectedCoap = NULL;
     coap_message_handler_destroy(handle);

features/nanostack/FEATURE_NANOSTACK/coap-service/test/coap-service/unittest/coap_service_api/coap_service_apitest.cpp

@@ -117,9 +117,3 @@ TEST(coap_service_api, test_coap_service_handshake_limit_set)
 {
     CHECK(test_coap_service_handshake_limit_set())
 }
-
-TEST(coap_service_api, test_coap_service_secure_session_close)
-{
-    CHECK(test_coap_service_secure_session_close())
-}
-

features/nanostack/FEATURE_NANOSTACK/coap-service/test/coap-service/unittest/coap_service_api/test_coap_service_api.c

@@ -352,22 +352,22 @@ bool test_conn_handler_callbacks()
 
     if( thread_conn_handler_stub.receive_from_sock_cb ){
         coap_message_handler_stub.int16_value = 2;
-        if( -1 != thread_conn_handler_stub.receive_from_sock_cb(1, -1, buf, 12, NULL, NULL, 0))
+        if( -1 != thread_conn_handler_stub.receive_from_sock_cb(1, buf, 12, NULL, NULL, 0))
             return false;
 
         nsdynmemlib_stub.returnCounter = 1;
         uint8_t * ptr = ns_dyn_mem_alloc(5);
         memset(ptr, 3, 5);
         nsdynmemlib_stub.returnCounter = 1;
-        if( 2 != thread_conn_handler_stub.receive_from_sock_cb(1, -1, buf, 12, NULL, ptr, 5))
+        if( 2 != thread_conn_handler_stub.receive_from_sock_cb(1, buf, 12, NULL, ptr, 5))
             return false;
         ns_dyn_mem_free(ptr);
         coap_message_handler_stub.int16_value = 0;
 
         //This could be moved to own test function,
         //but thread_conn_handler_stub.receive_from_sock_cb must be called successfully
         if( coap_message_handler_stub.cb ){
-            if( -1 != coap_message_handler_stub.cb(1, -1, NULL, NULL) )
+            if( -1 != coap_message_handler_stub.cb(1, NULL, NULL) )
                 return false;
 
             sn_coap_hdr_s * coap = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
@@ -377,21 +377,21 @@ bool test_conn_handler_callbacks()
             coap->uri_path_ptr = &uri;
             coap->uri_path_len=2;
 
-            if( -1 != coap_message_handler_stub.cb(1, -1, coap, NULL) )
+            if( -1 != coap_message_handler_stub.cb(1, coap, NULL) )
                 return false;
 
             thread_conn_handler_stub.bool_value = true;
             nsdynmemlib_stub.returnCounter = 2;
             if( 0 != coap_service_register_uri(1, "as", 1, &request_recv_cb) )
                 return false;
 
-            if( -1 != coap_message_handler_stub.cb(1, -1, coap, NULL) )
+            if( -1 != coap_message_handler_stub.cb(1, coap, NULL) )
                 return false;
 
             coap_transaction_t *tr = (coap_transaction_t *)malloc(sizeof(coap_transaction_t));
             memset(tr, 0, sizeof(coap_transaction_t));
 
-            if( 2 != coap_message_handler_stub.cb(1, -1, coap, tr) )
+            if( 2 != coap_message_handler_stub.cb(1, coap, tr) )
                 return false;
 
             free(tr);
@@ -589,24 +589,3 @@ bool test_coap_service_handshake_limit_set()
 
     return true;
 }
-
-bool test_coap_service_secure_session_close()
-{
-    int service_id;
-    uint8_t addr_ptr[16] = {0};
-
-    thread_conn_handler_stub.handler_obj = (coap_conn_handler_t*)malloc(sizeof(coap_conn_handler_t));
-    memset(thread_conn_handler_stub.handler_obj, 0, sizeof(coap_conn_handler_t));
-
-    nsdynmemlib_stub.returnCounter = 1;
-    service_id = coap_service_initialize(1, 2, 0, NULL, NULL );
-
-    coap_service_close_secure_connection(0, NULL, 0);
-
-    coap_service_close_secure_connection(service_id, NULL, 0);
-
-    coap_service_close_secure_connection(service_id, addr_ptr, 1234);
-
-    coap_service_delete(service_id);
-    free(thread_conn_handler_stub.handler_obj);
-}

features/nanostack/FEATURE_NANOSTACK/coap-service/test/coap-service/unittest/coap_service_api/test_coap_service_api.h

@@ -59,8 +59,6 @@ bool test_coap_service_if_find_by_socket();
 
 bool test_coap_service_handshake_limit_set();
 
-bool test_coap_service_secure_session_close();
-
 
 #ifdef __cplusplus
 }