GNOME.org
 

[calls] manager: Use state flags instead of an enum and adapt to changes

commit d75d4916af035e3710c46ca751632f2b0fb2466d
Author: Evangelos Ribeiro Tzaras <devrtz fortysixandtwo eu>
Date:   Wed Jan 26 14:18:24 2022 +0100

    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 20239fe4..1948f410 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 7103bb98..4823397a 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;
-
-    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;
+  CallsManagerFlags state_flags = calls_manager_get_state_flags (manager);
 
-    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 f5ed10b5..205e3d32 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;
 
-  n_items = g_list_model_get_n_items (G_LIST_MODEL (self->origins));
+      if (calls_provider_is_operational (provider))
+        state_flags |= CALLS_MANAGER_FLAGS_HAS_CELLULAR_MODEM;
+    } else {
+      state_flags |= CALLS_MANAGER_FLAGS_HAS_VOIP_PROVIDER;
 
-  if (n_items)
-    set_state (self, CALLS_MANAGER_STATE_READY);
-  else
-    set_state (self, CALLS_MANAGER_STATE_NO_ORIGIN);
+      if (calls_provider_is_operational (provider))
+        state_flags |= CALLS_MANAGER_FLAGS_HAS_VOIP_ACCOUNT;
+    }
+  }
+  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 1eadc2e1..13c4e732 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 210840f7..9d4c71f9 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);
 
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]