From cdb6f90acc3f7bea2aba9c8c1b08413b70eeac44 Mon Sep 17 00:00:00 2001 From: Evangelos Ribeiro Tzaras Date: Tue, 21 Dec 2021 08:20:56 +0100 Subject: [PATCH] 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 963773d..18bd7e7 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 1824c5c..ca7294c 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 3a6365c..414b714 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 7dcf815..8f56660 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 d0847b8..d6d8bd5 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