Delete transactions that are not supposed to get reply (#37)

* Delete transactions that are not supposed to get reply

When sending non-confirmable request that is not supposed to get reply,
it must be deleted after sending.

Also some general code-cleaning included.

* Remove outdated transactions from queue

If transaction does not get response, it will be deleted from queue.

Author: Tero Heinonen

source/coap_message_handler.c

@@ -74,12 +74,13 @@ static coap_transaction_t *transaction_create(void)
     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();
         ns_list_add_to_start(&request_list, this);
     }
 
     return this;
 }
-static void transaction_delete(coap_transaction_t *this)
+void transaction_delete(coap_transaction_t *this)
 {
     if (this) {
         ns_list_remove(&request_list, this);
@@ -111,8 +112,6 @@ static int8_t coap_rx_function(sn_coap_hdr_s *resp_ptr, sn_nsdl_addr_s *address_
     return 0;
 }
 
-static void coap_service_build_content_format(sn_coap_hdr_s *header, sn_coap_content_format_e format);
-
 coap_msg_handler_t *coap_message_handler_init(void *(*used_malloc_func_ptr)(uint16_t), void (*used_free_func_ptr)(void *),
                                   uint8_t (*used_tx_callback_ptr)(uint8_t *, uint16_t, sn_nsdl_addr_s *, void *)){
 
@@ -280,7 +279,7 @@ uint16_t coap_message_handler_request_send(coap_msg_handler_t *handle, int8_t se
     request.msg_code = msg_code;
     request.uri_path_ptr = (uint8_t *)uri;
     request.uri_path_len = strlen(uri);
-    coap_service_build_content_format(&request, cont_type);
+    request.content_format = cont_type;
 
     do{
         randLIB_get_n_bytes_random(token,4);
@@ -310,7 +309,6 @@ uint16_t coap_message_handler_request_send(coap_msg_handler_t *handle, int8_t se
     return transaction_ptr->msg_id;
 }
 
-//TODO: refactor this to use nsdl
 int8_t coap_message_handler_response_send(coap_msg_handler_t *handle, int8_t service_id, uint8_t options, sn_coap_hdr_s *request_ptr, sn_coap_msg_code_e message_code, sn_coap_content_format_e content_type, const uint8_t *payload_ptr, uint16_t payload_len)
 {
     coap_transaction_t *transaction_ptr;
@@ -344,46 +342,34 @@ int8_t coap_message_handler_response_send(coap_msg_handler_t *handle, int8_t ser
     }
     response->payload_len = payload_len;
     response->payload_ptr = (uint8_t *) payload_ptr;  // Cast away const and trust that nsdl doesn't modify...
-    coap_service_build_content_format(response, content_type);
+    response->content_format = content_type;
 
     data_len = sn_coap_builder_calc_needed_packet_data_size(response);
     data_ptr = own_alloc(data_len);
     if (data_len > 0 && !data_ptr) {
         sn_coap_parser_release_allocated_coap_msg_mem(handle->coap, response);
-        //TODO deallocate stuff i quess
         return -1;
     }
     sn_coap_protocol_build(handle->coap, &dst_addr, data_ptr, response, transaction_ptr);
     sn_coap_parser_release_allocated_coap_msg_mem(handle->coap, response);
     handle->sn_coap_tx_callback(data_ptr, data_len, &dst_addr, transaction_ptr);
     sn_coap_parser_release_allocated_coap_msg_mem(handle->coap, request_ptr);
-    transaction_delete(transaction_ptr);
     own_free(data_ptr);
     return 0;
 }
 
 int8_t coap_message_handler_exec(coap_msg_handler_t *handle, uint32_t current_time){
+
     if( !handle ){
         return -1;
     }
-    return sn_coap_protocol_exec(handle->coap, current_time);
-}
 
-static void coap_service_build_content_format(sn_coap_hdr_s *header, sn_coap_content_format_e format)
-{
-    header->content_format = format;
-
-//    if (format == COAP_CT_NONE) {
-//        return;
-//    }
-//    if (format == 0) { /* text/plain */
-//        header->content_type_len = 0;
-//    } else if (format <= 0xff) {
-//        header->content_type_ptr[0] = format;
-//        header->content_type_len = 1;
-//    } else {
-//        header->content_type_ptr[0] = format >> 8;
-//        header->content_type_ptr[1] = format & 0xff;
-//        header->content_type_len = 2;
-//    }
+    // 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);
+        }
+    }
+
+    return sn_coap_protocol_exec(handle->coap, current_time);
 }

source/coap_service_api.c

@@ -151,6 +151,8 @@ static uint8_t coap_tx_function(uint8_t *data_ptr, uint16_t data_len, sn_nsdl_ad
         }
         memcpy(transaction_ptr->data_ptr, data_ptr, data_len);
         transaction_ptr->data_len = data_len;
+    } else if (transaction_ptr->resp_cb == NULL ){
+        transaction_delete(transaction_ptr);
     }
 
     return 0;
@@ -191,7 +193,7 @@ static int16_t coap_msg_process_callback(int8_t socket_id, sn_coap_hdr_s *coap_m
     }
 
     uri_registration_t *uri_reg_ptr = uri_registration_find(this, coap_message->uri_path_ptr, coap_message->uri_path_len);
-    if (transaction_ptr && uri_reg_ptr && uri_reg_ptr->request_recv_cb) {
+    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);
 
         if ((this->service_options & COAP_SERVICE_OPTIONS_SECURE_BYPASS) == COAP_SERVICE_OPTIONS_SECURE_BYPASS ) {//TODO Add secure bypass option
@@ -240,13 +242,6 @@ static int send_cb(int8_t socket_id, uint8_t address[static 16], uint16_t port,
     return -1;
 }
 
-//static void sec_conn_closed_cb(int8_t socket_id)
-//{
-//    coap_service_t *this = service_find_by_socket(socket_id);
-
-//    tr_debug("Secure socket was closed by end device");
-//}
-
 static void sec_done_cb(int8_t socket_id, uint8_t address[static 16], uint16_t port, uint8_t keyblock[static 40])
 {
     //TODO: this is not enough if shared socket. Inform all!
@@ -255,7 +250,6 @@ static void sec_done_cb(int8_t socket_id, uint8_t address[static 16], uint16_t p
         this->coap_security_done_cb(this->service_id, address, keyblock);
     }
 
-    //TODO refactor this away. There should be no transaction_ptr(s) before done_cb has been called
     //TODO: send all unsend transactions if more than 1
     coap_transaction_t *transaction_ptr = coap_message_handler_find_transaction(address, port);
     if (transaction_ptr && transaction_ptr->data_ptr) {
@@ -269,7 +263,9 @@ static void sec_done_cb(int8_t socket_id, uint8_t address[static 16], uint16_t p
         ns_dyn_mem_free(transaction_ptr->data_ptr);
         transaction_ptr->data_ptr = NULL;
         transaction_ptr->data_len = 0;
-        //TODO: who deletes transaction incase no response is required
+        if (transaction_ptr->resp_cb == NULL) {
+            transaction_delete(transaction_ptr);
+        }
     }
 }
 

source/include/coap_message_handler.h

@@ -24,6 +24,8 @@
 #include "sn_nsdl.h"
 #include "ns_list.h"
 
+#define TRANSACTION_LIFETIME 180
+
 /**
  * \brief Service message response receive callback.
  *
@@ -47,14 +49,16 @@ typedef struct coap_msg_handler_s {
 
 typedef struct coap_transaction {
     uint8_t remote_address[16];
-    uint16_t remote_port;
     uint8_t token[4];
+    uint32_t create_time;
+    uint16_t remote_port;
     uint16_t msg_id;
+    uint16_t data_len;
     int8_t service_id;
     uint8_t options;
-    bool client_request: 1;
     uint8_t *data_ptr;
-    uint16_t data_len;
+    bool client_request: 1;
+
     coap_message_handler_response_recv *resp_cb;
     ns_list_link_t link;
 } coap_transaction_t;
@@ -81,4 +85,6 @@ extern int8_t coap_message_handler_response_send(coap_msg_handler_t *handle, int
 
 extern int8_t coap_message_handler_exec(coap_msg_handler_t *handle, uint32_t current_time);
 
+extern void transaction_delete(coap_transaction_t *this);
+
 #endif

test/coap-service/unittest/stub/coap_message_handler_stub.c

@@ -17,6 +17,11 @@ coap_msg_handler_t *coap_message_handler_init(void *(*used_malloc_func_ptr)(uint
     return coap_message_handler_stub.coap_ptr;
 }
 
+void transaction_delete(coap_transaction_t *this)
+{
+
+}
+
 int8_t coap_message_handler_destroy(coap_msg_handler_t *handle)
 {
     return coap_message_handler_stub.int8_value;