GNOME.org
 

[calls] account: Rework account states

commit cdb6f90acc3f7bea2aba9c8c1b08413b70eeac44
Author: Evangelos Ribeiro Tzaras <devrtz fortysixandtwo eu>
Date:   Tue Dec 21 08:20:56 2021 +0100

    account: Rework account states
    
    Introduce a state-changed signal which also gives a reason for why the state
    changed. This will allow the UI to give some meaningful feedback to the user.
    
    Additionally we can get rid of a number of things that were not really states,
    but rather reasons for why a state changed (f.e. authentication failures).

 plugins/sip/calls-sip-origin.c | 105 ++++++++++++++++++++++++++++-------------
 src/calls-account-row.c        |   2 +-
 src/calls-account.c            |  28 ++++++++++-
 src/calls-account.h            |  89 +++++++++++++++++++++++-----------
 tests/test-sip.c               |   6 +--
 5 files changed, 164 insertions(+), 66 deletions(-)
---
diff --git a/plugins/sip/calls-sip-origin.c b/plugins/sip/calls-sip-origin.c
index 963773db..18bd7e7c 100644
--- a/plugins/sip/calls-sip-origin.c
+++ b/plugins/sip/calls-sip-origin.c
@@ -135,6 +135,25 @@ G_DEFINE_TYPE_WITH_CODE (CallsSipOrigin, calls_sip_origin, G_TYPE_OBJECT,
                          G_IMPLEMENT_INTERFACE (CALLS_TYPE_ACCOUNT,
                                                 calls_sip_origin_accounts_interface_init))
 
+static void
+change_state (CallsSipOrigin          *self,
+              CallsAccountState        new_state,
+              CallsAccountStateReason  reason)
+{
+  CallsAccountState old_state;
+
+  g_assert (CALLS_SIP_ORIGIN (self));
+
+  if (self->state == new_state)
+    return;
+
+  old_state = self->state;
+  self->state = new_state;
+
+  g_signal_emit_by_name (self, "account-state-changed", old_state, new_state, reason);
+}
+
+
 static void
 remove_call (CallsSipOrigin *self,
              CallsCall      *call,
@@ -292,7 +311,9 @@ dial (CallsOrigin *origin,
     return;
   }
 
-  if (calls_account_get_state (CALLS_ACCOUNT (origin)) != CALLS_ACCOUNT_ONLINE) {
+  if (calls_account_get_state (CALLS_ACCOUNT (origin)) !=
+      CALLS_ACCOUNT_STATE_ONLINE) {
+
     g_warning ("Tried dialing '%s' on origin '%s', but it's not online",
                address, name);
     return;
@@ -416,7 +437,9 @@ sip_r_register (int              status,
 
   if (status == 200) {
     g_debug ("REGISTER successful");
-    origin->state = CALLS_ACCOUNT_ONLINE;
+    change_state (origin,
+                  CALLS_ACCOUNT_STATE_ONLINE,
+                  CALLS_ACCOUNT_STATE_REASON_CONNECTED);
     nua_get_params (nua, TAG_ANY (), TAG_END());
 
     if (sip->sip_contact && sip->sip_contact->m_url && sip->sip_contact->m_url->url_host) {
@@ -426,18 +449,19 @@ sip_r_register (int              status,
 
   } else if (status == 401 || status == 407) {
     sip_authenticate (origin, nh, sip);
-    origin->state = CALLS_ACCOUNT_AUTHENTICATING;
 
   } else if (status == 403) {
     g_warning ("wrong credentials?");
-    origin->state = CALLS_ACCOUNT_AUTHENTICATION_FAILURE;
+    change_state (origin,
+                  CALLS_ACCOUNT_STATE_OFFLINE,
+                  CALLS_ACCOUNT_STATE_REASON_AUTHENTICATION_FAILURE);
 
   } else if (status == 904) {
     g_warning ("unmatched challenge");
-    origin->state = CALLS_ACCOUNT_AUTHENTICATION_FAILURE;
+    change_state (origin,
+                  CALLS_ACCOUNT_STATE_ERROR,
+                  CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR);
   }
-
-  g_object_notify_by_pspec (G_OBJECT (origin), props[PROP_ACC_STATE]);
 }
 
 
@@ -455,14 +479,15 @@ sip_r_unregister (int              status,
 
   if (status == 200) {
     g_debug ("Unregistering successful");
-    origin->state = CALLS_ACCOUNT_OFFLINE;
-
+    change_state (origin,
+                  CALLS_ACCOUNT_STATE_OFFLINE,
+                  CALLS_ACCOUNT_STATE_REASON_DISCONNECTED);
   } else {
     g_warning ("Unregisterung unsuccessful: %03d %s", status, phrase);
-    origin->state = CALLS_ACCOUNT_UNKNOWN_ERROR;
+    change_state (origin,
+                  CALLS_ACCOUNT_STATE_ERROR,
+                  CALLS_ACCOUNT_STATE_REASON_UNKNOWN);
   }
-
-  g_object_notify_by_pspec (G_OBJECT (origin), props[PROP_ACC_STATE]);
 }
 
 
@@ -925,7 +950,7 @@ go_online (CallsAccount *account,
   if (online) {
     g_autofree char *registrar_url = NULL;
 
-    if (self->state == CALLS_ACCOUNT_ONLINE)
+    if (self->state == CALLS_ACCOUNT_STATE_ONLINE)
       return;
 
     registrar_url = get_registrar_url (self);
@@ -937,7 +962,7 @@ go_online (CallsAccount *account,
                   TAG_END ());
 
   } else {
-    if (self->state == CALLS_ACCOUNT_OFFLINE)
+    if (self->state == CALLS_ACCOUNT_STATE_OFFLINE)
       return;
 
     nua_unregister (self->oper->register_handle,
@@ -1002,6 +1027,12 @@ static gboolean
 init_sip_account (CallsSipOrigin *self,
                   GError        **error)
 {
+  g_assert (CALLS_IS_SIP_ORIGIN (self));
+
+  change_state (self,
+                CALLS_ACCOUNT_STATE_INITIALIZING,
+                CALLS_ACCOUNT_STATE_REASON_INITIALIZATION_STARTED);
+
   if (self->use_direct_connection) {
     g_debug ("Direct connection case. Using user and hostname");
     setup_account_for_direct_connection (self);
@@ -1013,8 +1044,10 @@ init_sip_account (CallsSipOrigin *self,
                  "init_sip_account (). "
                  "Try again when account is setup");
 
-    self->state = CALLS_ACCOUNT_NO_CREDENTIALS;
-    goto err;
+    change_state (self,
+                  CALLS_ACCOUNT_STATE_ERROR,
+                  CALLS_ACCOUNT_STATE_REASON_NO_CREDENTIALS);
+    return FALSE;
   }
 
   // setup_nua() and setup_sip_handles() only after account data has been set
@@ -1023,8 +1056,10 @@ init_sip_account (CallsSipOrigin *self,
     g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                  "Failed setting up nua context");
 
-    self->state = CALLS_ACCOUNT_NULL;
-    goto err;
+    change_state (self,
+                  CALLS_ACCOUNT_STATE_ERROR,
+                  CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR);
+    return FALSE;
   }
 
   self->oper = setup_sip_handles (self);
@@ -1032,26 +1067,21 @@ init_sip_account (CallsSipOrigin *self,
     g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                  "Failed setting operation handles");
 
-    self->state = CALLS_ACCOUNT_NULL;
-    goto err;
+    change_state (self,
+                  CALLS_ACCOUNT_STATE_ERROR,
+                  CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR);
+    return FALSE;
   }
 
   /* In the case of a direct connection we're immediately good to go */
-  if (self->use_direct_connection)
-    self->state = CALLS_ACCOUNT_ONLINE;
-  else {
-    self->state = CALLS_ACCOUNT_OFFLINE;
-  }
+  change_state (self,
+                self->use_direct_connection ? CALLS_ACCOUNT_STATE_ONLINE : CALLS_ACCOUNT_STATE_OFFLINE,
+                CALLS_ACCOUNT_STATE_REASON_INITIALIZED);
 
   if (self->auto_connect)
     go_online (CALLS_ACCOUNT (self), TRUE);
 
-  g_object_notify_by_pspec (G_OBJECT (self), props[PROP_ACC_STATE]);
   return TRUE;
-
- err:
-  g_object_notify_by_pspec (G_OBJECT (self), props[PROP_ACC_STATE]);
-  return FALSE;
 }
 
 
@@ -1060,9 +1090,13 @@ deinit_sip_account (CallsSipOrigin *self)
 {
   g_assert (CALLS_IS_SIP_ORIGIN (self));
 
-  if (self->state == CALLS_ACCOUNT_NULL)
+  if (self->state == CALLS_ACCOUNT_STATE_UNKNOWN)
     return TRUE;
 
+  change_state (self,
+                CALLS_ACCOUNT_STATE_DEINITIALIZING,
+                CALLS_ACCOUNT_STATE_REASON_DEINITIALIZATION_STARTED);
+
   remove_calls (self, NULL);
 
   if (self->nua) {
@@ -1078,6 +1112,9 @@ deinit_sip_account (CallsSipOrigin *self)
 
     if (!self->is_shutdown_success) {
       g_warning ("nua_shutdown() timed out. Cannot proceed");
+      change_state (self,
+                    CALLS_ACCOUNT_STATE_ERROR,
+                    CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR);
       return FALSE;
     }
     g_debug ("nua_shutdown() complete. Destroying nua handle");
@@ -1086,7 +1123,9 @@ deinit_sip_account (CallsSipOrigin *self)
   }
 
   g_clear_pointer (&self->own_ip, g_free);
-  self->state = CALLS_ACCOUNT_NULL;
+  change_state (self,
+                CALLS_ACCOUNT_STATE_UNKNOWN,
+                CALLS_ACCOUNT_STATE_REASON_DEINITIALIZED);
   return TRUE;
 }
 
@@ -1318,7 +1357,7 @@ calls_sip_origin_dispose (GObject *object)
 
   g_clear_pointer (&self->own_ip, g_free);
 
-  if (!self->use_direct_connection && self->state == CALLS_ACCOUNT_ONLINE)
+  if (!self->use_direct_connection && self->state == CALLS_ACCOUNT_STATE_ONLINE)
     go_online (CALLS_ACCOUNT (self), FALSE);
 
   deinit_sip_account (self);
diff --git a/src/calls-account-row.c b/src/calls-account-row.c
index 1824c5c7..ca7294ce 100644
--- a/src/calls-account-row.c
+++ b/src/calls-account-row.c
@@ -74,7 +74,7 @@ on_account_state_changed (CallsAccountRow *self)
 {
   CallsAccountState state = calls_account_get_state (self->account);
 
-  gtk_switch_set_active (self->online_switch, state == CALLS_ACCOUNT_ONLINE);
+  gtk_switch_set_active (self->online_switch, state == CALLS_ACCOUNT_STATE_ONLINE);
 }
 
 static void
diff --git a/src/calls-account.c b/src/calls-account.c
index 3a6365c1..414b7141 100644
--- a/src/calls-account.c
+++ b/src/calls-account.c
@@ -35,6 +35,12 @@
 
 G_DEFINE_INTERFACE (CallsAccount, calls_account, CALLS_TYPE_ORIGIN)
 
+enum {
+  SIGNAL_STATE_CHANGED,
+  SIGNAL_LAST_SIGNAL,
+};
+static guint signals [SIGNAL_LAST_SIGNAL];
+
 
 static void
 calls_account_default_init (CallsAccountInterface *iface)
@@ -44,7 +50,7 @@ calls_account_default_init (CallsAccountInterface *iface)
                        "Account state",
                        "The state of the account",
                        CALLS_TYPE_ACCOUNT_STATE,
-                       CALLS_ACCOUNT_NULL,
+                       CALLS_ACCOUNT_STATE_UNKNOWN,
                        G_PARAM_READABLE |
                        G_PARAM_STATIC_STRINGS |
                        G_PARAM_EXPLICIT_NOTIFY));
@@ -57,6 +63,24 @@ calls_account_default_init (CallsAccountInterface *iface)
                          G_PARAM_READABLE |
                          G_PARAM_STATIC_STRINGS |
                          G_PARAM_EXPLICIT_NOTIFY));
+
+  /**
+   * CallsAccount::account-state-changed:
+   * @self: The #CallsAccount
+   * @new_state: The new #CALLS_ACCOUNT_STATE of the account
+   * @old_state: The old #CALLS_ACCOUNT_STATE of the account
+   * @reason: The #CALLS_ACCOUNT_STATE_REASON for the change
+   */
+  signals[SIGNAL_STATE_CHANGED] =
+    g_signal_new ("account-state-changed",
+                  G_TYPE_FROM_INTERFACE (iface),
+                  G_SIGNAL_RUN_LAST,
+                  0, NULL, NULL, NULL,
+                  G_TYPE_NONE,
+                  3,
+                  CALLS_TYPE_ACCOUNT_STATE,
+                  CALLS_TYPE_ACCOUNT_STATE,
+                  CALLS_TYPE_ACCOUNT_STATE_REASON);
 }
 
 /**
@@ -91,7 +115,7 @@ calls_account_get_state (CallsAccount *self)
 {
   CallsAccountState state;
 
-  g_return_val_if_fail (CALLS_IS_ACCOUNT (self), CALLS_ACCOUNT_NULL);
+  g_return_val_if_fail (CALLS_IS_ACCOUNT (self), CALLS_ACCOUNT_STATE_UNKNOWN);
 
   g_object_get (self, "account-state", &state, NULL);
 
diff --git a/src/calls-account.h b/src/calls-account.h
index 7dcf815b..8f566605 100644
--- a/src/calls-account.h
+++ b/src/calls-account.h
@@ -33,7 +33,62 @@ G_BEGIN_DECLS
 
 #define CALLS_TYPE_ACCOUNT (calls_account_get_type ())
 
-G_DECLARE_INTERFACE (CallsAccount, calls_account, CALLS, ACCOUNT, CallsOrigin);
+G_DECLARE_INTERFACE (CallsAccount, calls_account, CALLS, ACCOUNT, CallsOrigin)
+
+/**
+ * CallsAccountState:
+ * @CALLS_ACCOUNT_STATE_UNKNOWN: Default state for new accounts
+ * @CALLS_ACCOUNT_STATE_INITIALIZING: Account is initializing
+ * @CALLS_ACCOUNT_STATE_DEINITIALIZING: Account is being shutdown
+ * @CALLS_ACCOUNT_STATE_CONNECTING: Connecting to server
+ * @CALLS_ACCOUNT_STATE_ONLINE: Account is online
+ * @CALLS_ACCOUNT_STATE_DISCONNECTING: Disconnecting from server
+ * @CALLS_ACCOUNT_STATE_OFFLINE: Account is offline
+ * @CALLS_ACCOUNT_STATE_ERROR: Account is in an error state
+ */
+typedef enum {
+  CALLS_ACCOUNT_STATE_UNKNOWN = 0,
+  CALLS_ACCOUNT_STATE_INITIALIZING,
+  CALLS_ACCOUNT_STATE_DEINITIALIZING,
+  CALLS_ACCOUNT_STATE_CONNECTING,
+  CALLS_ACCOUNT_STATE_ONLINE,
+  CALLS_ACCOUNT_STATE_DISCONNECTING,
+  CALLS_ACCOUNT_STATE_OFFLINE,
+  CALLS_ACCOUNT_STATE_ERROR,
+} CallsAccountState;
+
+/**
+ * CallsAccountStateReason:
+ * @CALLS_ACCOUNT_STATE_REASON_UNKNOWN: Default unspecified reason
+ * @CALLS_ACCOUNT_STATE_REASON_INITIALIZATION_STARTED: Initialization started
+ * @CALLS_ACCOUNT_STATE_REASON_INITIALIZED: Initialization done
+ * @CALLS_ACCOUNT_STATE_REASON_DEINITIALIZATION_STARTED: Deinitialization started
+ * @CALLS_ACCOUNT_STATE_REASON_DEINITIALIZED: Deinitialization done
+ * @CALLS_ACCOUNT_STATE_REASON_NO_CREDENTIALS: No credentials were set
+ * @CALLS_ACCOUNT_STATE_REASON_CONNECT_STARTED: Starting to connect
+ * @CALLS_ACCOUNT_STATE_REASON_CONNECTION_TIMEOUT: A connection has timed out
+ * @CALLS_ACCOUNT_STATE_REASON_CONNECTION_DNS_ERROR: A domain name could not be resolved
+ * @CALLS_ACCOUNT_STATE_REASON_AUTHENTICATION_FAILURE: Could not authenticate, possibly wrong credentials
+ * @CALLS_ACCOUNT_STATE_REASON_CONNECTED: Connected successfully
+ * @CALLS_ACCOUNT_STATE_REASON_DISCONNECTED: Disconnected successfully
+ * @CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR: An internal error has occurred
+ */
+typedef enum {
+  CALLS_ACCOUNT_STATE_REASON_UNKNOWN = 0,
+  CALLS_ACCOUNT_STATE_REASON_INITIALIZATION_STARTED,
+  CALLS_ACCOUNT_STATE_REASON_INITIALIZED,
+  CALLS_ACCOUNT_STATE_REASON_DEINITIALIZATION_STARTED,
+  CALLS_ACCOUNT_STATE_REASON_DEINITIALIZED,
+  CALLS_ACCOUNT_STATE_REASON_NO_CREDENTIALS,
+  CALLS_ACCOUNT_STATE_REASON_CONNECT_STARTED,
+  CALLS_ACCOUNT_STATE_REASON_CONNECTION_TIMEOUT,
+  CALLS_ACCOUNT_STATE_REASON_CONNECTION_DNS_ERROR,
+  CALLS_ACCOUNT_STATE_REASON_AUTHENTICATION_FAILURE,
+  CALLS_ACCOUNT_STATE_REASON_CONNECTED,
+  CALLS_ACCOUNT_STATE_REASON_DISCONNECT_STARTED,
+  CALLS_ACCOUNT_STATE_REASON_DISCONNECTED,
+  CALLS_ACCOUNT_STATE_REASON_INTERNAL_ERROR,
+} CallsAccountStateReason;
 
 
 struct _CallsAccountInterface
@@ -44,33 +99,13 @@ struct _CallsAccountInterface
                                                              gboolean      online);
   const char        *(*get_address)                         (CallsAccount *self);
 };
-/**
- * CallsAccountState:
- * @CALLS_ACCOUNT_NULL: Not initialized or error
- * @CALLS_ACCOUNT_OFFLINE: Account considered offline (not ready for placing or receiving calls)
- * @CALLS_ACCOUNT_CONNECTING: Trying to connect to server
- * @CALLS_ACCOUNT_CONNECTION_FAILURE: Could not connect to server (f.e. DNS error, unreachable)
- * @CALLS_ACCOUNT_AUTHENTICATING: Authenticating using web-auth/proxy-auth
- * @CALLS_ACCOUNT_AUTHENTICATION_FAILURE: Could not authenticate to server (f.e. wrong credentials)
- * @CALLS_ACCOUNT_NO_CREDENTIALS: Credentials are missing
- * @CALLS_ACCOUNT_ONLINE: Account considered online (can place and receive calls)
- */
-typedef enum {
-  CALLS_ACCOUNT_NULL = 0,
-  CALLS_ACCOUNT_OFFLINE,
-  CALLS_ACCOUNT_CONNECTING,
-  CALLS_ACCOUNT_CONNECTION_FAILURE,
-  CALLS_ACCOUNT_AUTHENTICATING,
-  CALLS_ACCOUNT_AUTHENTICATION_FAILURE,
-  CALLS_ACCOUNT_NO_CREDENTIALS,
-  CALLS_ACCOUNT_UNKNOWN_ERROR,
-  CALLS_ACCOUNT_ONLINE
-} CallsAccountState;
 
 
-void                   calls_account_go_online          (CallsAccount *self,
-                                                         gboolean      online);
-const char            *calls_account_get_address        (CallsAccount *self);
-CallsAccountState      calls_account_get_state          (CallsAccount *self);
+void                   calls_account_go_online              (CallsAccount            *self,
+                                                             gboolean                 online);
+const char            *calls_account_get_address            (CallsAccount            *self);
+CallsAccountState      calls_account_get_state              (CallsAccount            *self);
+const char            *calls_account_state_to_string        (CallsAccountState       *state);
+const char            *calls_account_state_reason_to_string (CallsAccountStateReason *reason);
 
 G_END_DECLS
diff --git a/tests/test-sip.c b/tests/test-sip.c
index d0847b8c..d6d8bd5c 100644
--- a/tests/test-sip.c
+++ b/tests/test-sip.c
@@ -110,9 +110,9 @@ test_sip_origin_objects (SipFixture   *fixture,
                 "account-state", &state_offline,
                 NULL);
 
-  g_assert_cmpint (state_alice, ==, CALLS_ACCOUNT_ONLINE);
-  g_assert_cmpint (state_bob, ==, CALLS_ACCOUNT_ONLINE);
-  g_assert_cmpint (state_offline, ==, CALLS_ACCOUNT_OFFLINE);
+  g_assert_cmpint (state_alice, ==, CALLS_ACCOUNT_STATE_ONLINE);
+  g_assert_cmpint (state_bob, ==, CALLS_ACCOUNT_STATE_ONLINE);
+  g_assert_cmpint (state_offline, ==, CALLS_ACCOUNT_STATE_OFFLINE);
 }
 
 static void
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]