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/22 11:30:50 UTC

[plc4x] branch feature/c-api-promises created (now f3d8d12)

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

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


      at f3d8d12  - Refactored the API to use a promise-like concept - Started writing a document with the design guidelines

This branch includes the following new commits:

     new f3d8d12  - Refactored the API to use a promise-like concept - Started writing a document with the design guidelines

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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

Posted by cd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cdutz pushed a commit to branch feature/c-api-promises
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_