From fc136ef1295d1115ffeecbd4a8cd4575f9947176 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 2 Dec 2024 00:41:39 +0000 Subject: [PATCH] fix: replace connectivity state "Connected" with "Preparing" This better reflects that this state means we just connected and there may me work to do. This state is converted to DC_CONNECTIVITY_WORKING instead of DC_CONNECTIVITY_CONNECTED state now. Before this change when IMAP connected to the server, it switched from DC_CONNECTIVITY_NOT_CONNECTED to DC_CONNECTIVITY_CONNECTING, then to DC_CONNECTIVITY_CONNECTED (actually preparing) then to DC_CONNECTIVITY_WORKING and then to DC_CONNECTIVITY_CONNECTED again (actually idle). On fast connections this resulted in flickering "Connected" string in the status bar right before "Updating..." and on slow connections this "Connected" state before "Updating..." lasted for a while leaving the user to wonder if there are no new messages or if Delta Chat will still switch to "Updating..." before going into "Connected" state again. --- python/tests/test_1_online.py | 2 +- src/imap.rs | 2 +- src/scheduler/connectivity.rs | 66 ++++++++++++++++++++++++----------- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index ea1e45e278..13b8b1a17a 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -1899,7 +1899,7 @@ def test_connectivity(acfactory, lp): ac1.start_io() ac1._evtracker.wait_for_connectivity(dc.const.DC_CONNECTIVITY_CONNECTING) - ac1._evtracker.wait_for_connectivity_change(dc.const.DC_CONNECTIVITY_CONNECTING, dc.const.DC_CONNECTIVITY_CONNECTED) + ac1._evtracker.wait_for_connectivity_change(dc.const.DC_CONNECTIVITY_CONNECTING, dc.const.DC_CONNECTIVITY_WORKING) lp.sec( "Test that after calling start_io(), maybe_network() and waiting for `all_work_done()`, " diff --git a/src/imap.rs b/src/imap.rs index 4eb432599d..2358f70865 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -407,7 +407,7 @@ impl Imap { "IMAP-LOGIN as {}", lp.user ))); - self.connectivity.set_connected(context).await; + self.connectivity.set_preparing(context).await; info!(context, "Successfully logged into IMAP server"); return Ok(session); } diff --git a/src/scheduler/connectivity.rs b/src/scheduler/connectivity.rs index 337b126caf..284ff1ee58 100644 --- a/src/scheduler/connectivity.rs +++ b/src/scheduler/connectivity.rs @@ -16,10 +16,25 @@ use super::InnerSchedulerState; #[derive(Debug, Clone, Copy, PartialEq, Eq, EnumProperty, PartialOrd, Ord)] pub enum Connectivity { + /// Not connected. + /// + /// This may be because we just started, + /// because we lost connection and + /// were not able to connect and login yet + /// or because I/O is not started. NotConnected = 1000, + + /// Attempting to connect and log in. Connecting = 2000, - /// Fetching or sending messages + + /// Fetching or sending messages. Working = 3000, + + /// We are connected but not doing anything. + /// + /// UIs display this as Connected, + /// which signals that no more messages + /// are coming in. Connected = 4000, } @@ -32,13 +47,17 @@ enum DetailedConnectivity { Error(String), #[default] Uninitialized, + + /// Attempting to connect, + /// until we successully log in. Connecting, - /// Connection is just established, but there may be work to do. - Connected, + /// Connection is just established, + /// there may be work to do. + Preparing, /// There is actual work to do, e.g. there are messages in SMTP queue - /// or we detected a message that should be downloaded. + /// or we detected a message on IMAP server that should be downloaded. Working, InterruptingIdle, @@ -58,7 +77,13 @@ impl DetailedConnectivity { DetailedConnectivity::Connecting => Some(Connectivity::Connecting), DetailedConnectivity::Working => Some(Connectivity::Working), DetailedConnectivity::InterruptingIdle => Some(Connectivity::Connected), - DetailedConnectivity::Connected => Some(Connectivity::Connected), + + // At this point IMAP has just connected, + // but does not know yet if there are messages to download. + // We still convert this to Working state + // so user can see "Updating..." and not "Connected" + // which is reserved for idle state. + DetailedConnectivity::Preparing => Some(Connectivity::Working), // Just don't return a connectivity, probably the folder is configured not to be // watched or there is e.g. no "Sent" folder, so we are not interested in it @@ -74,9 +99,9 @@ impl DetailedConnectivity { | DetailedConnectivity::Uninitialized | DetailedConnectivity::NotConfigured => "".to_string(), DetailedConnectivity::Connecting => "".to_string(), - DetailedConnectivity::Working + DetailedConnectivity::Preparing + | DetailedConnectivity::Working | DetailedConnectivity::InterruptingIdle - | DetailedConnectivity::Connected | DetailedConnectivity::Idle => "".to_string(), } } @@ -86,10 +111,12 @@ impl DetailedConnectivity { DetailedConnectivity::Error(e) => stock_str::error(context, e).await, DetailedConnectivity::Uninitialized => "Not started".to_string(), DetailedConnectivity::Connecting => stock_str::connecting(context).await, - DetailedConnectivity::Working => stock_str::updating(context).await, - DetailedConnectivity::InterruptingIdle - | DetailedConnectivity::Connected - | DetailedConnectivity::Idle => stock_str::connected(context).await, + DetailedConnectivity::Preparing | DetailedConnectivity::Working => { + stock_str::updating(context).await + } + DetailedConnectivity::InterruptingIdle | DetailedConnectivity::Idle => { + stock_str::connected(context).await + } DetailedConnectivity::NotConfigured => "Not configured".to_string(), } } @@ -107,7 +134,7 @@ impl DetailedConnectivity { // since sending the last message, connectivity could have changed, which we don't notice // until another message is sent DetailedConnectivity::InterruptingIdle - | DetailedConnectivity::Connected + | DetailedConnectivity::Preparing | DetailedConnectivity::Idle => stock_str::last_msg_sent_successfully(context).await, DetailedConnectivity::NotConfigured => "Not configured".to_string(), } @@ -120,7 +147,7 @@ impl DetailedConnectivity { DetailedConnectivity::Connecting => false, DetailedConnectivity::Working => false, DetailedConnectivity::InterruptingIdle => false, - DetailedConnectivity::Connected => false, // Just connected, there may still be work to do. + DetailedConnectivity::Preparing => false, // Just connected, there may still be work to do. DetailedConnectivity::NotConfigured => true, DetailedConnectivity::Idle => true, } @@ -148,8 +175,8 @@ impl ConnectivityStore { pub(crate) async fn set_working(&self, context: &Context) { self.set(context, DetailedConnectivity::Working).await; } - pub(crate) async fn set_connected(&self, context: &Context) { - self.set(context, DetailedConnectivity::Connected).await; + pub(crate) async fn set_preparing(&self, context: &Context) { + self.set(context, DetailedConnectivity::Preparing).await; } pub(crate) async fn set_not_configured(&self, context: &Context) { self.set(context, DetailedConnectivity::NotConfigured).await; @@ -169,7 +196,7 @@ impl ConnectivityStore { } } -/// Set all folder states to InterruptingIdle in case they were `Connected` before. +/// Set all folder states to InterruptingIdle in case they were `Idle` before. /// Called during `dc_maybe_network()` to make sure that `dc_all_work_done()` /// returns false immediately after `dc_maybe_network()`. pub(crate) async fn idle_interrupted(inbox: ConnectivityStore, oboxes: Vec) { @@ -179,8 +206,7 @@ pub(crate) async fn idle_interrupted(inbox: ConnectivityStore, oboxes: Vec