Implement event subscription

The way C modules are initialized was affected by one of the major
breaking changes between 5.x versions.
Rather than trying to cater to each version individually, we now
just backport from newer versions. Namely from 5.2 to 5.1.

Makefile

@@ -43,6 +43,8 @@ OBJS = $(shell find src -type f -iname '*.c' | sed 's/\(.*\)\.c$$/$(BUILD_DIR)\/
 
 TARGET = $(BUILD_DIR)/$(PROJECT).so
 
+LUA_CPATH = $(shell echo "$${PWD}/$(BUILD_DIR)/?.so;$${LUA_CPATH}")
+
 ifdef CI
 CHECK_ARGS ?= --formatter TAP
 TEST_ARGS ?= --output=TAP
@@ -50,6 +52,11 @@ TEST_ARGS ?= --output=TAP
 CCFLAGS += -Werror
 endif
 
+bold := $(shell tput bold)
+orange := $(shell tput setaf 7)
+title := $(bold)$(orange)
+reset := $(shell tput sgr0)
+
 .PHONY: all clean doc doc-content doc-styles install uninstall test check rock
 
 all: build doc
@@ -58,23 +65,23 @@ build: $(TARGET)
 
 $(BUILD_DIR)/%.o: %.c
 	@mkdir -p $(shell dirname "$@")
-	@echo "\033[1;97m$(CC) $< -o $@\033[0m"
+	@echo "$(title)$(CC) $< -o $@$(reset)"
 	@$(CC) -c $(CCFLAGS) $< -o $@
 
 $(TARGET): $(OBJS)
-	@echo "\033[1;97m$(CC) -o $@\033[0m"
+	@echo "$(title)$(CC) -o $@$(reset)"
 	@$(CC) $(LIBFLAG) -o $@ $(OBJS) $(LIBS)
 
 $(BUILD_DIR)/doc/index.html:
 	@mkdir -p "$(BUILD_DIR)/doc" "$(BUILD_DIR)/src"
-	@echo "\033[1;97mPreprocess sources\033[0m"
+	@echo "$(title)Preprocess sources$(reset)"
 	sh tools/process_docs.sh "$(BUILD_DIR)"
-	@echo "\033[1;97mGenerate documentation\033[0m"
+	@echo "$(title)Generate documentation$(reset)"
 	ldoc --config=doc/config.ld --dir "$(BUILD_DIR)/doc" --project $(PROJECT) "$(BUILD_DIR)/src"
 
 $(BUILD_DIR)/doc/ldoc.css: doc/ldoc.scss
 	@mkdir -p "$(BUILD_DIR)/doc"
-	@echo "\033[1;97mGenerate stylesheet\033[0m"
+	@echo "$(title)Generate stylesheet$(reset)"
 	sass doc/ldoc.scss $(BUILD_DIR)/doc/ldoc.css
 
 doc-styles: $(BUILD_DIR)/doc/ldoc.css
@@ -87,10 +94,10 @@ clean:
 	rm -r out/
 
 install: build doc
-	@echo "\033[1;97mInstall C library\033[0m"
+	@echo "$(title)Install C library\033[0m"
 	install -vDm 644 -t $(INSTALL_LIBDIR) $(TARGET)
 
-	@echo "\033[1;97mInstall documentation\033[0m"
+	@echo "$(title)Install documentation\033[0m"
 	install -vd $(INSTALL_DOCDIR)
 	cp -vr $(BUILD_DIR)/doc/* $(INSTALL_DOCDIR)
 
@@ -101,8 +108,11 @@ uninstall:
 check:
 	@echo "Nothing to do"
 
-test:
+spec: build
 	busted --config-file=.busted.lua --lua=$(LUA) $(TEST_ARGS)
 
+test: build
+	$(LUA) test.lua
+
 rock:
 	luarocks --local --lua-version $(LUA_VERSION) make rocks/lua-libpulse-glib-scm-1.rockspec

src/lua_libpulse_glib/callback.h

@@ -1,6 +1,4 @@
-#ifndef callback_h_INCLUDED
+#pragma once
-#define callback_h_INCLUDED
-
 
 #include <lua.h>
 #include <pulse/context.h>
@@ -40,6 +38,3 @@ void free_lua_callback(simple_callback_data*);
 
 // Simple implementation of `pa_context_success_cb_t` that calls a provided Lua function.
 void success_callback(pa_context*, int, void*);
-
-#endif // callback_h_INCLUDED
-

src/lua_libpulse_glib/context.c

@@ -1,7 +1,7 @@
 #include "context.h"
 
-#include "pulseaudio.h"
 #include "lua_util.h"
+#include "pulseaudio.h"
 
 #include <pulse/context.h>
 #include <pulse/error.h>
@@ -33,19 +33,30 @@ void context_event_callback(pa_context* c, pa_subscription_event_type_t event_ty
     simple_callback_data* data = (simple_callback_data*) userdata;
     lua_State* L = data->L;
 
-    luaL_checktype(L, 1, LUA_TFUNCTION);
+    luaL_checktype(L, 1, LUA_TTABLE);
     luaL_checkudata(L, 2, LUA_PA_CONTEXT);
 
-    // `lua_call` will pop the function and arguments from the stack, but this callback will likely be called
+    // Iterate over the list of subscription callbacks and call each one
-    // multiple times.
+    lua_pushnil(L);
-    // To preseve the values for future calls, we need to duplicate them.
+    while (lua_next(L, 1) != 0) {
-    lua_pushvalue(L, 1);
+        // TODO: Once we do have the "nothing here" value, we need to check for that here.
+
+        // Copy the `self` parameter
         lua_pushvalue(L, 2);
         lua_pushinteger(L, event_type);
         lua_pushinteger(L, index);
 
         lua_call(L, 3, 0);
     }
+}
+
+
+void context_subscribe_success_callback(pa_context* _c, int success, void* userdata) {
+    simple_callback_data* data = (simple_callback_data*) userdata;
+    if (!success) {
+        luaL_error(data->L, "Failed to subscribe to events");
+    }
+}
 
 
 int context_new(lua_State* L, pa_mainloop_api* pa_api) {
@@ -62,6 +73,7 @@ int context_new(lua_State* L, pa_mainloop_api* pa_api) {
     }
     lgi_ctx->context = ctx;
     lgi_ctx->connected = FALSE;
+    lgi_ctx->subscribed = FALSE;
     lgi_ctx->state_callback_data = prepare_lua_callback(L, 0);
     lgi_ctx->event_callback_data = prepare_lua_callback(L, 0);
 
@@ -71,6 +83,10 @@ int context_new(lua_State* L, pa_mainloop_api* pa_api) {
     luaL_getmetatable(L, LUA_PA_CONTEXT);
     lua_setmetatable(L, -2);
 
+    // Copy the `context` value to the event callback
+    lua_pushvalue(L, -1);
+    lua_xmove(L, lgi_ctx->event_callback_data->L, 1);
+
     return 1;
 }
 
@@ -197,8 +213,19 @@ int context_subscribe(lua_State* L) {
     lua_pa_context* ctx = luaL_checkudata(L, 1, LUA_PA_CONTEXT);
     luaL_checktype(L, 2, LUA_TFUNCTION);
 
+    // This call is only effective when the connection state is "ready".
+    // So we have to do it here, rather than during `context_connect`,
+    // but we also need to make sure it's only called once.
+    if (!ctx->subscribed) {
+        pa_context_subscribe(ctx->context,
+                             PA_SUBSCRIPTION_MASK_ALL,
+                             context_subscribe_success_callback,
+                             ctx->event_callback_data);
+    }
+
     size_t pos = lua_rawlen(ctx->event_callback_data->L, 1) + 1;
     // Duplicate the callback function, so we can move it over to the other thread
+    // TODO: Do we actually need to duplicate?
     lua_pushvalue(L, 2);
     lua_xmove(L, ctx->event_callback_data->L, 1);
     lua_rawseti(ctx->event_callback_data->L, 1, pos);
@@ -218,6 +245,14 @@ int context_unsubscribe(lua_State* L) {
         return 0;
     }
 
+    // TODO: Handle calling this twice on the same index.
+    // Given that we use an array to track things, and Lua's way of counting in arrays
+    // doesn't like `nil`s, we probably need a special "nothing here" value that's not `nil`,
+    // and signifies an index that has already been unsubscribed.
+
+    // TODO: Simplify things. Supporting just the index is enough.
+    // Comparing by function is convenient, but also confusing to inexperienced devs.
+
     switch (lua_type(L, 2)) {
     case LUA_TNUMBER: {
         pos = lua_tointeger(L, 2);
@@ -257,6 +292,8 @@ int context_unsubscribe(lua_State* L) {
     }
     }
 
+    // TODO: As explained above, we need to handle calling unsubscribe twice better.
+    // Indices should not be re-used but replaced by a special "nothing here" value.
     for (; pos < len; ++pos) {
         lua_rawgeti(thread_L, 1, pos + 1);
         lua_rawseti(thread_L, 1, pos);

src/lua_libpulse_glib/context.h

@@ -24,6 +24,7 @@
 typedef struct lua_pa_context {
     pa_context* context;
     bool connected;
+    bool subscribed;
     simple_callback_data* state_callback_data;
     simple_callback_data* event_callback_data;
 } lua_pa_context;
@@ -95,6 +96,9 @@ int context_get_state(lua_State*);
  * Any number of callbacks may be registered at the same time, and can be unscubscribed with
  * @{Context:unsubscribe}, using the returned subscription ID.
  *
+ * This must called after the connection state changed to "ready" (index `4`). Otherwise
+ * it will have no effect.
+ *
  * @function Context:subscribe
  * @tparam function cb
  * @treturn number The subscription ID.

src/lua_libpulse_glib/lua_util.h

@@ -1,5 +1,4 @@
-#ifndef lua_util_h_INCLUDED
+#pragma once
-#define lua_util_h_INCLUDED
 
 #include <lauxlib.h>
 #include <lua.h>
@@ -8,6 +7,9 @@
 
 #if LUA_VERSION_NUM <= 501
 #define lua_rawlen lua_objlen
+
+#define luaL_newlib(L, l)      (luaL_newlibtable(L, l), luaL_setfuncs(L, l, 0))
+#define luaL_newlibtable(L, l) (lua_createtable(L, 0, sizeof(l) / sizeof(l[0])))
 #endif
 
 #if LUA_VERSION_NUM > 501
@@ -18,6 +20,3 @@ typedef struct luaU_enumfield {
     const char* name;
     const char* value;
 } luaU_enumfield;
-
-
-#endif // lua_util_h_INCLUDED

src/lua_libpulse_glib/pulseaudio.c

@@ -10,10 +10,8 @@
 #include <lualib.h>
 #include <pulse/glib-mainloop.h>
 
-
 #if LUA_VERSION_NUM <= 501
 // Shamelessly copied from Lua 5.3 source.
-// TODO: What's the official way to do this in 5.1?
 void luaL_setfuncs(lua_State* L, const luaL_Reg* l, int nup) {
     luaL_checkstack(L, nup, "too many upvalues");
     for (; l->name != NULL; l++) { /* fill the table with given functions */
@@ -29,12 +27,8 @@ void luaL_setfuncs(lua_State* L, const luaL_Reg* l, int nup) {
     }
     lua_pop(L, nup); /* remove upvalues */
 }
-
-
-#define luaL_newlib(L, l) (luaL_register(L, LUA_PULSEAUDIO, l))
 #endif
 
-
 int pulseaudio_new(lua_State* L) {
     GMainContext* ctx = g_main_context_default();
     if (ctx == NULL) {
@@ -86,17 +80,13 @@ int pulseaudio_new_context(lua_State* L) {
 void createlib_volume(lua_State* L) {
     luaL_newmetatable(L, LUA_PA_VOLUME);
 
-    lua_createtable(L, 0, sizeof volume_f / sizeof volume_f[0]);
+    luaL_newlib(L, volume_f);
-    luaL_setfuncs(L, volume_f, 0);
     lua_setfield(L, -2, "__index");
 
     luaL_setfuncs(L, volume_mt, 0);
 
-#if LUA_VERSION_NUM <= 501
-    luaL_register(L, LUA_PA_VOLUME, volume_lib);
-#else
     luaL_newlib(L, volume_lib);
-#endif
+
     lua_setmetatable(L, -2);
 }
 
@@ -104,17 +94,12 @@ void createlib_volume(lua_State* L) {
 void createlib_proplist(lua_State* L) {
     luaL_newmetatable(L, LUA_PA_PROPLIST);
 
-    lua_createtable(L, 0, sizeof proplist_f / sizeof proplist_f[0]);
+    luaL_newlib(L, proplist_f);
-    luaL_setfuncs(L, proplist_f, 0);
     lua_setfield(L, -2, "__index");
 
     luaL_setfuncs(L, proplist_mt, 0);
 
-#if LUA_VERSION_NUM <= 501
-    luaL_register(L, LUA_PA_PROPLIST, proplist_lib);
-#else
     luaL_newlib(L, proplist_lib);
-#endif
 
     // Create a metatable with an `__index` table for read-only enum fields.
     lua_createtable(L, 0, 1);
@@ -133,8 +118,7 @@ void createlib_proplist(lua_State* L) {
 void createlib_context(lua_State* L) {
     luaL_newmetatable(L, LUA_PA_CONTEXT);
 
-    lua_createtable(L, 0, sizeof context_f / sizeof context_f[0]);
+    luaL_newlib(L, context_f);
-    luaL_setfuncs(L, context_f, 0);
     lua_setfield(L, -2, "__index");
 
     luaL_setfuncs(L, context_mt, 0);
@@ -144,17 +128,12 @@ void createlib_context(lua_State* L) {
 void createlib_pulseaudio(lua_State* L) {
     luaL_newmetatable(L, LUA_PULSEAUDIO);
 
-    lua_createtable(L, 0, sizeof pulseaudio_f / sizeof pulseaudio_f[0]);
+    luaL_newlib(L, pulseaudio_f);
-    luaL_setfuncs(L, pulseaudio_f, 0);
     lua_setfield(L, -2, "__index");
 
     luaL_setfuncs(L, pulseaudio_mt, 0);
 
-#if LUA_VERSION_NUM <= 501
-    luaL_register(L, LUA_PULSEAUDIO, pulseaudio_lib);
-#else
     luaL_newlib(L, pulseaudio_lib);
-#endif
 }
 
 
@@ -177,17 +156,11 @@ LUA_MOD_EXPORT int luaopen_lua_libpulse_glib(lua_State* L) {
 LUA_MOD_EXPORT int luaopen_lua_libpulse_glib_volume(lua_State* L) {
     luaL_newmetatable(L, LUA_PA_VOLUME);
 
-    lua_createtable(L, 0, sizeof volume_f / sizeof volume_f[0]);
+    luaL_newlib(L, volume_f);
-    luaL_setfuncs(L, volume_f, 0);
     lua_setfield(L, -2, "__index");
 
     luaL_setfuncs(L, volume_mt, 0);
 
-#if LUA_VERSION_NUM <= 501
-    luaL_register(L, LUA_PA_VOLUME, volume_lib);
-#else
     luaL_newlib(L, volume_lib);
-#endif
     return 1;
 }
-