events: Rework event subsystem (add_event, send_event) to be more versatile.

add_event_ex is added that takes an extra user_data pointer. This pointer is
passed to the callback (add_event and add_event_ex have slightly different
callbacks types). All callbacks also get the event id passed. Events added
with add_event_ex must be removed with remove_event_ex because the user_data
pointer must match in addition to the callback pointer.

On the other add_event is simplified to omit the oneshort parameter which
was almost always false (still there with add_event_ex).

As a side effect the ata_idle_notify callbacks are changed as well, they
do not take a data parameter anymore which was always NULL anyway.

This commit also adds some documentation to events.h

Change-Id: I13e29a0f88ef908f175b376d83550f9e0231f772

5 files changed, 128 insertions, 31 deletions

diff --git a/firmware/ata_idle_notify.c b/firmware/ata_idle_notify.c
index 35d192b..ee9f3c0 100644
--- a/firmware/ata_idle_notify.c
+++ b/firmware/ata_idle_notify.c
@@ -25,12 +25,20 @@
 #include "kernel.h"
 #include "string.h"
 
-void register_storage_idle_func(void (*function)(void *data))
+static void wrapper(unsigned short id, void *ev_data, void *user_data)
+{
+    (void)id;
+    (void)ev_data;
+    void (*func)(void) = user_data;
+    func();
+}
+
+void register_storage_idle_func(void (*function)(void))
 {
 #if USING_STORAGE_CALLBACK
-    add_event(DISK_EVENT_SPINUP, true, function);
+    add_event_ex(DISK_EVENT_SPINUP, true, wrapper, function);
 #else
-    function(NULL); /* just call the function now */
+    function(); /* just call the function now */
 /* this _may_ cause problems later if the calling function
    sets a variable expecting the callback to unset it, because
    the callback will be run before this function exits, so before the var is set */
@@ -38,12 +46,12 @@ void register_storage_idle_func(void (*function)(void *data))
 }
 
 #if USING_STORAGE_CALLBACK
-void unregister_storage_idle_func(void (*func)(void *data), bool run)
+void unregister_storage_idle_func(void (*func)(void), bool run)
 {
-    remove_event(DISK_EVENT_SPINUP, func);
+    remove_event_ex(DISK_EVENT_SPINUP, wrapper, func);
     
     if (run)
-        func(NULL);
+        func();
 }
 
 bool call_storage_idle_notifys(bool force)
diff --git a/firmware/events.c b/firmware/events.c
index 74172e1..4a51e7a 100644
--- a/firmware/events.c
+++ b/firmware/events.c
@@ -28,30 +28,41 @@
 struct sysevent {
     unsigned short id;
     bool oneshot;
-    void (*callback)(void *data);
+    bool has_user_data;
+    union {
+        void (*callback)(unsigned short id, void *event_data);
+        struct {
+            void (*callback2)(unsigned short id, void *event_data, void *user_data);
+            void *user_data;
+        };
+    } handler;
 };
 
 static struct sysevent events[MAX_SYS_EVENTS];
 
-bool add_event(unsigned short id, bool oneshot, void (*handler)(void *data))
+static bool do_add_event(unsigned short id, bool oneshot, bool user_data_valid,
+                         void *handler, void *user_data)
 {
     int i;
     
     /* Check if the event already exists. */
     for (i = 0; i < MAX_SYS_EVENTS; i++)
     {
-        if (events[i].callback == handler && events[i].id == id)
+        if (events[i].handler.callback == handler && events[i].id == id
+                && (!user_data_valid || (user_data == events[i].handler.callback)))
             return false;
     }
     
     /* Try to find a free slot. */
     for (i = 0; i < MAX_SYS_EVENTS; i++)
     {
-        if (events[i].callback == NULL)
+        if (events[i].handler.callback == NULL)
         {
             events[i].id = id;
             events[i].oneshot = oneshot;
-            events[i].callback = handler;
+            if ((events[i].has_user_data = user_data_valid))
+                events[i].handler.user_data = user_data;
+            events[i].handler.callback = handler;
             return true;
         }
     }
@@ -60,33 +71,59 @@ bool add_event(unsigned short id, bool oneshot, void (*handler)(void *data))
     return false;
 }
 
-void remove_event(unsigned short id, void (*handler)(void *data))
+bool add_event(unsigned short id, void (*handler)(unsigned short id, void *data))
+{
+    return do_add_event(id, false, false, handler, NULL);
+}
+
+bool add_event_ex(unsigned short id, bool oneshot, void (*handler)(unsigned short id, void *event_data, void *user_data), void *user_data)
+{
+    return do_add_event(id, oneshot, true, handler, user_data);
+}
+
+void do_remove_event(unsigned short id, bool user_data_valid,
+                  void *handler, void *user_data)
 {
     int i;
     
     for (i = 0; i < MAX_SYS_EVENTS; i++)
     {
-        if (events[i].id == id && events[i].callback == handler)
+        if (events[i].id == id && events[i].handler.callback == handler
+                && (!user_data_valid || (user_data == events[i].handler.callback)))
         {
-            events[i].callback = NULL;
+            events[i].handler.callback = NULL;
             return;
         }
     }
 }
 
+void remove_event(unsigned short id, void (*handler)(unsigned short id, void *data))
+{
+    do_remove_event(id, false, handler, NULL);
+}
+
+void remove_event_ex(unsigned short id,
+                     void (*handler)(unsigned short id, void *event_data, void *user_data),
+                     void *user_data)
+{
+    do_remove_event(id, true, handler, user_data);
+}
+
 void send_event(unsigned short id, void *data)
 {
     int i;
     
     for (i = 0; i < MAX_SYS_EVENTS; i++)
     {
-        if (events[i].id == id && events[i].callback != NULL)
+        if (events[i].id == id && events[i].handler.callback != NULL)
         {
-            events[i].callback(data);
+            if (events[i].has_user_data)
+                events[i].handler.callback2(id, data, events[i].handler.user_data);
+            else
+                events[i].handler.callback(id, data);
             
             if (events[i].oneshot)
-                events[i].callback = NULL;
+                events[i].handler.callback = NULL;
         }
     }
 }
-
diff --git a/firmware/export/ata_idle_notify.h b/firmware/export/ata_idle_notify.h
index 93a53ee..0443f8e 100644
--- a/firmware/export/ata_idle_notify.h
+++ b/firmware/export/ata_idle_notify.h
@@ -48,9 +48,9 @@ enum {
  */
 #define USING_STORAGE_CALLBACK  !defined(BOOTLOADER) && !defined(APPLICATION) && !defined(__PCTOOL__)
 
-extern void register_storage_idle_func(void (*function)(void *data));
+extern void register_storage_idle_func(void (*function)(void));
 #if USING_STORAGE_CALLBACK
-extern void unregister_storage_idle_func(void (*function)(void *data), bool run);
+extern void unregister_storage_idle_func(void (*function)(void), bool run);
 extern bool call_storage_idle_notifys(bool force);
 #else
 #define unregister_storage_idle_func(f,r)
diff --git a/firmware/export/events.h b/firmware/export/events.h
index 859901c..fd7f9df 100644
--- a/firmware/export/events.h
+++ b/firmware/export/events.h
@@ -23,12 +23,24 @@
 #define _EVENTS_H
 
 #include <stdbool.h>
-
-/** Only CLASS defines and firmware/ level events should be defined here.
- *  apps/ level events are defined in apps/appevents.h 
- */
-
 /**
+ * Synchronouos event system.
+ *
+ * Callbacks are subscribed with add_event() or add_event_ex(). events
+ * are fired using send_event().
+ *
+ * Events are always dispatched synchronously: the callbacks are called
+ * in the thread context of the event sender, without context switch. This
+ * also means that callbacks should be as simple as possible to avoid
+ * blocking the sender and other callbacks
+ *
+ * Use the kernel-level event_queue for cross-thread event dispatching.
+ * */
+
+/*
+ * Only CLASS defines and firmware/ level events should be defined here.
+ * apps/ level events are defined in apps/appevents.h
+ * 
  * High byte = Event class definition
  * Low byte  = Event ID
  */
@@ -40,9 +52,50 @@
 #define EVENT_CLASS_RECORDING  0x1000
 #define EVENT_CLASS_LCD        0x2000
 
-bool add_event(unsigned short id, bool oneshot, void (*handler)(void *data));
-void remove_event(unsigned short id, void (*handler)(void *data));
+/**
+ * Subscribe to an event with a simple callback. The callback will be called
+ * synchronously everytime the event fires, passing the event id and data to
+ * the callback.
+ *
+ * Must be removed with remove_event(). 
+ */
+bool add_event(unsigned short id, void (*handler)(unsigned short id, void *event_data));
+
+/**
+ * Subscribe to an event with a detailed callback. The callback will be called
+ * synchronously everytime the event fires, passing the event id and data, as
+ * well as the user_data pointer passed here, to the callback.
+ *
+ * With oneshot == true, the callback is unsubscribed automatically after
+ * the event fired for the first time. In this case the event need not to be
+ * removed with remove_event_ex().
+ *
+ * Must be removed with remove_event_ex(). remove_event() will never remove
+ * events added with this function. 
+ */
+bool add_event_ex(unsigned short id, bool oneshot, void (*handler)(unsigned short id, void *event_data, void *user_data), void *user_data);
+
+/**
+ * Unsubscribe a callback from an event. The handler pointer is matched.
+ *
+ * This will only work for subscriptions made with add_event().
+ */
+void remove_event(unsigned short id, void (*handler)(unsigned short id, void *data));
+
+/**
+ * Unsubscribe a callback from an event. The handler and user_data pointers
+ * are matched. That means the same user_data that was passed to add_event_ex()
+ * must be passed to this too.
+ *
+ * This will only work for subscriptions made with add_event_ex().
+ */
+void remove_event_ex(unsigned short id, void (*handler)(unsigned short id, void *event_data, void *user_data), void *user_data);
+
+/**
+ * Fire an event, which synchronously calls all subscribed callbacks. The
+ * event id and data pointer are passed to the callbacks as well, and
+ * optionally the user_data pointer from add_event_ex().
+ */
 void send_event(unsigned short id, void *data);
 
 #endif
-
diff --git a/firmware/logf.c b/firmware/logf.c
index a24a635..ade5458 100644
--- a/firmware/logf.c
+++ b/firmware/logf.c
@@ -322,7 +322,7 @@ static int logdiskf_push(void *userp, unsigned char c)
     return true;
 }
 
-static void flush_buffer(void* data);
+static void flush_buffer(void);
 
 void _logdiskf(const char* file, const char level, const char *fmt, ...)
 {
@@ -350,9 +350,8 @@ void _logdiskf(const char* file, const char level, const char *fmt, ...)
     register_storage_idle_func(flush_buffer);
 }
 
-static void flush_buffer(void* data)
+static void flush_buffer(void)
 {
-    (void)data;
     int fd;
     if(logdiskfindex < 1)
         return;