You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2020/04/23 09:01:55 UTC

[plc4x] 01/02: - Refactored the API to use a promise-like concept - Started writing a document with the design guidelines

This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch feature/c-api
in repository https://gitbox.apache.org/repos/asf/plc4x.git

commit f3d8d1262c93f4481c8198d11f781e78c4fad185
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Wed Apr 22 13:30:38 2020 +0200

    - Refactored the API to use a promise-like concept
    - Started writing a document with the design guidelines
---
 .../plc4c/api/src/main/include/plc4c_connection.h  |  2 +-
 sandbox/plc4c/api/src/main/include/plc4c_system.h  | 27 ++++---
 sandbox/plc4c/api/src/main/include/plc4c_types.h   | 67 +++++++++++++--
 sandbox/plc4c/design-guidelines.adoc               | 53 +++++++++---
 .../examples/hello-world/src/main/c/hello_world.c  | 94 +++++++++++-----------
 sandbox/plc4c/spi/src/main/c/plc4c_connection.c    |  2 +-
 sandbox/plc4c/spi/src/main/c/plc4c_system.c        | 17 ++--
 sandbox/plc4c/spi/src/main/c/plc4c_types.c         | 22 ++++-
 .../spi/src/main/include/plc4c_types_private.h     |  6 ++
 9 files changed, 202 insertions(+), 88 deletions(-)

diff --git a/sandbox/plc4c/api/src/main/include/plc4c_connection.h b/sandbox/plc4c/api/src/main/include/plc4c_connection.h
index a9043f7..b672166 100644
--- a/sandbox/plc4c/api/src/main/include/plc4c_connection.h
+++ b/sandbox/plc4c/api/src/main/include/plc4c_connection.h
@@ -43,7 +43,7 @@ extern "C" {
  * @param connection
  * @param plc4c_connection
  */
-error_code plc4c_connection_disconnect(plc4c_connection *connection);
+return_code plc4c_connection_disconnect(plc4c_connection *connection);
 
 /**
  * Get the connection string from a given connection.
diff --git a/sandbox/plc4c/api/src/main/include/plc4c_system.h b/sandbox/plc4c/api/src/main/include/plc4c_system.h
index 25c53ef..fa5ca9b 100644
--- a/sandbox/plc4c/api/src/main/include/plc4c_system.h
+++ b/sandbox/plc4c/api/src/main/include/plc4c_system.h
@@ -43,7 +43,7 @@ typedef void (*plc4c_system_callback_on_driver_loaded)(plc4c_driver *driver);
  * @param driver_name
  * @param error_code
  */
-typedef void (*plc4c_system_callback_driver_load_error)(const char *driver_name, error_code error);
+typedef void (*plc4c_system_callback_driver_load_error)(const char *driver_name, return_code error);
 
 /**
  * Function pointer for a callback called when is successfully made
@@ -58,7 +58,7 @@ typedef void (*plc4c_system_callback_on_connection)(plc4c_connection *connection
  * @param connection_string
  * @param error_code
  */
-typedef void (*plc4c_system_callback_connection_error)(const char *connection_string, error_code error);
+typedef void (*plc4c_system_callback_connection_error)(const char *connection_string, return_code error);
 
 /**
  * Function pointer for a callback called when is successfully made
@@ -73,7 +73,7 @@ typedef void (*plc4c_system_callback_on_disconnection)(plc4c_connection *connect
  * @param connection
  * @param error_code
  */
-typedef void (*plc4c_system_callback_disconnection_error)(plc4c_connection *connection, error_code error);
+typedef void (*plc4c_system_callback_disconnection_error)(plc4c_connection *connection, return_code error);
 
 /**
  * Function pointer for a callback called when a driver returns an error
@@ -82,7 +82,8 @@ typedef void (*plc4c_system_callback_disconnection_error)(plc4c_connection *conn
  * @param error_code
  */
 typedef void(*plc4c_system_callback_loop_error)
-        (plc4c_driver *driver, plc4c_connection *connection, error_code error);
+        (plc4c_driver *driver, plc4c_connection *connection, return_code error);
+
 
 /**
  * OTHER FUNCTION DEFS FOR SYSTEM
@@ -98,7 +99,7 @@ typedef void(*plc4c_system_callback_loop_error)
  * @param system
  * @return NO_MEMORY if failed to create system
  */
-error_code plc4c_system_create(plc4c_system **system);
+return_code plc4c_system_create(plc4c_system **system);
 
 /**
  * Function to destroy a plc4c_system
@@ -166,9 +167,9 @@ void plc4c_system_set_on_loop_error(plc4c_system *system,
 /**
  * Function to initialize the PLC4C system (Initialize the driver manager and the list of enabled drivers)
  * @param system
- * @return error_code
+ * @return return_code
  */
-error_code plc4c_system_init(plc4c_system *system);
+return_code plc4c_system_init(plc4c_system *system);
 
 /**
  * Function to clean up the PLC4C system (Free any still used resources, terminate live connections, ...)
@@ -182,20 +183,20 @@ void plc4c_system_shutdown(plc4c_system *system);
  *
  * @param system
  * @param connectionString
- * @return error_code INVALID_CONNECTION_STRING, NO_MEMORY
+ * @return return_code INVALID_CONNECTION_STRING, NO_MEMORY
  */
-error_code plc4c_system_connect(plc4c_system *system,
-                                const char *connectionString,
-                                plc4c_connection **connection);
+plc4c_promise* plc4c_system_connect(plc4c_system *system,
+                                   const char *connectionString,
+                                   plc4c_connection **connection);
 
 /**
  * Function to give any drivers the chance to do their work.
  * In single-threaded environments we can't operate with event
  * handler loops as they would block the rest of the application.
  *
- * @return error_code
+ * @return return_code
  */
-error_code plc4c_system_loop();
+return_code plc4c_system_loop();
 
 #ifdef __cplusplus
 }
diff --git a/sandbox/plc4c/api/src/main/include/plc4c_types.h b/sandbox/plc4c/api/src/main/include/plc4c_types.h
index 99e2c35..849ad07 100644
--- a/sandbox/plc4c/api/src/main/include/plc4c_types.h
+++ b/sandbox/plc4c/api/src/main/include/plc4c_types.h
@@ -18,6 +18,9 @@
  */
 #ifndef PLC4C_TYPES_H_
 #define PLC4C_TYPES_H_
+
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -26,7 +29,8 @@ extern "C" {
  *
  * PLC4C error codes
 */
-typedef enum error_code {
+typedef enum return_code {
+    UNFINISHED,
     OK,
     UNKNOWN_ERROR,
     NO_MEMORY,
@@ -34,15 +38,15 @@ typedef enum error_code {
     NOT_REACHABLE,
     PERMISSION_DENIED,
     INTERNAL_ERROR
-} error_code;
+} return_code;
 
 /**
- * Helper that translates from an error_code enum value to something a human can work with.
+ * Helper that translates from an return_code enum value to something a human can work with.
  *
- * @param err error code.
- * @return A human readable error description.
+ * @param err return code.
+ * @return A human readable description.
  */
-char *plc4c_error_code_to_error_message(error_code err);
+char *plc4c_return_code_to_message(return_code err);
 
 /**
  * the plc4c system
@@ -59,6 +63,57 @@ typedef struct plc4c_driver_t plc4c_driver;
  */
 typedef struct plc4c_connection_t plc4c_connection;
 
+/**
+ * Return type for any form of async operation.
+ */
+typedef struct plc4c_promise_t plc4c_promise;
+
+/**
+ * Callback for any form of successful async operation.
+ */
+typedef void (*plc4c_success_callback)(plc4c_promise *promise);
+
+/**
+ * Callback for any form of failed async operation.
+ */
+typedef void (*plc4c_failure_callback)(plc4c_promise *promise);
+
+/**
+ * Function to register a success callback on a given plc4c_promise.
+ * @param promise the promise to set the callback on
+ * @param successCallback the callback
+ */
+void plc4c_types_promise_set_success_callback(plc4c_promise* promise, plc4c_success_callback successCallback);
+
+/**
+ * Function to register a failure callback on a given plc4c_promise.
+ * @param promise the promise to set the callback on
+ * @param successCallback the callback
+ */
+void plc4c_types_promise_set_failure_callback(plc4c_promise* promise, plc4c_failure_callback failureCallback);
+
+/**
+ * Check if a promise is completed
+ * @param promise the promise
+ * @return true if the promise is in a final state
+ */
+bool plc4c_types_promise_completed(plc4c_promise* promise);
+
+/**
+ * Check if a promise is completed successfully
+ * @param promise the promise
+ * @return true if the promise is the state OK
+ */
+bool plc4c_types_promise_completed_successfully(plc4c_promise* promise);
+
+/**
+ * Check if a promise is completed unsuccessfully
+ * @param promise the promise
+ * @return true if the promise is in a final state other than OK
+ */
+bool plc4c_types_promise_completed_unsuccessfully(plc4c_promise* promise);
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/sandbox/plc4c/design-guidelines.adoc b/sandbox/plc4c/design-guidelines.adoc
index d58b3e9..7f18c82 100644
--- a/sandbox/plc4c/design-guidelines.adoc
+++ b/sandbox/plc4c/design-guidelines.adoc
@@ -15,16 +15,47 @@
 //  limitations under the License.
 //
 
-http://www.apibook.com/blog/archives/186
+== General thoughts
 
-- All PLC4C API related functions should use the prefix `plc4c_`
-- Only public API related symbols should be exported
-- Establish an object model
-- Supply a plc4c_init() function to initialize the DriverManager
-- Supply a plc4c_shutdown() function for cleaning up open connections
-- Provide a free()/unref() function for each kind of allocated object
+* Generally all operations in PLC4X that require any form of interaction with a remote system are to be considered async operations.
+** Async functions should return a plc4x_promise object
+** Example functions:
+*** Connection
+*** Read
+*** Write
+*** Subscribe
+*** Unsubscribe
+*** Disconnect
+*** Destroy PLC4X System (As perhaps open connections need to be terminated)
+*** Shutdown PLC4X System (As perhaps open connections need to be terminated)
+* Generally all others are considered synchronous operations
+** Sync functions should return a return_code
+** Example functions
+*** Create PLC4X System
+*** Init PLC4X System
+*** Load Driver
+* Every function (except setters) should return either a return_code or a plc4c_promise
+* No function generally does any IO, they just interact with the state
+** Example: read operation just adds a new task to the connection object
+** Only function doing IO is the plc4c_system_loop function which has the job of sending outgoing data and processing incoming data
+** Different strategies could be applied to handle scheduling of task processing in plc4c_system_loop to avoid hogging too much CPU time
 
-- Error handling
-    - Every function returns an error state (ideally an enum)
-    - Function for converting enum error code to something human-readable
-    - "Return" structures or real return values by double-pointer arguments
+== API Usage
+
+* At first the user needs to create an instance of the plc4c_system, which contains all system-relevant data:
+** System configuration
+** Drivers
+** Connections
+** ...
+* After the plc4c_system is created the user then initializes the system
+** Initializes the driver manager
+** Initializes any internal structures
+** Loads drivers
+* After the plc4c_system is initialized, the user can create plc4c_connections by using the plc4c_system_connect function
+** Returns a promise
+* The user can attach success- and failure-callbacks to the promise
+
+== Open Questions
+
+* Perhaps all functions should use the async pattern as otherwise users have to know these details.
+* Not sure if perhaps the serialization of messages could be done in the functions and the loop just deals with the sending/reading ... however I would opt for having this functionality in the loop as for intermediate states it would have to handle these anyway.
\ No newline at end of file
diff --git a/sandbox/plc4c/examples/hello-world/src/main/c/hello_world.c b/sandbox/plc4c/examples/hello-world/src/main/c/hello_world.c
index 50461c9..666f3b5 100644
--- a/sandbox/plc4c/examples/hello-world/src/main/c/hello_world.c
+++ b/sandbox/plc4c/examples/hello-world/src/main/c/hello_world.c
@@ -19,68 +19,66 @@
 #include <stdio.h>
 #include "../../../../../api/src/main/include/plc4c.h"
 
-void onConnectionSuccess(plc4c_connection *connection) {
+void onGlobalConnectionSuccess(plc4c_connection *connection) {
     printf("Connected to %s", plc4c_connection_get_connection_string(connection));
 }
 
-void onConnectionError(const char *connection_string, error_code error) {
-    printf("Error connecting to %s. Got error %s", connection_string, plc4c_error_code_to_error_message(error));
+void onGlobalConnectionError(const char *connection_string, return_code error) {
+    printf("Error connecting to %s. Got error %s", connection_string, plc4c_return_code_to_message(error));
 }
 
-void onConnection2Success(plc4c_connection *connection) {
+void onLocalConnectionSuccess(plc4c_promise* promise) {
 
 }
 
-void onConnection2Error(const char *connection_string, error_code error) {
+void onLocalConnectionFailure(plc4c_promise* promise) {
 
 }
 
 int main() {
-  bool loop = true;
-  plc4c_system *system = NULL;
-  plc4c_connection *connection1 = NULL;
-  plc4c_connection *connection2 = NULL;
-
-  // Create a new uninitialized plc4c_system
-  error_code error = plc4c_system_create(&system);
-  if (error != OK) {
-    return -1;
-  }
-
-  // Initialize the plc4c_system (loading of drivers, setting up other stuff, ...)
-  error = plc4c_system_init(system);
-  if (error != OK) {
-    return -1;
-  }
-
-  // Register the global callbacks.
-  plc4c_system_set_on_connection(system, &onConnectionSuccess);
-  plc4c_system_set_on_connection_error(system, &onConnectionError);
-
-  // Establish connections to remote devices
-  // you may or may not care about the connection handle
-  error = plc4c_system_connect(system, "s7://192.168.42.20", &connection1);
-  if (error != OK) {
-    return -1;
-  }
-
-  error = plc4c_system_connect(system, "s7://192.168.42.22", &connection2);
-  if (error != OK) {
-    return -1;
-  }
-
-  // Central program loop ...
-  while (loop) {
-    if (plc4c_system_loop(system) != OK) {
-      break;
+    bool loop = true;
+    plc4c_system *system = NULL;
+    plc4c_connection *connection = NULL;
+
+    // Create a new uninitialized plc4c_system
+    return_code result = plc4c_system_create(&system);
+    if (result != OK) {
+        return -1;
     }
-  }
 
-  // Make sure everything is cleaned up correctly.
-  plc4c_system_shutdown(system);
+    // Initialize the plc4c_system (loading of drivers, setting up other stuff, ...)
+    result = plc4c_system_init(system);
+    if (result != OK) {
+        return -1;
+    }
+
+    // Register the global callbacks.
+    plc4c_system_set_on_connection(system, &onGlobalConnectionSuccess);
+    plc4c_system_set_on_connection_error(system, &onGlobalConnectionError);
+
+    // Establish connections to remote devices
+    // you may or may not care about the connection handle
+    plc4c_promise* connect_promise = plc4c_system_connect(system, "s7://192.168.42.20", &connection);
+    plc4c_types_promise_set_success_callback(connect_promise, &onLocalConnectionSuccess);
+    plc4c_types_promise_set_failure_callback(connect_promise, &onLocalConnectionFailure);
+    if (plc4c_types_promise_completed_unsuccessfully(connect_promise)) {
+        return -1;
+    }
+
+    // Central program loop ...
+    while (loop) {
+        if (plc4c_system_loop(system) != OK) {
+            break;
+        }
+
+
+    }
+
+    // Make sure everything is cleaned up correctly.
+    plc4c_system_shutdown(system);
 
-  // Finally destroy the plc4c_system, freeing up all memory allocated by plc4c.
-  plc4c_system_destroy(system);
+    // Finally destroy the plc4c_system, freeing up all memory allocated by plc4c.
+    plc4c_system_destroy(system);
 
-  return 0;
+    return 0;
 }
diff --git a/sandbox/plc4c/spi/src/main/c/plc4c_connection.c b/sandbox/plc4c/spi/src/main/c/plc4c_connection.c
index c629007..54b66e8 100644
--- a/sandbox/plc4c/spi/src/main/c/plc4c_connection.c
+++ b/sandbox/plc4c/spi/src/main/c/plc4c_connection.c
@@ -20,7 +20,7 @@
 #include "plc4c_connection.h"
 #include "plc4c_types_private.h"
 
-error_code plc4c_connection_disconnect(plc4c_connection *connection) {
+return_code plc4c_connection_disconnect(plc4c_connection *connection) {
     return OK;
 }
 
diff --git a/sandbox/plc4c/spi/src/main/c/plc4c_system.c b/sandbox/plc4c/spi/src/main/c/plc4c_system.c
index 5532443..487689e 100644
--- a/sandbox/plc4c/spi/src/main/c/plc4c_system.c
+++ b/sandbox/plc4c/spi/src/main/c/plc4c_system.c
@@ -17,10 +17,11 @@
  * under the License.
  */
 
+#include <stdlib.h>
 #include "plc4c_system.h"
 #include "plc4c_types_private.h"
 
-error_code plc4c_system_create(plc4c_system **system) {
+return_code plc4c_system_create(plc4c_system **system) {
     return OK;
 }
 
@@ -63,7 +64,7 @@ void plc4c_system_set_on_loop_error(plc4c_system *system,
 
 }
 
-error_code plc4c_system_init(plc4c_system *system) {
+return_code plc4c_system_init(plc4c_system *system) {
     return OK;
 }
 
@@ -71,13 +72,15 @@ void plc4c_system_shutdown(plc4c_system *system) {
 
 }
 
-error_code plc4c_system_connect(plc4c_system *system,
-                                const char *connectionString,
-                                plc4c_connection **connection) {
-    return OK;
+plc4c_promise* plc4c_system_connect(plc4c_system *system,
+                                   const char *connectionString,
+                                   plc4c_connection **connection) {
+    plc4c_promise* result = (plc4c_promise*) malloc(sizeof(plc4c_promise));
+    result->returnCode = UNFINISHED;
+    return result;
 }
 
-error_code plc4c_system_loop() {
+return_code plc4c_system_loop() {
     return OK;
 }
 
diff --git a/sandbox/plc4c/spi/src/main/c/plc4c_types.c b/sandbox/plc4c/spi/src/main/c/plc4c_types.c
index 0cea7ef..ce3e6d0 100644
--- a/sandbox/plc4c/spi/src/main/c/plc4c_types.c
+++ b/sandbox/plc4c/spi/src/main/c/plc4c_types.c
@@ -20,7 +20,27 @@
 #include "plc4c_types.h"
 #include "plc4c_types_private.h"
 
-char *plc4c_error_code_to_error_message(error_code err) {
+char *plc4c_return_code_to_message(return_code err) {
     return "hurz";
 }
 
+void plc4c_types_promise_set_success_callback(plc4c_promise* promise, plc4c_success_callback successCallback) {
+    promise->successCallback = successCallback;
+}
+
+void plc4c_types_promise_set_failure_callback(plc4c_promise* promise, plc4c_failure_callback failureCallback) {
+    promise->failureCallback = failureCallback;
+}
+
+bool plc4c_types_promise_completed(plc4c_promise* promise) {
+    return promise->returnCode != UNFINISHED;
+}
+
+bool plc4c_types_promise_completed_successfully(plc4c_promise* promise) {
+    return promise->returnCode == OK;
+}
+
+bool plc4c_types_promise_completed_unsuccessfully(plc4c_promise* promise) {
+    return plc4c_types_promise_completed(promise) && !plc4c_types_promise_completed_successfully(promise);
+}
+
diff --git a/sandbox/plc4c/spi/src/main/include/plc4c_types_private.h b/sandbox/plc4c/spi/src/main/include/plc4c_types_private.h
index fe6f990..cc2104f 100644
--- a/sandbox/plc4c/spi/src/main/include/plc4c_types_private.h
+++ b/sandbox/plc4c/spi/src/main/include/plc4c_types_private.h
@@ -38,4 +38,10 @@ struct plc4c_connection_t {
   /* ???? */
 };
 
+struct plc4c_promise_t {
+    return_code returnCode;
+    plc4c_success_callback successCallback;
+    plc4c_failure_callback failureCallback;
+};
+
 #endif //PLC4C_SPI_TYPES_PRIVATE_H_