From d75d4916af035e3710c46ca751632f2b0fb2466d Mon Sep 17 00:00:00 2001 From: Evangelos Ribeiro Tzaras Date: Wed, 26 Jan 2022 14:18:24 +0100 Subject: [PATCH] manager: Use state flags instead of an enum and adapt to changes This gives us some more granularity and is overall a better fit than the states previously used. Closes #327 --- src/calls-application.c | 6 ++-- src/calls-main-window.c | 32 ++++------------- src/calls-manager.c | 76 +++++++++++++++++++---------------------- src/calls-manager.h | 17 +++++---- tests/test-manager.c | 29 +++++++++------- 5 files changed, 70 insertions(+), 90 deletions(-) diff --git a/src/calls-application.c b/src/calls-application.c index 20239fe..1948f41 100644 --- a/src/calls-application.c +++ b/src/calls-application.c @@ -324,9 +324,11 @@ static void manager_state_changed_cb (GApplication *application) { GAction* dial_action = g_action_map_lookup_action (G_ACTION_MAP (application), "dial"); - CallsManagerState state = calls_manager_get_state (calls_manager_get_default ()); + CallsManagerFlags state_flags = calls_manager_get_state_flags (calls_manager_get_default ()); + gboolean enabled = (state_flags & CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM) || + (state_flags & CALLS_MANAGER_FLAGS_HAS_VOIP_ACCOUNT); - g_simple_action_set_enabled (G_SIMPLE_ACTION (dial_action), state == CALLS_MANAGER_STATE_READY); + g_simple_action_set_enabled (G_SIMPLE_ACTION (dial_action), enabled); } static const GActionEntry actions[] = diff --git a/src/calls-main-window.c b/src/calls-main-window.c index 7103bb9..4823397 100644 --- a/src/calls-main-window.c +++ b/src/calls-main-window.c @@ -313,34 +313,16 @@ state_changed_cb (CallsMainWindow *self, CallsManager *manager) { const gchar *error = NULL; - switch (calls_manager_get_state (manager)) - { - case CALLS_MANAGER_STATE_READY: - break; + CallsManagerFlags state_flags = calls_manager_get_state_flags (manager); - case CALLS_MANAGER_STATE_NO_VOICE_MODEM: - error = _("Can't place calls: No voice-capable modem available"); - break; - - case CALLS_MANAGER_STATE_NO_ORIGIN: - error = _("Can't place calls: No modem or VoIP account available"); - break; - - case CALLS_MANAGER_STATE_UNKNOWN: - case CALLS_MANAGER_STATE_NO_PROVIDER: - error = _("Can't place calls: No backend service"); - break; - - case CALLS_MANAGER_STATE_NO_PLUGIN: - error = _("Can't place calls: No plugin"); - break; - - default: - g_assert_not_reached (); - } + if (!(state_flags & CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM) && + !(state_flags & CALLS_MANAGER_FLAGS_HAS_VOIP_ACCOUNT)) + error = _("Can't place calls: No modem or VoIP account available"); + else if (state_flags & CALLS_MANAGER_FLAGS_UNKNOWN) + error = _("Can't place calls: No plugin loaded"); gtk_label_set_text (self->permanent_error_label, error); - gtk_revealer_set_reveal_child (self->permanent_error_revealer, error != NULL); + gtk_revealer_set_reveal_child (self->permanent_error_revealer, !!error); } diff --git a/src/calls-manager.c b/src/calls-manager.c index f5ed10b..205e3d3 100644 --- a/src/calls-manager.c +++ b/src/calls-manager.c @@ -61,7 +61,7 @@ struct _CallsManager CallsContactsProvider *contacts_provider; - CallsManagerState state; + CallsManagerFlags state_flags; CallsSettings *settings; }; @@ -76,7 +76,7 @@ G_DEFINE_TYPE_WITH_CODE (CallsManager, calls_manager, G_TYPE_OBJECT, enum { PROP_0, - PROP_STATE, + PROP_STATE_FLAGS, PROP_LAST_PROP, }; static GParamSpec *props[PROP_LAST_PROP]; @@ -95,48 +95,44 @@ static guint signals [SIGNAL_LAST_SIGNAL]; static void -set_state (CallsManager *self, CallsManagerState state) +set_state_flags (CallsManager *self, CallsManagerFlags state_flags) { - if (self->state == state) + if (self->state_flags == state_flags) return; - self->state = state; + self->state_flags = state_flags; - g_object_notify_by_pspec (G_OBJECT (self), props[PROP_STATE]); + g_object_notify_by_pspec (G_OBJECT (self), props[PROP_STATE_FLAGS]); } static void -update_state (CallsManager *self) +update_state_flags (CallsManager *self) { - guint n_items; GHashTableIter iter; gpointer value; + CallsManagerFlags state_flags = CALLS_MANAGER_FLAGS_UNKNOWN; g_assert (CALLS_IS_MANAGER (self)); - if (g_hash_table_size (self->providers) == 0) { - set_state (self, CALLS_MANAGER_STATE_NO_PROVIDER); - return; - } - g_hash_table_iter_init (&iter, self->providers); while (g_hash_table_iter_next (&iter, NULL, &value)) { CallsProvider *provider = CALLS_PROVIDER (value); - if (calls_provider_is_modem (provider) && !calls_provider_is_operational (provider)) { - set_state (self, CALLS_MANAGER_STATE_NO_VOICE_MODEM); - return; + if (calls_provider_is_modem (provider)) { + state_flags |= CALLS_MANAGER_FLAGS_HAS_CELLULAR_PROVIDER; + + if (calls_provider_is_operational (provider)) + state_flags |= CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM; + } else { + state_flags |= CALLS_MANAGER_FLAGS_HAS_VOIP_PROVIDER; + + if (calls_provider_is_operational (provider)) + state_flags |= CALLS_MANAGER_FLAGS_HAS_VOIP_ACCOUNT; } } - - n_items = g_list_model_get_n_items (G_LIST_MODEL (self->origins)); - - if (n_items) - set_state (self, CALLS_MANAGER_STATE_READY); - else - set_state (self, CALLS_MANAGER_STATE_NO_ORIGIN); + set_state_flags (self, state_flags); } @@ -366,8 +362,6 @@ remove_origin (CallsManager *self, CallsOrigin *origin) origin); else g_list_store_remove (self->origins, position); - - update_state (self); } /* rebuild_origins_by_protocols() when any origins were added or removed */ @@ -448,7 +442,7 @@ remove_provider (CallsManager *self, calls_provider_unload_plugin (name); rebuild_origins_by_protocols (self); - update_state (self); + update_state_flags (self); g_signal_emit (self, signals[PROVIDERS_CHANGED], 0); } @@ -532,7 +526,7 @@ origin_items_changed_cb (GListModel *model, } rebuild_origins_by_protocols (self); - update_state (self); + update_state_flags (self); } @@ -579,8 +573,8 @@ calls_manager_get_property (GObject *object, CallsManager *self = CALLS_MANAGER (object); switch (property_id) { - case PROP_STATE: - g_value_set_enum (value, calls_manager_get_state (self)); + case PROP_STATE_FLAGS: + g_value_set_flags (value, calls_manager_get_state_flags (self)); break; default: @@ -677,13 +671,13 @@ calls_manager_class_init (CallsManagerClass *klass) G_TYPE_NONE, 0); - props[PROP_STATE] = - g_param_spec_enum ("state", - "state", - "The state of the Manager", - CALLS_TYPE_MANAGER_STATE, - CALLS_MANAGER_STATE_NO_PROVIDER, - G_PARAM_READABLE | G_PARAM_EXPLICIT_NOTIFY); + props[PROP_STATE_FLAGS] = + g_param_spec_flags ("state", + "state", + "The state of the Manager", + CALLS_TYPE_MANAGER_FLAGS, + CALLS_MANAGER_FLAGS_UNKNOWN, + G_PARAM_READABLE | G_PARAM_EXPLICIT_NOTIFY); g_object_class_install_properties (object_class, PROP_LAST_PROP, props); } @@ -697,7 +691,7 @@ calls_manager_init (CallsManager *self) PeasEngine *peas; const gchar *dir; - self->state = CALLS_MANAGER_STATE_NO_PROVIDER; + self->state_flags = CALLS_MANAGER_FLAGS_UNKNOWN; self->providers = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, @@ -843,12 +837,12 @@ calls_manager_is_modem_provider (CallsManager *self, } -CallsManagerState -calls_manager_get_state (CallsManager *self) +CallsManagerFlags +calls_manager_get_state_flags (CallsManager *self) { - g_return_val_if_fail (CALLS_IS_MANAGER (self), CALLS_MANAGER_STATE_UNKNOWN); + g_return_val_if_fail (CALLS_IS_MANAGER (self), CALLS_MANAGER_FLAGS_UNKNOWN); - return self->state; + return self->state_flags; } diff --git a/src/calls-manager.h b/src/calls-manager.h index 1eadc2e..13c4e73 100644 --- a/src/calls-manager.h +++ b/src/calls-manager.h @@ -37,15 +37,14 @@ G_BEGIN_DECLS G_DECLARE_FINAL_TYPE (CallsManager, calls_manager, CALLS, MANAGER, GObject) -typedef enum +typedef enum /*< flags >*/ { - CALLS_MANAGER_STATE_UNKNOWN = 1, - CALLS_MANAGER_STATE_NO_PLUGIN, - CALLS_MANAGER_STATE_NO_PROVIDER, - CALLS_MANAGER_STATE_NO_ORIGIN, - CALLS_MANAGER_STATE_NO_VOICE_MODEM, - CALLS_MANAGER_STATE_READY, -} CallsManagerState; + CALLS_MANAGER_FLAGS_UNKNOWN = 0, + CALLS_MANAGER_FLAGS_HAS_CELLULAR_PROVIDER = (1<<0), + CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM = (1<<1), + CALLS_MANAGER_FLAGS_HAS_VOIP_PROVIDER = (1<<2), + CALLS_MANAGER_FLAGS_HAS_VOIP_ACCOUNT = (1<<3), +} CallsManagerFlags; CallsManager *calls_manager_new (void); @@ -60,7 +59,7 @@ gboolean calls_manager_has_provider (CallsManager *sel const char *name); gboolean calls_manager_is_modem_provider (CallsManager *self, const char *name); -CallsManagerState calls_manager_get_state (CallsManager *self); +CallsManagerFlags calls_manager_get_state_flags (CallsManager *self); GListModel *calls_manager_get_origins (CallsManager *self); GList *calls_manager_get_calls (CallsManager *self); void calls_manager_dial (CallsManager *self, diff --git a/tests/test-manager.c b/tests/test-manager.c index 210840f..9d4c71f 100644 --- a/tests/test-manager.c +++ b/tests/test-manager.c @@ -32,7 +32,7 @@ test_calls_manager_without_provider (void) g_autoptr (CallsManager) manager = calls_manager_new (); g_assert_true (CALLS_IS_MANAGER (manager)); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); origins = calls_manager_get_origins (manager); no_origins = g_list_model_get_n_items (origins); @@ -61,7 +61,7 @@ test_calls_manager_dummy_provider (void) g_autoptr (CallsOrigin) origin = NULL; g_assert_true (CALLS_IS_MANAGER (manager)); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); origins = calls_manager_get_origins (manager); g_assert_true (origins); @@ -70,7 +70,10 @@ test_calls_manager_dummy_provider (void) calls_manager_add_provider (manager, "dummy"); g_assert_true (calls_manager_has_any_provider (manager)); g_assert_true (calls_manager_has_provider (manager, "dummy")); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_READY); + /* Dummy plugin fakes being a modem */ + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, + CALLS_MANAGER_FLAGS_HAS_CELLULAR_PROVIDER | + CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM); g_assert_cmpuint (g_list_model_get_n_items (origins), >, 0); g_assert_null (calls_manager_get_calls (manager)); @@ -104,7 +107,7 @@ test_calls_manager_dummy_provider (void) g_assert_cmpuint (g_list_model_get_n_items (origins), ==, 0); g_assert_cmpuint (g_list_model_get_n_items (origins_tel), ==, 0); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); } static void @@ -114,13 +117,13 @@ test_calls_manager_mm_provider (void) g_autoptr (CallsManager) manager = calls_manager_new (); g_assert_true (CALLS_IS_MANAGER (manager)); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); calls_manager_add_provider (manager, "mm"); g_assert_true (calls_manager_has_any_provider (manager)); g_assert_true (calls_manager_has_provider (manager, "mm")); - g_assert_cmpuint (calls_manager_get_state (manager), >, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), >, CALLS_MANAGER_FLAGS_UNKNOWN); g_assert_null (calls_manager_get_calls (manager)); @@ -129,7 +132,7 @@ test_calls_manager_mm_provider (void) g_assert_cmpuint (g_list_model_get_n_items (origins_tel), ==, 0); calls_manager_remove_provider (manager, "mm"); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); } static void @@ -148,7 +151,7 @@ test_calls_manager_multiple_providers_mm_sip (void) origins = calls_manager_get_origins (manager); g_assert_true (G_IS_LIST_MODEL (origins)); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_PROVIDER); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN); origins_tel = calls_manager_get_suitable_origins (manager, "tel:+123456789"); g_assert_nonnull (origins_tel); @@ -167,6 +170,7 @@ test_calls_manager_multiple_providers_mm_sip (void) g_assert_true (calls_manager_has_any_provider (manager)); g_assert_true (calls_manager_has_provider (manager, "sip")); g_assert_true (calls_manager_is_modem_provider (manager, "sip") == FALSE); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_HAS_VOIP_PROVIDER); /* Still no origins */ origins_tel = calls_manager_get_suitable_origins (manager, "tel:+123456789"); @@ -178,11 +182,8 @@ test_calls_manager_multiple_providers_mm_sip (void) origins_sips = calls_manager_get_suitable_origins (manager, "sips:bob@example.org"); g_assert_cmpuint (g_list_model_get_n_items (origins_sips), ==, 0); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_ORIGIN); - /** - * If we now load the MM plugin, the manager state should be *_STATE_NO_VOICE_MODEM - * (unless run on a phone I guess?) + * If we now load the MM plugin, the manager flag _HAS_CELLULAR_PROVIDER should be set * TODO DBus mock to add modems * see https://source.puri.sm/Librem5/calls/-/issues/280 * and https://source.puri.sm/Librem5/calls/-/issues/178 @@ -190,7 +191,9 @@ test_calls_manager_multiple_providers_mm_sip (void) calls_manager_add_provider (manager, "mm"); g_assert_true (calls_manager_has_any_provider (manager)); g_assert_true (calls_manager_has_provider (manager, "mm")); - g_assert_cmpuint (calls_manager_get_state (manager), ==, CALLS_MANAGER_STATE_NO_VOICE_MODEM); + g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, + CALLS_MANAGER_FLAGS_HAS_VOIP_PROVIDER | + CALLS_MANAGER_FLAGS_HAS_CELLULAR_PROVIDER); }