From d1da6779322fd785669e83eca9fe7f8cd5313acd Mon Sep 17 00:00:00 2001 From: James Magahern Date: Sat, 4 Jul 2020 19:43:50 -0700 Subject: [PATCH] Fixes issue where connectivity issues can lead to persistent "Offline" status in UI This fixes the problem where either internet connectivity issues on the device or intermittant server issues can cause a persistent "OFFLINE" indicator even though that is not the case. This requires the server protocol to be more granular about what their connection status is, and adds a new signal from the multiplexer to the MainViewController allowing it to notify the delegate that an acknowledgement was heard from one of the servers, allowing the main view controller to update its UI according to whatever status best represents the group of servers as a whole. Testing - This can be tested by using the network link conditioner to simulate connection issues *after* the control panel has connected to all servers in the multiplex. - Tested losing internet connection and regaining internet connection - Tested starting off with no connection and regaining connection later --- .../Controllers/MainViewController.swift | 12 +++--- XIONControlPanel/Servers/HubitatServer.swift | 10 ++--- .../Servers/ServerMultiplex.swift | 38 +++++++++++++++---- XIONControlPanel/Servers/ServerProtocol.swift | 10 ++++- XIONControlPanel/Servers/WemoServer.swift | 16 ++------ 5 files changed, 55 insertions(+), 31 deletions(-) diff --git a/XIONControlPanel/Controllers/MainViewController.swift b/XIONControlPanel/Controllers/MainViewController.swift index ea39e6d..eaa8b8c 100644 --- a/XIONControlPanel/Controllers/MainViewController.swift +++ b/XIONControlPanel/Controllers/MainViewController.swift @@ -206,11 +206,6 @@ class MainViewController: UIViewController, SwitchesViewControllerDelegate, Serv { _switchesController.devices = Array(multiplex.devices) _updateVisualization(false) - - // Update connection status too, if applicable - if _serverMultiplex.numServers == _serverMultiplex.numberOfConnectedServers() { - _updateConnectivityStatus(.connected) - } } func serverMultiplex(_ multiplex: ServerMultiplex, devicesStateChanged devices: [AnyDevice]) @@ -218,6 +213,13 @@ class MainViewController: UIViewController, SwitchesViewControllerDelegate, Serv _switchesController.devicesStateChanged(devices) } + func serverMultiplex(_ multiplex: ServerMultiplex, didReceiveAcknowledgementFromServer server: Server) + { + // Update connection status too, if applicable + let groupConnectionStatus = multiplex.groupConnectionStatus() + _updateConnectivityStatus(groupConnectionStatus) + } + func serverMultiplex(_ multiplex: ServerMultiplex, didEncounterError error: Error) { _updateConnectivityStatus(.error) diff --git a/XIONControlPanel/Servers/HubitatServer.swift b/XIONControlPanel/Servers/HubitatServer.swift index 5d7eb56..a6183b7 100644 --- a/XIONControlPanel/Servers/HubitatServer.swift +++ b/XIONControlPanel/Servers/HubitatServer.swift @@ -16,7 +16,7 @@ public enum HubitatServerError : Error public class HubitatServer : Server { - var connected: Bool = false + var connectionStatus: ConnectionStatus = .disconnected public fileprivate(set) var devices: [HubitatDevice] = [] @@ -33,13 +33,13 @@ public class HubitatServer : Server func connect(_ completion: @escaping (Error?) -> Void) { // RESTful, assume we're connected unless we get an error - self.connected = true + connectionStatus = .connected completion(nil) } func disconnect(_ completion: (Error?) -> Void) { - self.connected = false + connectionStatus = .disconnected } func fetchDevices(_ fetchCompletion: @escaping (Result<[AnyDevice], Error>) -> Void) @@ -52,10 +52,10 @@ public class HubitatServer : Server .decode(type: [HubitatDevice].self, decoder: JSONDecoder()) .sink(receiveCompletion: { completion in if case let Subscribers.Completion.failure(error) = completion { - self.connected = false + self.connectionStatus = .error fetchCompletion(.failure(error)) } else { - self.connected = true + self.connectionStatus = .connected } }) { (devices: [HubitatDevice]) in self.devices = devices diff --git a/XIONControlPanel/Servers/ServerMultiplex.swift b/XIONControlPanel/Servers/ServerMultiplex.swift index cdfcfbf..90a6542 100644 --- a/XIONControlPanel/Servers/ServerMultiplex.swift +++ b/XIONControlPanel/Servers/ServerMultiplex.swift @@ -12,6 +12,7 @@ protocol ServerMultiplexDelegate { func serverMultiplex(_ multiplex: ServerMultiplex, didAddDevices devices: [AnyDevice]) func serverMultiplex(_ multiplex: ServerMultiplex, devicesStateChanged devices: [AnyDevice]) + func serverMultiplex(_ multiplex: ServerMultiplex, didReceiveAcknowledgementFromServer server: Server) func serverMultiplex(_ multiplex: ServerMultiplex, didEncounterError error: Error) } @@ -33,9 +34,26 @@ class ServerMultiplex servers.append(server) } - public func numberOfConnectedServers() -> Int + /// Returns a status that best represents the status of the whole group, as if it were one device + public func groupConnectionStatus() -> ConnectionStatus { - return servers.filter { $0.connected == true }.count + let relevancyRank: (ConnectionStatus) -> Int = { status in + switch status { + case .disconnected: return 0 + case .connected: return 1 + case .connecting: return 2 + case .error: return 3 + } + } + + var groupStatus = ConnectionStatus.connected + servers.forEach { server in + if relevancyRank(server.connectionStatus) > relevancyRank(groupStatus) { + groupStatus = server.connectionStatus + } + } + + return groupStatus } public func toggleDeviceState(_ device: AnyDevice, state: DeviceState, completion: @escaping (Error?) -> Void) @@ -56,7 +74,7 @@ class ServerMultiplex { self.servers.forEach { (server: Server) in server.fetchDevices { (result: Result<[AnyDevice], Error>) in - self.handleServerFetchResult(result) + self.handleServerFetchResult(forServer: server, result: result) } } } @@ -64,18 +82,22 @@ class ServerMultiplex extension ServerMultiplex { - private func handleServerFetchResult(_ result: Result<[AnyDevice], Error>) + private func handleServerFetchResult(forServer server: Server, result: Result<[AnyDevice], Error>) { switch result { case .success(let devices): - handleDevicesChanged(devices) + handleDevicesChanged(forServer: server, devicesChanged: devices) case .failure(let error): - handleError(error) + handleError(forServer: server, error: error) } } - private func handleDevicesChanged(_ devicesChanged: [AnyDevice]) + private func handleDevicesChanged(forServer server: Server, devicesChanged: [AnyDevice]) { + // First send server acknowledgement + self.delegate?.serverMultiplex(self, didReceiveAcknowledgementFromServer: server) + + // Then, optionally notify about new devices or device state changes let newDevicesSet = Set(devicesChanged) let additions = newDevicesSet.subtracting(self.devices) let changed = newDevicesSet.filter { (device: AnyDevice) in @@ -101,7 +123,7 @@ extension ServerMultiplex } } - private func handleError(_ error: Error) + private func handleError(forServer server: Server, error: Error) { DispatchQueue.main.async { self.delegate?.serverMultiplex(self, didEncounterError: error) diff --git a/XIONControlPanel/Servers/ServerProtocol.swift b/XIONControlPanel/Servers/ServerProtocol.swift index c9ab118..a5acb1c 100644 --- a/XIONControlPanel/Servers/ServerProtocol.swift +++ b/XIONControlPanel/Servers/ServerProtocol.swift @@ -8,9 +8,17 @@ import Foundation +enum ConnectionStatus +{ + case disconnected + case connecting + case connected + case error +} + protocol Server { - var connected: Bool { get } + var connectionStatus: ConnectionStatus { get } /// Designated initializer. Takes an API endpoint URL init(_ url: URL) diff --git a/XIONControlPanel/Servers/WemoServer.swift b/XIONControlPanel/Servers/WemoServer.swift index 5e688ac..829f043 100644 --- a/XIONControlPanel/Servers/WemoServer.swift +++ b/XIONControlPanel/Servers/WemoServer.swift @@ -8,14 +8,6 @@ import Foundation -enum ConnectionStatus -{ - case disconnected - case connecting - case connected - case error -} - enum ConnectionError : Error { case unknown @@ -25,11 +17,9 @@ enum ConnectionError : Error class WemoServer : Server { private var devices: [WemoDevice] = [] - - public var connected: Bool { get { return self.connectionStatus == .connected } } + public var connectionStatus: ConnectionStatus = .disconnected fileprivate(set) var baseURL: URL - fileprivate(set) var connectionStatus: ConnectionStatus = .disconnected fileprivate var _urlSession: URLSession fileprivate var _errorStream: StandardErrorOutputStream = StandardErrorOutputStream() @@ -48,6 +38,8 @@ class WemoServer : Server func connect(_ completion: @escaping (Error?) -> Void) { if (self.connectionStatus == .disconnected) { + self.connectionStatus = .connecting + let op = ConnectOperation(baseURL: self.baseURL, session: _urlSession) weak var weakOp = op op.completionBlock = { @@ -103,7 +95,7 @@ class WemoServer : Server func toggleDevice(_ device: AnyDevice, state: DeviceState, completion: @escaping (Error?) -> Void) { - if self.connected, let device = findDevice(device) { + if connectionStatus == .connected, let device = findDevice(device) { let op = ToggleDeviceOperation(baseURL: self.baseURL, session: _urlSession, device: device, state: state) weak var weakOp = op op.completionBlock = {