AFBClient: make all call through call()

* Make all calls through call().
* dispatch() aggressively to ensure we actually dispatch our call
  reply too.
* Extend scope tracing, guarded by NDEBUG.
* Remove onReply, always use the locally supplied callable.
* Simplify method implementations making API calls.
* Propagate API call result through call() method return value.

Signed-off-by: Marcus Fritzsch <marcus_fritzsch@mentor.com>

2 files changed, 108 insertions, 94 deletions

diff --git a/AFBClient.cpp b/AFBClient.cpp
index eef3617..407aeac 100644
--- a/AFBClient.cpp
+++ b/AFBClient.cpp
@@ -1,10 +1,11 @@
 #include "AFBClient.h"
 
+#include <cassert>
 #include <cctype>
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
+#include <cerrno>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
 #include <unistd.h>
 
 #define UNUSED(x) (void)(x)
@@ -16,9 +17,15 @@ constexpr const char *const wmAPI = "winman";
 
 #ifdef NDEBUG
 #define TRACE()
+#define TRACEN(N)
 #else
+#define CONCAT_(X, Y) X##Y
+#define CONCAT(X, Y) CONCAT_(X, Y)
+
 #define TRACE() \
-    ScopeTrace __attribute__((unused)) trace_scope_here__(__PRETTY_FUNCTION__)
+    ScopeTrace __attribute__((unused)) CONCAT(trace_scope_, __LINE__)(__func__)
+#define TRACEN(N) \
+    ScopeTrace __attribute__((unused)) CONCAT(named_trace_scope_, __LINE__)(#N)
 
 struct ScopeTrace {
     static int indent;
@@ -66,17 +73,6 @@ void onHangup(void *closure, afb_wsj1 *wsj1) {
     exit(0);
 }
 
-/* called when wsj1 receives a reply */
-void onReply(void *closure, afb_wsj1_msg *msg) {
-    TRACE();
-    printf("ON-REPLY %s: %s\n%s\n", (char *)closure,
-           afb_wsj1_msg_is_reply_ok(msg) ? "OK" : "ERROR",
-           json_object_to_json_string_ext(afb_wsj1_msg_object_j(msg),
-                                          JSON_C_TO_STRING_PRETTY));
-    fflush(stdout);
-    free(closure);
-}
-
 }  // namespace
 
 AFBClient &AFBClient::instance() {
@@ -87,7 +83,6 @@ AFBClient &AFBClient::instance() {
 
 AFBClient::AFBClient() : wsj1{}, itf{}, loop{} {
     TRACE();
-    ///* itinializing the callback interface for wsj1 */
     itf.on_hangup = onHangup;
     itf.on_call = onCall;
     itf.on_event = onEvent;
@@ -95,6 +90,7 @@ AFBClient::AFBClient() : wsj1{}, itf{}, loop{} {
 
 AFBClient::~AFBClient() {
     TRACE();
+    afb_wsj1_unref(wsj1);
     sd_event_unref(loop);
     loop = nullptr;
 }
@@ -150,126 +146,141 @@ fail:
 }
 
 int AFBClient::dispatch(uint64_t timeout) {
-    TRACE();
     return sd_event_run(loop, timeout);
 }
 
 int AFBClient::requestSurface(const char *label) {
     TRACE();
-    constexpr char const *verb = "request_surface";
-
     json_object *jp = json_object_new_object();
     json_object_object_add(jp, "drawing_name", json_object_new_string(label));
-
-    // std::experimental::optional look-alike
-    struct optional {
-        int value;
-        bool is_not_set;
-    };
-
-    constexpr struct optional const nullopt = {0, true};
-    auto id = nullopt;
-
+    int rc = -1;
     /* send the request */
-    int rc = afb_wsj1_call_j(
-        wsj1, wmAPI, verb, jp,
-        [](void *closure, afb_wsj1_msg *msg) {
-            if (afb_wsj1_msg_is_reply_ok(msg)) {
-                int id = json_object_get_int(json_object_object_get(
-                    afb_wsj1_msg_object_j(msg), "response"));
-                auto oid = (optional *)closure;
-                *oid = optional{id};
-            } else
-                fprintf(stderr, "wrong request surface reply received!\n");
-        },
-        (void *)&id);
-
-    if (rc < 0) {
-        fprintf(stderr, "calling %s/%s(%s) failed: %m\n", wmAPI, verb,
-                json_object_to_json_string(jp));
-    } else {
-        // Lets make this call sync here...
-        dispatch(-1);
-
-        if (!id.is_not_set) {
+    int rc2 = call("request_surface", jp, [&rc](bool ok, json_object *j) {
+        if (ok) {
+            int id = json_object_get_int(json_object_object_get(j, "response"));
             char *buf;
-            asprintf(&buf, "%d", id.value);
+            asprintf(&buf, "%d", id);
             printf("setenv(\"QT_IVI_SURFACE_ID\", %s, 1)\n", buf);
             if (setenv("QT_IVI_SURFACE_ID", buf, 1) != 0) {
                 fprintf(stderr, "putenv failed: %m\n");
+                rc = -errno;
             } else {
                 rc = 0;  // Single point of success
             }
         } else {
-            fprintf(stderr, "Could not get surface ID from WM\n");
+            fprintf(
+                stderr, "Could not get surface ID from WM: %s\n",
+                j ? json_object_to_json_string_ext(j, JSON_C_TO_STRING_PRETTY)
+                  : "no-info");
             rc = -EINVAL;
         }
-    }
+    });
 
-    return rc;
+    return rc2 < 0 ? rc2 : rc;
 }
 
 int AFBClient::activateSurface(const char *label) {
     TRACE();
-
-    const char begin[] = "{\"drawing_name\":\"";
-    const char end[] = "\"}";
-    const char verb[] = "activate_surface";
-    char *parameter =
-        (char *)malloc(strlen(begin) + strlen(label) + strlen(end) + 1);
-    strcpy(parameter, begin);
-    strcat(parameter, label);
-    strcat(parameter, end);
-    call(wmAPI, verb, parameter);
-
-    // Sync this one too
-    dispatch(-1);
-
-    return 0;
+    json_object *j = json_object_new_object();
+    json_object_object_add(j, "drawing_name", json_object_new_string(label));
+    return call("activate_surface", j, [](bool ok, json_object *j) {
+        if (!ok) {
+            fprintf(
+                stderr, "API Call activate_surface() failed: %s\n",
+                j ? json_object_to_json_string_ext(j, JSON_C_TO_STRING_PRETTY)
+                  : "no-info");
+        }
+    });
 }
 
 int AFBClient::deactivateSurface(const char *label) {
     TRACE();
     json_object *j = json_object_new_object();
     json_object_object_add(j, "drawing_name", json_object_new_string(label));
-    call(wmAPI, "deactivate_surface", json_object_to_json_string(j));
-    json_object_put(j);
-    dispatch(-1);
-    return 0;
+    return call("deactivate_surface", j, [](bool ok, json_object *j) {
+        if (!ok) {
+            fprintf(
+                stderr, "API Call deactivate_surface() failed: %s\n",
+                j ? json_object_to_json_string_ext(j, JSON_C_TO_STRING_PRETTY)
+                  : "no-info");
+        }
+    });
 }
 
 int AFBClient::endDraw(const char *label) {
     TRACE();
     json_object *j = json_object_new_object();
     json_object_object_add(j, "drawing_name", json_object_new_string(label));
-    call(wmAPI, "enddraw", json_object_to_json_string(j));
-    json_object_put(j);
-    dispatch(-1);
-    return 0;
+    return call("enddraw", j, [](bool ok, json_object *j) {
+        if (!ok) {
+            fprintf(
+                stderr, "API Call endDraw() failed: %s\n",
+                j ? json_object_to_json_string_ext(j, JSON_C_TO_STRING_PRETTY)
+                  : "no-info");
+        }
+    });
 }
 
-/* makes a call */
-void AFBClient::call(const char *api, const char *verb, const char *object) {
+/// object will be json_object_put
+int AFBClient::call(const char *verb, json_object *object,
+                    std::function<void(bool, json_object *)> onReply) {
     TRACE();
-    static int num = 0;
-    char *key;
-    int rc;
 
-    fflush(stdout);
+    // We need to wrap the actual onReply call once in order to
+    // *look* like a normal functions pointer (std::functions<>
+    // with captures cannot convert to function pointers).
+    // Alternatively we could setup a local struct and use it as
+    // closure, but I think it is cleaner this way.
+    int call_rc = 0;
+    bool returned = false;
+    std::function<void(bool, json_object *)> wrappedOnReply =
+        [&returned, &call_rc, &onReply](bool ok, json_object *j) {
+            TRACEN(wrappedOnReply);
+            call_rc = ok ? 0 : -EINVAL;
+            // We know it failed, but there may be an explanation in the
+            // json object.
+            onReply(ok, j);
+            returned = true;
+        };
+
+    // make the actual call, use wrappedOnReply as closure
+    int rc = afb_wsj1_call_j(
+        wsj1, wmAPI, verb, object,
+        [](void *closure, afb_wsj1_msg *msg) {
+            TRACEN(callClosure);
+            auto *onReply =
+                reinterpret_cast<std::function<void(bool, json_object *)> *>(
+                    closure);
+            (*onReply)(!!afb_wsj1_msg_is_reply_ok(msg),
+                       afb_wsj1_msg_object_j(msg));
+        },
+        &wrappedOnReply);
 
-    /* allocates an id for the request */
-    rc = asprintf(&key, "%d:%s/%s", ++num, api, verb);
+    if (rc < 0) {
+        fprintf(
+            stderr, "calling %s/%s(%s) failed: %m\n", wmAPI, verb,
+            json_object_to_json_string_ext(object, JSON_C_TO_STRING_PRETTY));
+        // Call the reply handler regardless with a NULL json_object*
+        onReply(false, nullptr);
+    } else {
+        // We need to dispatch until "returned" got set, this is necessary
+        // if events get triggered by the call (and would be dispatched before
+        // the actual call-reply).
+        while (!returned) {
+            dispatch(-1);
+        }
 
-    /* send the request */
-    rc = afb_wsj1_call_s(wsj1, api, verb, object, onReply, key);
-    if (rc < 0)
-        fprintf(stderr, "calling %s/%s(%s) failed: %m\n", api, verb, object);
+        // return the actual API call result
+        rc = call_rc;
+    }
 
-    fflush(stdout);
+    return rc;
 }
 
-void AFBClient::set_event_handler(enum EventType at,
+void AFBClient::set_event_handler(enum EventType et,
                                   std::function<void(char const *)> func) {
+    UNUSED(et);
+    UNUSED(func);
     TRACE();
     // XXX todo
 }
diff --git a/AFBClient.h b/AFBClient.h
index d776057..a31479c 100644
--- a/AFBClient.h
+++ b/AFBClient.h
@@ -42,10 +42,13 @@ public:
     int deactivateSurface(const char *label);
     int endDraw(const char *label);
 
-    void set_event_handler(enum EventType et, std::function<void(char const *label)> f);
+    void set_event_handler(enum EventType et,
+          std::function<void(char const *label)> f);
 
 private:
-    void call(const char *api, const char *verb, const char *object);
+    /// object will be json_object_put
+    int call(const char *verb, json_object *object,
+          std::function<void(bool ok, json_object*)> onReply);
 
     struct afb_wsj1 *wsj1;
     struct afb_wsj1_itf itf;