From 4f27d18bd32a6949d4db2ceb2a55430c8d247a85 Mon Sep 17 00:00:00 2001 From: George Kiagiadakis Date: Thu, 18 May 2023 16:19:49 +0300 Subject: [PATCH] log: rename "message" level to "notice" and print criticals with "E" Syslog calls this level "notice" and I prefer it because we use it to display significant messages that are not warnings, but they are not really "standard", as GLib wants them to be. There is nothing "standard" about log messages in general. Also, make these notice messages be enabled at debug level 2, together with warnings. The default log.level is 2 and it is a good idea to show notices by default too. Finally, show them in the log with "N" and also change criticals to be shown with "E", meaning "error"... Then promote G_LOG_LEVEL_ERROR messages to be shown with "F", meaning "fatal", because in fact these messages are always fatal and always call abort(). Still, keep the term "critical" in the functions to make sure that whoever uses them is aware that this level is only for critical conditions and not suitable to display any kind of error. --- docs/rst/lua_api/lua_log_api.rst | 4 ++-- lib/wp/device.c | 2 +- lib/wp/event-dispatcher.c | 2 +- lib/wp/log.c | 23 ++++++++++--------- lib/wp/log.h | 6 ++--- lib/wp/private/pipewire-object-mixin.c | 6 ++--- modules/module-lua-scripting/api/api.c | 10 ++++---- modules/module-mixer-api.c | 4 ++-- modules/module-reserve-device/plugin.c | 6 ++--- .../module-reserve-device/reserve-device.c | 2 +- modules/module-si-audio-adapter.c | 2 +- src/main.c | 4 ++-- src/scripts/monitors/alsa.lua | 4 ++-- src/scripts/monitors/bluez-midi.lua | 4 ++-- src/scripts/monitors/bluez.lua | 2 +- src/scripts/monitors/libcamera.lua | 2 +- src/scripts/monitors/v4l2.lua | 2 +- tests/common/base-test-fixture.h | 4 ++-- 18 files changed, 45 insertions(+), 44 deletions(-) diff --git a/docs/rst/lua_api/lua_log_api.rst b/docs/rst/lua_api/lua_log_api.rst index 32916039..9e53cc65 100644 --- a/docs/rst/lua_api/lua_log_api.rst +++ b/docs/rst/lua_api/lua_log_api.rst @@ -12,9 +12,9 @@ Debug Logging :type object: GObject or GBoxed :param string message: the warning message to log -.. function:: Log.message(object, message) +.. function:: Log.notice(object, message) - Logs a normal message, like :c:macro:`wp_message_object` + Logs a notice message, like :c:macro:`wp_notice_object` :param object: optional object to associate the message with; you may skip this and just start with the *message* as the first parameter diff --git a/lib/wp/device.c b/lib/wp/device.c index 7d230786..5ba98678 100644 --- a/lib/wp/device.c +++ b/lib/wp/device.c @@ -616,7 +616,7 @@ wp_spa_device_new_from_spa_factory (WpCore * core, handle = pw_context_load_spa_handle (pw_context, factory_name, props ? wp_properties_peek_dict (props) : NULL); if (!handle) { - wp_message ("SPA handle '%s' could not be loaded; is it installed?", + wp_notice ("SPA handle '%s' could not be loaded; is it installed?", factory_name); return NULL; } diff --git a/lib/wp/event-dispatcher.c b/lib/wp/event-dispatcher.c index 20187f37..c704563d 100644 --- a/lib/wp/event-dispatcher.c +++ b/lib/wp/event-dispatcher.c @@ -84,7 +84,7 @@ on_event_hook_done (WpEventHook * hook, GAsyncResult * res, EventData * data) if (!wp_event_hook_finish (hook, res, &error) && error && error->domain != G_IO_ERROR && error->code != G_IO_ERROR_CANCELLED) - wp_message_object (hook, "failed: %s", error->message); + wp_notice_object (hook, "failed: %s", error->message); g_clear_object (&data->current_hook_in_async); spa_system_eventfd_write (dispatcher->system, dispatcher->eventfd, 1); diff --git a/lib/wp/log.c b/lib/wp/log.c index 82e44815..392e81e7 100644 --- a/lib/wp/log.c +++ b/lib/wp/log.c @@ -77,20 +77,20 @@ WP_DEFINE_LOCAL_LOG_TOPIC ("wp-log") * \param ... A format string, followed by format arguments in printf() style */ /*! - * \def wp_message(...) - * \brief Logs a standard message to the standard log via GLib's logging system. + * \def wp_notice(...) + * \brief Logs a notice message to the standard log via GLib's logging system. * \param ... A format string, followed by format arguments in printf() style */ /*! - * \def wp_message_object(object, ...) - * \brief Logs a standard message to the standard log via GLib's logging system. + * \def wp_notice_object(object, ...) + * \brief Logs a notice message to the standard log via GLib's logging system. * \param object A GObject associated with the log; this is printed in a special * way to make it easier to track messages from a specific object * \param ... A format string, followed by format arguments in printf() style */ /*! - * \def wp_message_boxed(type, object, ...) - * \brief Logs a standard message to the standard log via GLib's logging system. + * \def wp_notice_boxed(type, object, ...) + * \brief Logs a notice message to the standard log via GLib's logging system. * \param type The type of \a object * \param object A boxed object associated with the log; this is printed in a * special way to make it easier to track messages from a specific object. @@ -235,10 +235,10 @@ static const struct { gchar color[8]; } log_level_info[] = { { 0, 0, "U", "0", COLOR_BRIGHT_MAGENTA }, - { G_LOG_LEVEL_ERROR, 0, "E", "3" /* LOG_ERR */, COLOR_BRIGHT_RED }, - { G_LOG_LEVEL_CRITICAL,SPA_LOG_LEVEL_ERROR,"C", "4" /* LOG_WARNING */, COLOR_RED }, + { G_LOG_LEVEL_ERROR, 0, "F", "3" /* LOG_ERR */, COLOR_BRIGHT_RED }, + { G_LOG_LEVEL_CRITICAL,SPA_LOG_LEVEL_ERROR,"E", "4" /* LOG_WARNING */, COLOR_RED }, { G_LOG_LEVEL_WARNING, SPA_LOG_LEVEL_WARN, "W", "4" /* LOG_WARNING */, COLOR_BRIGHT_YELLOW }, - { G_LOG_LEVEL_MESSAGE, SPA_LOG_LEVEL_WARN, "M", "5" /* LOG_NOTICE */, COLOR_BRIGHT_GREEN }, + { G_LOG_LEVEL_MESSAGE, SPA_LOG_LEVEL_WARN, "N", "5" /* LOG_NOTICE */, COLOR_BRIGHT_GREEN }, { G_LOG_LEVEL_INFO, SPA_LOG_LEVEL_INFO, "I", "6" /* LOG_INFO */, COLOR_GREEN }, { G_LOG_LEVEL_DEBUG, SPA_LOG_LEVEL_DEBUG,"D", "7" /* LOG_DEBUG */, COLOR_BRIGHT_CYAN }, { WP_LOG_LEVEL_TRACE, SPA_LOG_LEVEL_TRACE,"T", "7" /* LOG_DEBUG */, COLOR_CYAN }, @@ -276,13 +276,14 @@ level_index_to_full_flags (gint lvl_index) } /* map a SPA_LOG_LEVEL_* to an index in the log_level_info table; - index 4 (G_LOG_LEVEL_MESSAGE) can never be returned */ + note that SPA_LOG_LEVEL_WARN maps to 4 (G_LOG_LEVEL_MESSAGE) and + index 3 (G_LOG_LEVEL_WARNING) can never be returned */ static G_GNUC_CONST inline gint level_index_from_spa (gint spa_lvl) { if (G_UNLIKELY (spa_lvl <= SPA_LOG_LEVEL_NONE)) return 0; - else if (spa_lvl < SPA_LOG_LEVEL_INFO) + else if (spa_lvl < SPA_LOG_LEVEL_WARN) return spa_lvl + 1; else if (G_UNLIKELY (spa_lvl > SPA_LOG_LEVEL_TRACE)) return (gint) G_N_ELEMENTS (log_level_info) - 1; diff --git a/lib/wp/log.h b/lib/wp/log.h index 18343e2b..e6cc13d9 100644 --- a/lib/wp/log.h +++ b/lib/wp/log.h @@ -114,7 +114,7 @@ void wp_log_checked (const gchar *log_topic, GLogLevelFlags log_level, wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_CRITICAL, 0, NULL, __VA_ARGS__) #define wp_warning(...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_WARNING, 0, NULL, __VA_ARGS__) -#define wp_message(...) \ +#define wp_notice(...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_MESSAGE, 0, NULL, __VA_ARGS__) #define wp_info(...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_INFO, 0, NULL, __VA_ARGS__) @@ -127,7 +127,7 @@ void wp_log_checked (const gchar *log_topic, GLogLevelFlags log_level, wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_CRITICAL, (object) ? G_TYPE_FROM_INSTANCE (object) : G_TYPE_NONE, object, __VA_ARGS__) #define wp_warning_object(object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_WARNING, (object) ? G_TYPE_FROM_INSTANCE (object) : G_TYPE_NONE, object, __VA_ARGS__) -#define wp_message_object(object, ...) \ +#define wp_notice_object(object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_MESSAGE, (object) ? G_TYPE_FROM_INSTANCE (object) : G_TYPE_NONE, object, __VA_ARGS__) #define wp_info_object(object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_INFO, (object) ? G_TYPE_FROM_INSTANCE (object) : G_TYPE_NONE, object, __VA_ARGS__) @@ -140,7 +140,7 @@ void wp_log_checked (const gchar *log_topic, GLogLevelFlags log_level, wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_CRITICAL, type, object, __VA_ARGS__) #define wp_warning_boxed(type, object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_WARNING, type, object, __VA_ARGS__) -#define wp_message_boxed(type, object, ...) \ +#define wp_notice_boxed(type, object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_MESSAGE, type, object, __VA_ARGS__) #define wp_info_boxed(type, object, ...) \ wp_log (WP_LOCAL_LOG_TOPIC, G_LOG_LEVEL_INFO, type, object, __VA_ARGS__) diff --git a/lib/wp/private/pipewire-object-mixin.c b/lib/wp/private/pipewire-object-mixin.c index 7d51542c..e0c89720 100644 --- a/lib/wp/private/pipewire-object-mixin.c +++ b/lib/wp/private/pipewire-object-mixin.c @@ -176,7 +176,7 @@ wp_pw_object_mixin_enum_params_unchecked (gpointer obj, /* return early if seq contains an error */ if (G_UNLIKELY (SPA_RESULT_IS_ERROR (seq))) { - wp_message_object (obj, "enum_params failed: %s", spa_strerror (seq)); + wp_notice_object (obj, "enum_params failed: %s", spa_strerror (seq)); g_task_report_new_error (obj, callback, user_data, NULL, WP_DOMAIN_LIBRARY, WP_LIBRARY_ERROR_OPERATION_FAILED, "enum_params failed: %s", spa_strerror (seq)); @@ -300,7 +300,7 @@ wp_pw_object_mixin_set_param (WpPipewireObject * obj, const gchar * id, gint ret; if (!d->iface) { - wp_message_object (obj, "ignoring set_param on already destroyed objects"); + wp_notice_object (obj, "ignoring set_param on already destroyed objects"); return FALSE; } @@ -319,7 +319,7 @@ wp_pw_object_mixin_set_param (WpPipewireObject * obj, const gchar * id, ret = iface->set_param (obj, wp_spa_id_value_number (param_id), flags, param); if (G_UNLIKELY (SPA_RESULT_IS_ERROR (ret))) { - wp_message_object (obj, "set_param failed: %s", spa_strerror (ret)); + wp_notice_object (obj, "set_param failed: %s", spa_strerror (ret)); return FALSE; } return TRUE; diff --git a/modules/module-lua-scripting/api/api.c b/modules/module-lua-scripting/api/api.c index b7af32d6..34240ac6 100644 --- a/modules/module-lua-scripting/api/api.c +++ b/modules/module-lua-scripting/api/api.c @@ -357,7 +357,7 @@ static int log_warning (lua_State *L) { return log_log (L, G_LOG_LEVEL_WARNING); } static int -log_message (lua_State *L) { return log_log (L, G_LOG_LEVEL_MESSAGE); } +log_notice (lua_State *L) { return log_log (L, G_LOG_LEVEL_MESSAGE); } static int log_info (lua_State *L) { return log_log (L, G_LOG_LEVEL_INFO); } @@ -370,7 +370,7 @@ log_trace (lua_State *L) { return log_log (L, WP_LOG_LEVEL_TRACE); } static const luaL_Reg log_obj_funcs[] = { { "warning", log_warning }, - { "message", log_message }, + { "notice", log_notice }, { "info", log_info }, { "debug", log_debug }, { "trace", log_trace }, @@ -399,7 +399,7 @@ log_open_topic (lua_State *L) static const luaL_Reg log_funcs[] = { { "open_topic", log_open_topic }, { "warning", log_warning }, - { "message", log_message }, + { "notice", log_notice }, { "info", log_info }, { "debug", log_debug }, { "trace", log_trace }, @@ -435,7 +435,7 @@ object_activate_done (WpObject *o, GAsyncResult * res, GClosure * closure) int n_vals = 1; if (!wp_object_activate_finish (o, res, &error)) { - wp_message_object (o, "%s", error->message); + wp_notice_object (o, "%s", error->message); if (closure) { g_value_init (&val[1], G_TYPE_STRING); g_value_set_string (&val[1], error->message); @@ -1386,7 +1386,7 @@ si_adapter_set_ports_format_done (WpObject *o, GAsyncResult * res, int n_vals = 1; if (!wp_si_adapter_set_ports_format_finish (WP_SI_ADAPTER (o), res, &error)) { - wp_message_object (o, "%s", error->message); + wp_notice_object (o, "%s", error->message); if (closure) { g_value_init (&val[1], G_TYPE_STRING); g_value_set_string (&val[1], error->message); diff --git a/modules/module-mixer-api.c b/modules/module-mixer-api.c index eb3a16ab..f8e87fdf 100644 --- a/modules/module-mixer-api.c +++ b/modules/module-mixer-api.c @@ -446,7 +446,7 @@ wp_mixer_api_set_volume (WpMixerApi * self, guint32 id, GVariant * vvolume) channel = wp_spa_id_table_find_value_from_short_name ( t_audioChannel, channel_str); if (!channel) - wp_message_object (self, "invalid channel: %s", channel_str); + wp_notice_object (self, "invalid channel: %s", channel_str); } if (channel) { @@ -458,7 +458,7 @@ wp_mixer_api_set_volume (WpMixerApi * self, guint32 id, GVariant * vvolume) } if (index >= MIN(new_volume.channels, SPA_AUDIO_MAX_CHANNELS)) { - wp_message_object (self, "invalid channel index: %u", index); + wp_notice_object (self, "invalid channel index: %u", index); continue; } diff --git a/modules/module-reserve-device/plugin.c b/modules/module-reserve-device/plugin.c index a04369a0..469fe69f 100644 --- a/modules/module-reserve-device/plugin.c +++ b/modules/module-reserve-device/plugin.c @@ -143,7 +143,7 @@ wp_reserve_device_plugin_create_reservation (WpReserveDevicePlugin *self, { WpDBusState state = wp_dbus_get_state (self->dbus); if (state != WP_DBUS_STATE_CONNECTED) { - wp_message_object (self, "not connected to D-Bus"); + wp_notice_object (self, "not connected to D-Bus"); return NULL; } @@ -167,7 +167,7 @@ wp_reserve_device_plugin_destroy_reservation (WpReserveDevicePlugin *self, { WpDBusState state = wp_dbus_get_state (self->dbus); if (state != WP_DBUS_STATE_CONNECTED) { - wp_message_object (self, "not connected to D-Bus"); + wp_notice_object (self, "not connected to D-Bus"); return; } g_hash_table_remove (self->reserve_devices, name); @@ -179,7 +179,7 @@ wp_reserve_device_plugin_get_reservation (WpReserveDevicePlugin *self, { WpDBusState state = wp_dbus_get_state (self->dbus); if (state != WP_DBUS_STATE_CONNECTED) { - wp_message_object (self, "not connected to D-Bus"); + wp_notice_object (self, "not connected to D-Bus"); return NULL; } diff --git a/modules/module-reserve-device/reserve-device.c b/modules/module-reserve-device/reserve-device.c index fa5ca572..66913359 100644 --- a/modules/module-reserve-device/reserve-device.c +++ b/modules/module-reserve-device/reserve-device.c @@ -168,7 +168,7 @@ on_acquire_transition_done (GObject *rd, GAsyncResult *res, gpointer data) gboolean acquired = wp_reserve_device_acquire_transition_finish (res, &error); if (error) { - wp_message_object (self, "%s: Acquire error: %s", self->name, + wp_notice_object (self, "%s: Acquire error: %s", self->name, error->message); } diff --git a/modules/module-si-audio-adapter.c b/modules/module-si-audio-adapter.c index 284cff0e..fc81c86d 100644 --- a/modules/module-si-audio-adapter.c +++ b/modules/module-si-audio-adapter.c @@ -256,7 +256,7 @@ si_audio_adapter_configure (WpSessionItem * item, WpProperties *p) str = wp_properties_get (si_props, "item.features.no-format"); self->no_format = str && pw_properties_parse_bool (str); if (!self->no_format && !si_audio_adapter_find_format (self, node)) { - wp_message_object (item, "no usable format found for node %d", + wp_notice_object (item, "no usable format found for node %d", wp_proxy_get_bound_id (WP_PROXY (node))); return FALSE; } diff --git a/src/main.c b/src/main.c index 9d990b40..b458fe94 100644 --- a/src/main.c +++ b/src/main.c @@ -481,7 +481,7 @@ daemon_exit (WpDaemon * d, gint code) static void on_disconnected (WpCore *core, WpDaemon * d) { - wp_message ("disconnected from pipewire"); + wp_notice ("disconnected from pipewire"); daemon_exit (d, WP_EXIT_OK); } @@ -489,7 +489,7 @@ static gboolean signal_handler (int signal, gpointer data) { WpDaemon *d = data; - wp_message ("stopped by signal: %s", strsignal (signal)); + wp_notice ("stopped by signal: %s", strsignal (signal)); daemon_exit (d, WP_EXIT_OK); return G_SOURCE_CONTINUE; } diff --git a/src/scripts/monitors/alsa.lua b/src/scripts/monitors/alsa.lua index 3ddf6d91..73c5a138 100644 --- a/src/scripts/monitors/alsa.lua +++ b/src/scripts/monitors/alsa.lua @@ -340,7 +340,7 @@ end function createMonitor () local m = SpaDevice("api.alsa.enum.udev", config.properties) if m == nil then - Log.message("PipeWire's SPA ALSA udev plugin(\"api.alsa.enum.udev\")" + Log.notice("PipeWire's SPA ALSA udev plugin(\"api.alsa.enum.udev\")" .. "missing or broken. Sound Cards cannot be enumerated") return nil end @@ -391,7 +391,7 @@ end -- has failed and continue without it rd_plugin = Plugin.find("reserve-device") if rd_plugin and rd_plugin:call("get-dbus")["state"] ~= "connected" then - Log.message("reserve-device plugin is not connected to D-Bus, " + Log.notice("reserve-device plugin is not connected to D-Bus, " .. "disabling device reservation") rd_plugin = nil end diff --git a/src/scripts/monitors/bluez-midi.lua b/src/scripts/monitors/bluez-midi.lua index 46858975..cae236bc 100644 --- a/src/scripts/monitors/bluez-midi.lua +++ b/src/scripts/monitors/bluez-midi.lua @@ -92,7 +92,7 @@ function createMonitor() id_to_name_table[id] = nil end) else - Log.message("PipeWire's BlueZ MIDI SPA missing or broken. Bluetooth not supported.") + Log.notice("PipeWire's BlueZ MIDI SPA missing or broken. Bluetooth not supported.") return nil end @@ -127,7 +127,7 @@ function createServers() table.insert(servers, node) setLatencyOffset(node, latency_offset) else - Log.message("Failed to create BLE MIDI server.") + Log.notice("Failed to create BLE MIDI server.") end i = i + 1 end diff --git a/src/scripts/monitors/bluez.lua b/src/scripts/monitors/bluez.lua index cd1136c4..e0533c81 100644 --- a/src/scripts/monitors/bluez.lua +++ b/src/scripts/monitors/bluez.lua @@ -379,7 +379,7 @@ function createMonitor() if monitor then monitor:connect("create-object", createDevice) else - Log.message("PipeWire's BlueZ SPA missing or broken. Bluetooth not supported.") + Log.notice("PipeWire's BlueZ SPA missing or broken. Bluetooth not supported.") return nil end monitor:activate(Feature.SpaDevice.ENABLED) diff --git a/src/scripts/monitors/libcamera.lua b/src/scripts/monitors/libcamera.lua index bcc885fe..238a9248 100644 --- a/src/scripts/monitors/libcamera.lua +++ b/src/scripts/monitors/libcamera.lua @@ -148,5 +148,5 @@ if monitor then monitor:connect("create-object", createDevice) monitor:activate(Feature.SpaDevice.ENABLED) else - Log.message("PipeWire's libcamera SPA missing or broken. libcamera not supported.") + Log.notice("PipeWire's libcamera SPA missing or broken. libcamera not supported.") end diff --git a/src/scripts/monitors/v4l2.lua b/src/scripts/monitors/v4l2.lua index a416b980..4b4e2010 100644 --- a/src/scripts/monitors/v4l2.lua +++ b/src/scripts/monitors/v4l2.lua @@ -138,5 +138,5 @@ if monitor then monitor:connect("create-object", createDevice) monitor:activate(Feature.SpaDevice.ENABLED) else - Log.message("PipeWire's V4L SPA missing or broken. Video4Linux not supported.") + Log.notice("PipeWire's V4L SPA missing or broken. Video4Linux not supported.") end diff --git a/tests/common/base-test-fixture.h b/tests/common/base-test-fixture.h index afcaab1f..080206e4 100644 --- a/tests/common/base-test-fixture.h +++ b/tests/common/base-test-fixture.h @@ -43,7 +43,7 @@ typedef struct { static gboolean timeout_callback (WpBaseTestFixture * self) { - wp_message ("test timed out"); + wp_notice ("test timed out"); g_test_fail (); g_main_loop_quit (self->loop); @@ -53,7 +53,7 @@ timeout_callback (WpBaseTestFixture * self) static void disconnected_callback (WpCore *core, WpBaseTestFixture * self) { - wp_message_object (core, "%s core disconnected", + wp_notice_object (core, "%s core disconnected", (core == self->client_core) ? "client" : "sm"); g_test_fail (); g_main_loop_quit (self->loop);