doc comments, clarify data ownership in scene, use subtyping for scene struct

main/liquid/liquid.c

@@ -4,6 +4,7 @@
 #include <malloc.h>
 #include <assert.h>
 #include <esp_log.h>
+#include <nokia.h>
 
 static const char *TAG = "Liquid";
 
@@ -16,7 +17,7 @@ struct Liquid {
     SLIST_HEAD(, RunningScene) stack; // stack with the topmost scene as the first element / head
 };
 
-static struct SceneEvent Default_onChildReturn(struct Scene *scene, uint32_t tag, uint32_t status, void *data) {
+static struct SceneEvent Default_onChildReturn(struct Scene *scene, uint32_t tag, int32_t status, void *data) {
     if (data) free(data);
     return SceneEvent_Repaint();
 }
@@ -29,7 +30,7 @@ static void addChild(struct Liquid *container, struct Scene *child) {
     SLIST_INSERT_HEAD(&container->stack, elm, next);
 }
 
-struct Liquid *Liquid_start() {
+struct Liquid *Liquid_start(void) {
     struct Liquid *container = calloc(1, sizeof(struct Liquid));
 
     addChild(container, NewScene_Root());
@@ -39,14 +40,16 @@ struct Liquid *Liquid_start() {
 
 static struct SceneEvent handleSceneEvent(struct Liquid *container, struct SceneEvent event) {
     struct RunningScene *topmost = SLIST_FIRST(&container->stack);
+    struct Scene *newScene;
+    uint32_t tag;
     switch (event.kind) {
         case SceneEventKind_Close:
             assert(SLIST_NEXT(topmost, next) != NULL);
             SLIST_REMOVE_HEAD(&container->stack, next);
-            uint32_t tag = topmost->scene->tag;
+            tag = topmost->scene->tag;
 
-            if (topmost->scene->options) {
-                free(topmost->scene->options);
+            if (topmost->scene->free) {
+                topmost->scene->free(topmost->scene);
             }
             free(topmost->scene);
             free(topmost);
@@ -56,12 +59,12 @@ static struct SceneEvent handleSceneEvent(struct Liquid *container, struct Scene
             // this is always set.
             return topmost->scene->onChildReturn(topmost->scene, tag, event.close.status, event.close.data);
 
-        case SceneEventKind_OpenChild:;
+        case SceneEventKind_OpenChild:
             if (!event.open.scene) {
                 ESP_LOGE(TAG, "Attempt to open NULL scene!");
                 return SceneEvent_None();
             }
-            struct Scene *newScene = event.open.scene;
+            newScene = event.open.scene;
             newScene->tag = event.open.tag;
             addChild(container, newScene);
             return SceneEvent_Repaint();
@@ -74,14 +77,23 @@ static struct SceneEvent handleSceneEvent(struct Liquid *container, struct Scene
     }
 }
 
-bool processReturnValue(struct Liquid *container, struct SceneEvent result) {
+static bool processReturnValue(struct Liquid *container, struct SceneEvent result) {
+    bool repaint = false;
     while (result.kind != SceneEventKind_None) {
         if (result.kind == SceneEventKind_RequestRepaint) {
-            return 1; // repaint
+            // Repaint explicitly requested, there's no more chained events.
+            repaint = true;
+            break;
         }
+        if (result.kind == SceneEventKind_Close) {
+            // child close always triggers repaint, but the event handler can return
+            // another event (e.g. close itself, or open a new child).
+            repaint = true;
+        }
+        // one event can lead to another...
         result = handleSceneEvent(container, result);
     }
-    return 0;
+    return repaint;
 }
 
 bool Liquid_handleInput(struct Liquid *container, struct InputEvent event) {
@@ -112,4 +124,6 @@ void Liquid_paint(struct Liquid *container) {
     struct RunningScene *topmost = SLIST_FIRST(&container->stack);
     assert(topmost != NULL);
     topmost->scene->paint(topmost->scene);
+
+    LCD_updateDisplay();
 }

main/liquid/liquid.h

@@ -22,7 +22,7 @@ struct InputEvent {
     union {
         struct {
             // Wheel delta
-            int8_t delta;
+            int32_t delta;
         } wheel;
         struct {
             // Button state
@@ -31,99 +31,206 @@ struct InputEvent {
     };
 };
 
-#define InputEvent_Wheel(delta_) ((struct InputEvent) { \
-    .kind = InputEventKind_Wheel, \
-    .wheel = { .delta = delta_ } \
-})
+static inline struct InputEvent InputEvent_Wheel(int32_t delta)
+{
+    return (struct InputEvent) {
+        .kind = InputEventKind_Wheel,
+        .wheel = {.delta = delta}
+    };
+}
 
-#define InputEvent_Button(state_) ((struct InputEvent) { \
-    .kind = InputEventKind_Button, \
-    .button = { .state = state_ }, \
-})
+/**
+ * Button event (push, release)
+ *
+ * @param state - pushed
+ * @return event
+ */
+static inline struct InputEvent InputEvent_Button(bool state)
+{
+    return (struct InputEvent) {
+        .kind = InputEventKind_Button,
+        .button = {.state = state},
+    };
+}
 
 enum SceneEvent_Kind {
-    // Close this scene.
-    // The scene is responsible for cleaning up 'private'.
-    // 'options' and the scene itself will be freed by the GUI engine.
     SceneEventKind_Close,
     SceneEventKind_OpenChild,
     SceneEventKind_RequestRepaint,
     SceneEventKind_None,
 };
 
+// forward declaration
+struct Scene;
+
+/**
+ * Scene event, returned from some scene methods.
+ *
+ * Use the constructor functions to create this struct.
+ */
 struct SceneEvent {
+    /** Event kind enum */
     enum SceneEvent_Kind kind;
     union {
+        /* data for Close event kind */
         struct {
             // Status code
-            uint32_t status;
+            int32_t status;
             // Return data on heap
             void *data;
         } close;
+        /* Data for Open event kind */
         struct {
             // Scene (initialized, with options loaded in through the constructor)
-            void *scene;
+            struct Scene *scene;
             // Tag used by parent to identify the open child
             uint32_t tag;
         } open;
     };
 };
 
-struct Scene;
-
-#define SceneEvent_None() ((struct SceneEvent) { \
-    .kind = SceneEventKind_None\
-})
+/**
+ * Create empty (null object) scene event.
+ *
+ * @return event
+ */
+static inline struct SceneEvent SceneEvent_None(void)
+{
+    return (struct SceneEvent) {
+        .kind = SceneEventKind_None,
+    };
+}
 
-#define SceneEvent_Repaint() ((struct SceneEvent) { \
-    .kind = SceneEventKind_RequestRepaint\
-})
+/**
+ * Request scene repaint
+ *
+ * @return event
+ */
+static inline struct SceneEvent SceneEvent_Repaint(void)
+{
+    return (struct SceneEvent) {
+        .kind = SceneEventKind_RequestRepaint
+    };
+}
 
-#define SceneEvent_OpenChild(child_, tag_) ((struct SceneEvent) { \
-    .kind = SceneEventKind_OpenChild,\
-    .open = { .scene = (child_), .tag=tag_ }, \
-})
+/**
+ * Request a sub-scene to be opened
+ *
+ * @param child - child scene
+ * @param tag - scene tag
+ * @return event
+ */
+static inline struct SceneEvent SceneEvent_OpenChild(struct Scene *child, uint32_t tag) {
+    return (struct SceneEvent) {
+        .kind = SceneEventKind_OpenChild,
+        .open = { .scene = child, .tag=tag },
+    };
+}
 
-#define SceneEvent_Close(status_, data_) ((struct SceneEvent) { \
-    .kind = SceneEventKind_Close,\
-    .close = { .status = (status_), .data=(data_) }, \
-})
+/**
+ * Close this scene, returning to parent.
+ *
+ * @param status - status number for the parent
+ * @param data - heap-allocated data for parent, can be NULL; e.g. user input as string.
+ * @return event
+ */
+static inline struct SceneEvent SceneEvent_Close(int32_t status, void *data)
+{
+    return (struct SceneEvent) {
+        .kind = SceneEventKind_Close,
+        .close = {.status = status, .data=data},
+    };
+}
 
+/**
+ * Scene::onInput fp type - handle user input
+ *
+ * @param scene - self
+ * @param event - the input event
+ * @return follow-up scene event, can be SceneEvent_None() if not used.
+ */
 typedef struct SceneEvent (*Scene_onInput_t)(struct Scene *scene, struct InputEvent event);
-typedef struct SceneEvent (*Scene_onChildReturn_t)(struct Scene *scene, uint32_t tag, uint32_t status, void *data);
+
+/**
+ * Scene::onChildReturn fp type
+ *
+ * @param scene - self
+ * @param tag - child's tag
+ * @param status - status code returned from the child
+ * @param data - data returned from the child, must be heap-allocated if not NULL.
+ * @return follow-up scene event, can be SceneEvent_None() if not used.
+ */
+typedef struct SceneEvent (*Scene_onChildReturn_t)(struct Scene *scene, uint32_t tag, int32_t status, void *data);
+
+/**
+ * Scene::onTick fp type
+ *
+ * @param scene - self
+ * @return follow-up scene event, can be SceneEvent_None() if not used.
+ */
 typedef struct SceneEvent (*Scene_onTick_t)(struct Scene *scene);
-typedef void (*Scene_repaint_t)(struct Scene *scene);
 
+/**
+ * Scene::paint fp type
+ *
+ * @param scene - self
+ */
+typedef void (*Scene_paint_t)(struct Scene *scene);
+
+/**
+ * Scene::free fp type.
+ * Release internally allocated resources.
+ * This is called by the GUI engine when the scene is closed.
+ *
+ * @param scene - self
+ */
+typedef void (*Scene_free_t)(struct Scene *scene);
+
+/**
+ * Scene instance in the framework
+ */
 struct Scene {
+    /**
+     * Tag given to the scene by its parent to identify its close event.
+     */
     uint32_t tag;
-    void *options;
-    void *private;
-    /** Handle input */
+
+    /**
+     * Handle input event.
+     * Can return a follow-up scene event, e.g. repaint request.
+     * Nullable field.
+     */
     Scene_onInput_t onInput;
-    /** Handle child scene return */
+
+    /**
+     * Child scene closed, handle its return value and data, if any.
+     * Can return a follow-up scene event.
+     * In any case, the scene will be re-painted, if it stays open and on top.
+     * Nullable field.
+     */
     Scene_onChildReturn_t onChildReturn;
-    /** Periodic tick */
+
+    /**
+     * Handle a periodic tick (10ms).
+     * Can return a follow-up scene event, e.g. repaint request.
+     * Nullable field.
+     */
     Scene_onTick_t onTick;
-    /** Periodic tick */
-    Scene_repaint_t paint;
-};
 
-/**
- * Allocate scene and the inner private type.
- *
- * Requires <malloc.h>
- *
- * Must be used like:
- *
- * struct Scene *scene = SCENE_SAFE_ALLOC(struct private);
- * scene->onInput = ...
- * return scene;
- */
-#define SCENE_SAFE_ALLOC(private_type) \
-    calloc(1, sizeof(struct Scene)); \
-    if (!scene) return NULL; \
-    scene->private = calloc(1, sizeof(private_type)); \
-    if (!scene->private) return NULL;
+    /**
+     * Draw the scene to the LCD buffer.
+     * DO NOT write to the display yet, it will be done by the GUI engine.
+     *
+     * MANDATORY FIELD
+     */
+    Scene_paint_t paint;
+
+    /**
+     * Release internally allocated resources, if any. Called on close.
+     * Nullable field.
+     */
+    Scene_free_t free;
+};
 
 /** return 1 if repaint requested */
 bool Liquid_handleInput(struct Liquid *container, struct InputEvent event);
@@ -131,7 +238,10 @@ bool Liquid_handleInput(struct Liquid *container, struct InputEvent event);
 /** return 1 if repaint requested */
 bool Liquid_handleTick(struct Liquid *container);
 
+/** render the active scene */
 void Liquid_paint(struct Liquid *container);
-struct Liquid *Liquid_start();
+
+/** Initialize the GUI system with a root scene */
+struct Liquid *Liquid_start(void);
 
 #endif //REFLOWER_LIQUID_H

main/liquid/scene_car.c

@@ -3,17 +3,17 @@
 #include "../graphics/nokia.h"
 #include <malloc.h>
 
-struct private {
+struct CarScene {
+    struct Scene base;
     int32_t pos;
 };
 
-static struct SceneEvent Car_onInput(struct Scene *scene, struct InputEvent event) {
-    struct private *priv = scene->private;
+static struct SceneEvent Car_onInput(struct CarScene *self, struct InputEvent event) {
     switch (event.kind) {
         case InputEventKind_Wheel:
-            priv->pos += event.wheel.delta;
-            if (priv->pos < 0) priv->pos = 0;
-            if (priv->pos > LCD_WIDTH-21) priv->pos = LCD_WIDTH-21;
+            self->pos += event.wheel.delta;
+            if (self->pos < 0) self->pos = 0;
+            if (self->pos > LCD_WIDTH-21) self->pos = LCD_WIDTH-21;
             return SceneEvent_Repaint();
 
         case InputEventKind_Button:
@@ -26,19 +26,18 @@ static struct SceneEvent Car_onInput(struct Scene *scene, struct InputEvent even
     }
 }
 
-static void Car_paint(struct Scene *scene)
+static void Car_paint(struct CarScene *self)
 {
-    struct private *priv = scene->private;
-
     LCD_clearDisplay(0);
-    LCD_setRect(priv->pos, LCD_HEIGHT/2-10, priv->pos+20,LCD_HEIGHT/2+10,0,1);
+    LCD_setRect(self->pos, LCD_HEIGHT/2-10, self->pos+20,LCD_HEIGHT/2+10,0,1);
     LCD_updateDisplay();
 }
 
 struct Scene *NewScene_Car(void) {
-    struct Scene *scene = SCENE_SAFE_ALLOC(struct private);
+    struct CarScene *scene = calloc(1, sizeof(struct CarScene));
+    if (!scene) return NULL;
 
-    scene->onInput = Car_onInput;
-    scene->paint = Car_paint;
-    return scene;
+    scene->base.onInput = (Scene_onInput_t) Car_onInput;
+    scene->base.paint = (Scene_paint_t) Car_paint;
+    return (struct Scene *) scene;
 }

main/liquid/scene_root.c

@@ -5,17 +5,21 @@
 #include <malloc.h>
 #include <stdio.h>
 
-struct private {
+/**
+ * The struct is allocated bigger than 'Scene' to accommodate private fields.
+ * Since the base struct is located at the beginning, it can be cast and passed around as Scene.
+ */
+struct RootScene {
+    struct Scene base;
     int32_t pos;
     uint32_t timer_phase;
     uint32_t timer_prescale;
 };
 
-static struct SceneEvent Root_onInput(struct Scene *scene, struct InputEvent event) {
-    struct private *priv = scene->private;
+static struct SceneEvent Root_onInput(struct RootScene *self, struct InputEvent event) {
     switch (event.kind) {
         case InputEventKind_Wheel:
-            priv->pos += event.wheel.delta;
+            self->pos += event.wheel.delta;
             break;
 
         case InputEventKind_Button:
@@ -28,14 +32,13 @@ static struct SceneEvent Root_onInput(struct Scene *scene, struct InputEvent eve
     return SceneEvent_Repaint();
 }
 
-static struct SceneEvent Root_onTick(struct Scene *scene) {
-    struct private *priv = scene->private;
-    priv->timer_prescale += 1;
+static struct SceneEvent Root_onTick(struct RootScene *self) {
+    self->timer_prescale += 1;
 
-    if (priv->timer_prescale == 100) {
-        priv->timer_prescale = 0;
-        priv->timer_phase += 1;
-        priv->timer_phase = priv->timer_phase & 0b11; // 0..3
+    if (self->timer_prescale == 100) {
+        self->timer_prescale = 0;
+        self->timer_phase += 1;
+        self->timer_phase = self->timer_phase & 0b11; // 0..3
 
         return SceneEvent_Repaint();
     }
@@ -43,13 +46,11 @@ static struct SceneEvent Root_onTick(struct Scene *scene) {
     return SceneEvent_None();
 }
 
-static void Root_paint(struct Scene *scene)
+static void Root_paint(struct RootScene *self)
 {
-    struct private *priv = scene->private;
-
     LCD_clearDisplay(0);
     const char *header = "???";
-    switch (priv->timer_phase) {
+    switch (self->timer_phase) {
         case 0:
             header = "Drink water";
             break;
@@ -78,10 +79,10 @@ static void Root_paint(struct Scene *scene)
 }
 
 struct Scene *NewScene_Root(void) {
-    struct Scene *scene = SCENE_SAFE_ALLOC(struct private);
+    struct RootScene *scene = calloc(1, sizeof(struct RootScene));
 
-    scene->onInput = Root_onInput;
-    scene->paint = Root_paint;
-    scene->onTick = Root_onTick;
-    return scene;
+    scene->base.onInput = (Scene_onInput_t) Root_onInput;
+    scene->base.paint = (Scene_paint_t) Root_paint;
+    scene->base.onTick = (Scene_onTick_t) Root_onTick;
+    return (struct Scene *) scene;
 }