Merge pull request #146 from ottobackwards/c-api-m4-feedback

C api m4 feedback

sandbox/plc4c/api/include/plc4c/connection.h

@@ -24,6 +24,7 @@ extern "C" {
 
 #include <stdbool.h>
 #include "types.h"
+#include "utils/list.h"
 
 /**
  * Check if the connection is established successfully.
@@ -74,15 +75,22 @@ bool plc4c_connection_supports_reading(plc4c_connection *connection);
  *
  * @param connection connection that this read-request will be executed on.
  * @param num_items number of items we want to read.
- * @param addresses array of address strings.
+ * @param addresses list of address strings.
  * @param read_request pointer to the read-request
  * @param return_code
  */
 return_code plc4c_connection_create_read_request(plc4c_connection *connection,
-                                                 int num_items,
-                                                 char *addresses[],
+                                                 plc4c_list *addresses,
                                                  plc4c_read_request **read_request);
 
+
+
+/**
+ * Destroys a given read_response
+ * @param read_response the read_response
+ */
+void plc4c_connection_read_response_destroy(plc4c_connection *connection, plc4c_read_response *read_response);
+
 /**
  * Check if the current connection supports write operations.
  *
@@ -96,15 +104,14 @@ bool plc4c_connection_supports_writing(plc4c_connection *connection);
  *
  * @param connection connection that this write-request will be executed on.
  * @param num_items number of items we want to write.
- * @param addresses array of address strings.
- * @param values array of pointers to values.
+ * @param addresses list of address strings.
+ * @param values list of pointers to values.
  * @param write_request pointer to the write-request
  * @param return_code
  */
 return_code plc4c_connection_create_write_request(plc4c_connection *connection,
-                                                  int num_items,
-                                                  char *addresses[],
-                                                  void *values[],
+                                                  plc4c_list *addresses,
+                                                  plc4c_list *values,
                                                   plc4c_write_request **write_request);
 
 /**

sandbox/plc4c/api/include/plc4c/read.h

@@ -65,6 +65,7 @@ plc4c_read_response *plc4c_read_request_get_response(plc4c_read_request_executio
  */
 void plc4c_read_request_destroy(plc4c_read_request *read_request);
 
+
 /**
  * Destroys a given read-request execution.
  *

sandbox/plc4c/api/include/plc4c/types.h

@@ -33,6 +33,8 @@ typedef enum return_code {
     OK,
     NO_MEMORY,
     INVALID_CONNECTION_STRING,
+    NON_MATCHING_LISTS,
+    INVALID_LIST_SIZE,
     NOT_REACHABLE,
     PERMISSION_DENIED,
 

sandbox/plc4c/api/include/plc4c/utils/list.h

@@ -27,6 +27,7 @@ extern "C" {
 
 typedef struct plc4c_list plc4c_list;
 typedef struct plc4c_list_element plc4c_list_element;
+typedef void (*plc4c_list_delete_element_callback)(plc4c_list_element *element);
 
 struct plc4c_list {
     plc4c_list_element *head;
@@ -65,6 +66,9 @@ plc4c_list_element *plc4c_utils_list_head(plc4c_list *list);
 
 plc4c_list_element *plc4c_utils_list_tail(plc4c_list *list);
 
+void plc4c_utils_list_delete_elements(plc4c_list *list, plc4c_list_delete_element_callback);
+
+
 #ifdef __cplusplus
 }
 #endif

sandbox/plc4c/drivers/simulated/src/driver_simulated.c

@@ -221,6 +221,22 @@ return_code plc4c_driver_simulated_write_function(plc4c_system_task **task) {
     return OK;
 }
 
+void free_read_item(plc4c_list_element *read_item_element) {
+  plc4c_value_item *value_item = (plc4c_value_item*)read_item_element->value;
+  // do not delete the plc4c_item
+  // we also, in THIS case don't delete the random value which isn't really
+  // a pointer
+  //free(value_item->value);
+  value_item->value = NULL;
+}
+
+void plc4c_driver_simulated_free_read_response(plc4c_read_response *response) {
+  // the request will be cleaned up elsewhere
+  plc4c_utils_list_delete_elements(response->items, &free_read_item);
+}
+
+
+
 plc4c_driver *plc4c_driver_simulated_create() {
     plc4c_driver *driver = (plc4c_driver *) malloc(sizeof(plc4c_driver));
     driver->protocol_code = "simulated";
@@ -231,6 +247,7 @@ plc4c_driver *plc4c_driver_simulated_create() {
     driver->disconnect_function = &plc4c_driver_simulated_disconnect_function;
     driver->read_function = &plc4c_driver_simulated_read_function;
     driver->write_function = &plc4c_driver_simulated_write_function;
+    driver->free_read_response_function = &plc4c_driver_simulated_free_read_response;
     return driver;
 }
 

sandbox/plc4c/examples/hello-world/src/hello_world.c

@@ -43,6 +43,16 @@ void onGlobalDisconnect(plc4c_connection *cur_connection) {
     numOpenConnections--;
 }
 
+void delete_address(plc4c_list_element *address_data_element) {
+  // these are not malloc'd, no need to free
+  address_data_element->value = NULL;
+}
+
+void delete_read_reponse_item(plc4c_list_element *response_item_element) {
+
+
+}
+
 enum plc4c_connection_state_t {
     CONNECTING,
     CONNECTED,
@@ -141,8 +151,11 @@ int main() {
             case CONNECTED: {
                 // Create a new read-request.
                 printf("Preparing a read-request for 'RANDOM/foo:INTEGER' ... ");
-                char *addresses[] = {"RANDOM/foo:INTEGER"};
-                result = plc4c_connection_create_read_request(connection, 1, addresses, &read_request);
+
+                plc4c_list *address_list = NULL;
+                plc4c_utils_list_create(&address_list);
+                plc4c_utils_list_insert_head_value(address_list,(void *)"RANDOM/foo:INTEGER");
+                result = plc4c_connection_create_read_request(connection, address_list, &read_request);
                 if(result != OK) {
                     printf("FAILED\n");
                     return -1;
@@ -158,6 +171,9 @@ int main() {
                 } else {
                     state = READ_REQUEST_SENT;
                 }
+                // if you are going to re-use these, you wouldn't do this
+                plc4c_utils_list_delete_elements(address_list, &delete_address);
+                free(address_list);
                 break;
             }
             // Wait until the read-request execution is finished.
@@ -189,6 +205,10 @@ int main() {
                     cur_element = cur_element->next;
                 }
 
+                // TODO: THE Read Response was custom built opaquely by the driver
+                // SO IT HAS TO BE DELETED BY THE DRIVER
+                plc4c_connection_read_response_destroy(connection, response);
+
                 // TODO: Do something sensible ...
 
                 // Clean up.

sandbox/plc4c/spi/include/plc4c/spi/types_private.h

@@ -44,6 +44,8 @@ typedef return_code (*plc4c_connection_read_function)(plc4c_system_task **task);
 
 typedef return_code (*plc4c_connection_write_function)(plc4c_system_task **task);
 
+typedef void (*plc4c_connect_free_read_response_function)(plc4c_read_response * response);
+
 struct plc4c_system_t {
     /* drivers */
     plc4c_list *driver_list;
@@ -79,6 +81,7 @@ struct plc4c_driver_t {
     plc4c_connection_disconnect_function disconnect_function;
     plc4c_connection_read_function read_function;
     plc4c_connection_write_function write_function;
+    plc4c_connect_free_read_response_function free_read_response_function;
 };
 
 struct plc4c_driver_list_item_t {

sandbox/plc4c/spi/src/connection.c

@@ -51,38 +51,72 @@ bool plc4c_connection_supports_reading(plc4c_connection *connection) {
     return connection->supports_reading;
 }
 
-return_code plc4c_connection_create_read_request(plc4c_connection *connection, int num_items, char* addresses[], plc4c_read_request** read_request) {
+return_code plc4c_connection_create_read_request(plc4c_connection *connection, plc4c_list* addresses, plc4c_read_request** read_request) {
+   // NEED NULL ASSERTS
+
+   // we need something to do
+   if (plc4c_utils_list_size(addresses) == 0){
+    return INVALID_LIST_SIZE;
+   }
     plc4c_read_request *new_read_request = malloc(sizeof(plc4c_read_request));
     new_read_request->connection = connection;
     plc4c_utils_list_create(&(new_read_request->items));
-    for(int i = 0; i < num_items; i++) {
-        plc4c_item *item = connection->driver->parse_address_function(addresses[i]);
+    plc4c_list_element* element = plc4c_utils_list_head(addresses);
+    if (element != NULL) {
+      do {
+        plc4c_item *item = connection->driver->parse_address_function((char *)element->value);
         plc4c_utils_list_insert_tail_value(new_read_request->items, item);
+        element = element->next;
+      } while (element != NULL);
     }
     *read_request = new_read_request;
     return OK;
 }
 
+void plc4c_connection_read_response_destroy(plc4c_connection *connection, plc4c_read_response *read_response){
+ connection->driver->free_read_response_function(read_response);
+}
+
 bool plc4c_connection_supports_writing(plc4c_connection *connection) {
     return connection->supports_writing;
 }
 
-return_code plc4c_connection_create_write_request(plc4c_connection *connection, int num_items, char* addresses[], void* values[], plc4c_write_request** write_request) {
-    plc4c_write_request* new_write_request = (plc4c_write_request*) malloc(sizeof(plc4c_write_request));
+return_code plc4c_connection_create_write_request(plc4c_connection *connection, plc4c_list* addresses, plc4c_list *values, plc4c_write_request** write_request) {
+  // NEED NULL ASSERTS
+
+  // the address and value lists must match
+  if (plc4c_utils_list_size(addresses) != plc4c_utils_list_size(values)) {
+    return NON_MATCHING_LISTS;
+  }
+
+  // we need something to do
+  if (plc4c_utils_list_size(addresses) == 0){
+    return INVALID_LIST_SIZE;
+  }
+  plc4c_write_request* new_write_request = (plc4c_write_request*) malloc(sizeof(plc4c_write_request));
     plc4c_utils_list_create(&(new_write_request->items));
-    for(int i = 0; i < num_items; i++) {
-        char* address = addresses[i];
-        // Parse an address string and get a driver-dependent data-structure representing the address back.
-        plc4c_item* address_item = connection->driver->parse_address_function(address);
-
-        // Create a new value item, binding an address item to a value.
-        plc4c_value_item* value_item = malloc(sizeof(plc4c_value_item));
-        value_item->item = address_item;
-        value_item->value = values[i];
-
-        // Add the new item ot the list of items.
-        plc4c_utils_list_insert_tail_value(new_write_request->items, value_item);
-    }
+
+  plc4c_list_element* address_element = plc4c_utils_list_head(addresses);
+  plc4c_list_element* value_element = plc4c_utils_list_head(values);
+  if (address_element != NULL && value_element != NULL) {
+    do {
+      char *address = (char*)address_element->value;
+      // Parse an address string and get a driver-dependent data-structure representing the address back.
+      plc4c_item *address_item = connection->driver->parse_address_function(address);
+
+      // Create a new value item, binding an address item to a value.
+      plc4c_value_item *value_item = malloc(sizeof(plc4c_value_item));
+      value_item->item = address_item;
+      value_item->value = value_element->value;
+
+      // Add the new item ot the list of items.
+      plc4c_utils_list_insert_tail_value(new_write_request->items, value_item);
+
+      address_element = address_element->next;
+      value_element = value_element->next;
+    } while (address_element != NULL && value_element != NULL);
+  }
+
     write_request = &new_write_request;
     return OK;
 }

sandbox/plc4c/spi/src/utils/list.c

@@ -58,6 +58,10 @@ bool plc4c_utils_list_contains(plc4c_list* list, plc4c_list_element* element) {
 }
 
 void plc4c_utils_list_insert_head(plc4c_list* list, plc4c_list_element* element) {
+    if (list->head == NULL) {
+      list->head = element;
+      return;
+    }
     list->head->next = element;
     element->previous = list->head;
     list->head = element;
@@ -122,17 +126,32 @@ void plc4c_utils_list_remove(plc4c_list* list, plc4c_list_element* element) {
 
 plc4c_list_element *plc4c_utils_list_remove_head(plc4c_list* list) {
     plc4c_list_element *removed_element = list->head;
-    list->head = list->head->previous;
-    list->head->next = NULL;
-    removed_element->previous = NULL;
+    if (removed_element != NULL) {
+      if (list->head->next != NULL) {
+        list->head = list->head->next;
+        list->head->previous = NULL;
+      } else {
+        list->head = NULL;
+      }
+      removed_element->previous = NULL;
+      removed_element->next = NULL;
+    }
     return removed_element;
 }
 
 plc4c_list_element *plc4c_utils_list_remove_tail(plc4c_list* list) {
     plc4c_list_element *removed_element = list->tail;
-    list->tail = list->tail->next;
-    list->tail->previous = NULL;
-    removed_element->next = NULL;
+    if (removed_element != NULL) {
+      if (list->tail->previous != NULL) {
+        list->tail = list->tail->previous;
+        list->tail->next = NULL;
+      } else {
+        list->tail = NULL;
+        list->head = NULL;
+      }
+      removed_element->next = NULL;
+      removed_element->previous = NULL;
+    }
     return removed_element;
 }
 
@@ -143,3 +162,14 @@ plc4c_list_element *plc4c_utils_list_head(plc4c_list *list) {
 plc4c_list_element *plc4c_utils_list_tail(plc4c_list *list) {
     return list->tail;
 }
+
+void plc4c_utils_list_delete_elements(plc4c_list *list, plc4c_list_delete_element_callback callback) {
+  // for each of our elements, call the delete callback
+  plc4c_list_element *head = plc4c_utils_list_remove_head(list);
+  while (head != NULL) {
+    callback(head);
+    free(head);
+    head = plc4c_utils_list_remove_head(list);
+  }
+  // at this point the list is empty
+}