CoAP transaction delete improvements (ARMmbed#91)

Some improvements/fixes for the CoAP transaction delete:
- Calculate transaction lifetime from CoAP retransmission maximum times
- When transaction times out, callback must be called to inform that transaction
was failed

Author: Tero Heinonen

source/coap_message_handler.c

@@ -97,13 +97,28 @@ static coap_transaction_t *transaction_find_by_address(uint8_t *address_ptr, uin
     return this;
 }
 
+/* retransmission valid time is calculated to be max. time that CoAP message sending can take: */
+/* Number of retransmisisons, each retransmission is 2 * previous retransmisison time */
+/* + random factor (max. 1.5) */
+static uint32_t transaction_valid_time_calculate(void)
+{
+    int i;
+    uint32_t time_valid = 0;
+
+    for (i = 0; i <= COAP_RESENDING_COUNT; i++) {
+        time_valid += (COAP_RESENDING_INTERVAL << i) * 1.5;
+    }
+
+    return time_valid + coap_service_get_internal_timer_ticks();
+}
+
 static coap_transaction_t *transaction_create(void)
 {
     coap_transaction_t *this = ns_dyn_mem_alloc(sizeof(coap_transaction_t));
     if (this) {
         memset(this, 0, sizeof(coap_transaction_t));
         this->client_request = true;// default to client initiated method
-        this->create_time = coap_service_get_internal_timer_ticks();
+        this->valid_until = transaction_valid_time_calculate();
         ns_list_add_to_start(&request_list, this);
     }
 
@@ -199,6 +214,9 @@ coap_msg_handler_t *coap_message_handler_init(void *(*used_malloc_func_ptr)(uint
     /* Set default buffer size for CoAP duplicate message detection */
     sn_coap_protocol_set_duplicate_buffer_size(handle->coap, DUPLICATE_MESSAGE_BUFFER_SIZE);
 
+    /* Set default CoAP retransmission paramters */
+    sn_coap_protocol_set_retransmission_parameters(handle->coap, COAP_RESENDING_COUNT, COAP_RESENDING_INTERVAL);
+
     return handle;
 }
 
@@ -518,8 +536,13 @@ int8_t coap_message_handler_exec(coap_msg_handler_t *handle, uint32_t current_ti
 
     // Remove outdated transactions from queue
     ns_list_foreach_safe(coap_transaction_t, transaction, &request_list) {
-        if ((transaction->create_time + TRANSACTION_LIFETIME) < current_time) {
-            transaction_delete(transaction);
+        if (transaction->valid_until < current_time) {
+            tr_debug("transaction %d timed out", transaction->msg_id);
+            ns_list_remove(&request_list, transaction);
+            if (transaction->resp_cb) {
+                transaction->resp_cb(transaction->service_id, transaction->remote_address, transaction->remote_port, NULL);
+            }
+            transaction_free(transaction);
         }
     }
 

source/include/coap_message_handler.h

@@ -26,6 +26,10 @@
 /* Default value for CoAP duplicate message buffer (0 = disabled) */
 #define DUPLICATE_MESSAGE_BUFFER_SIZE 0
 
+/* Default values for CoAP resendings */
+#define COAP_RESENDING_COUNT 3
+#define COAP_RESENDING_INTERVAL 10
+
 /**
  * \brief Service message response receive callback.
  *
@@ -51,7 +55,7 @@ typedef struct coap_transaction {
     uint8_t remote_address[16];
     uint8_t local_address[16];
     uint8_t token[8];
-    uint32_t create_time;
+    uint32_t valid_until;
     uint8_t *data_ptr;
     coap_message_handler_response_recv *resp_cb;
     uint16_t remote_port;

test/coap-service/unittest/coap_message_handler/Makefile

@@ -37,5 +37,5 @@ TEST_SRC_FILES = \
 
 include ../MakefileWorker.mk
 
-CPPUTESTFLAGS += -DFEA_TRACE_SUPPORT
+CPPUTESTFLAGS += -DFEA_TRACE_SUPPORT -I ../../../../source/
 

test/coap-service/unittest/coap_message_handler/test_coap_message_handler.c

@@ -22,11 +22,13 @@
 #include "sn_coap_builder_stub.h"
 #include "sn_coap_parser_stub.h"
 #include "socket_api.h"
+#include "coap_message_handler.c"
 
 int retCounter = 0;
 int retValue = 0;
+int transaction_cb = 0;
 
-static void *own_alloc(uint16_t size)
+static void *test_own_alloc(uint16_t size)
 {
     if( retCounter > 0 ){
         retCounter--;
@@ -35,7 +37,7 @@ static void *own_alloc(uint16_t size)
     return NULL;
 }
 
-static void own_free(void *ptr)
+static void test_own_free(void *ptr)
 {
     if (ptr) {
         free(ptr);
@@ -56,24 +58,30 @@ int16_t process_cb(int8_t a, sn_coap_hdr_s *b, coap_transaction_t *c)
     return retValue;
 }
 
+static int transaction_recv_cb(int8_t service_id, uint8_t source_address[static 16], uint16_t source_port, sn_coap_hdr_s *response_ptr)
+{
+    transaction_cb = 1;
+    return 1;
+}
+
 bool test_coap_message_handler_init()
 {
     if( NULL != coap_message_handler_init(NULL, NULL, NULL) )
         return false;
-    if( NULL != coap_message_handler_init(&own_alloc, NULL, NULL) )
+    if( NULL != coap_message_handler_init(&test_own_alloc, NULL, NULL) )
         return false;
-    if( NULL != coap_message_handler_init(&own_alloc, &own_free, NULL) )
+    if( NULL != coap_message_handler_init(&test_own_alloc, &test_own_free, NULL) )
         return false;
-    if( NULL != coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function) )
+    if( NULL != coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function) )
         return false;
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = NULL;
-    if( NULL != coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function) )
+    if( NULL != coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function) )
         return false;
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
     if( NULL == handle )
         return false;
     free(sn_coap_protocol_stub.expectedCoap);
@@ -89,7 +97,7 @@ bool test_coap_message_handler_destroy()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
 
     if( 0 != coap_message_handler_destroy(handle) )
         return false;
@@ -105,7 +113,7 @@ bool test_coap_message_handler_find_transaction()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
 
     uint8_t buf[16];
     memset(&buf, 1, 16);
@@ -139,7 +147,7 @@ bool test_coap_message_handler_coap_msg_process()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
 
     sn_coap_protocol_stub.expectedHeader = NULL;
     /* Coap parse returns null */
@@ -213,7 +221,7 @@ bool test_coap_message_handler_request_send()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
 
     uint8_t buf[16];
     memset(&buf, 1, 16);
@@ -255,7 +263,7 @@ bool test_coap_message_handler_request_delete()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
 
     uint8_t buf[16];
     memset(&buf, 1, 16);
@@ -291,7 +299,7 @@ bool test_coap_message_handler_response_send()
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
     sn_coap_hdr_s *header = (sn_coap_hdr_s *)malloc(sizeof(sn_coap_hdr_s));
     memset(header, 0, sizeof(sn_coap_hdr_s));
 
@@ -340,15 +348,54 @@ bool test_coap_message_handler_response_send()
 
 bool test_coap_message_handler_exec()
 {
+    /* Null as a parameter */
     if( -1 != coap_message_handler_exec(NULL, 0))
         return false;
+
     retCounter = 1;
     sn_coap_protocol_stub.expectedCoap = (struct coap_s*)malloc(sizeof(struct coap_s));
     memset(sn_coap_protocol_stub.expectedCoap, 0, sizeof(struct coap_s));
-    coap_msg_handler_t *handle = coap_message_handler_init(&own_alloc, &own_free, &coap_tx_function);
+    coap_msg_handler_t *handle = coap_message_handler_init(&test_own_alloc, &test_own_free, &coap_tx_function);
+
+    if( 0 != coap_message_handler_exec(handle, 0))
+        return false;
+
+    nsdynmemlib_stub.returnCounter = 1;
+    coap_transaction_t *transact_ptr = transaction_create();
+
+    /* Transaction not timed out*/
     if( 0 != coap_message_handler_exec(handle, 0))
         return false;
 
+    if (transaction_cb != 0)
+        return false;
+
+    /* Timed out, no CB */
+    if( 0 != coap_message_handler_exec(handle, 300))
+        return false;
+
+    if (transaction_cb != 0)
+        return false;
+
+    nsdynmemlib_stub.returnCounter = 1;
+    transact_ptr = transaction_create();
+    transact_ptr->resp_cb = transaction_recv_cb;
+
+    /* Transaction not timed out */
+    if( 0 != coap_message_handler_exec(handle, 0))
+        return false;
+
+    if (transaction_cb != 0)
+        return false;
+
+    /* Transaction timed out */
+    if( 0 != coap_message_handler_exec(handle, 300))
+        return false;
+
+    if (transaction_cb == 0)
+        return false;
+
+    /* Teardown */
     free(sn_coap_protocol_stub.expectedCoap);
     sn_coap_protocol_stub.expectedCoap = NULL;
     coap_message_handler_destroy(handle);