From b46cbf229f6597ed64a506ecb138185563684b27 Mon Sep 17 00:00:00 2001 From: Daniel Barber Date: Wed, 26 Feb 2020 22:30:09 -0500 Subject: [PATCH] Refactor error and retain server connected state --- Persephone.xcodeproj/project.pbxproj | 12 ++++++ .../Shared/Extensions/Notification.swift | 1 + .../Components/Shared/MPDServerDelegate.swift | 30 +++++-------- .../Components/Window/WindowController.swift | 43 ++++++++++++++++--- .../Extensions/MPDClient+Command.swift | 4 +- .../Extensions/MPDClient+Connection.swift | 2 +- .../Extensions/MPDClient+Error.swift | 24 +++++++++-- Persephone/MPDClient/Models/MPDError.swift | 40 ++--------------- Persephone/State/Actions/PlayerActions.swift | 3 +- Persephone/State/Actions/ServerActions.swift | 13 ++++++ Persephone/State/AppState.swift | 1 + Persephone/State/PlayerState.swift | 1 - Persephone/State/Reducers/AppReducer.swift | 1 + Persephone/State/Reducers/PlayerReducer.swift | 8 ++++ Persephone/State/Reducers/ServerReducer.swift | 23 ++++++++++ Persephone/State/ServerState.swift | 13 ++++++ 16 files changed, 150 insertions(+), 69 deletions(-) create mode 100644 Persephone/State/Actions/ServerActions.swift create mode 100644 Persephone/State/Reducers/ServerReducer.swift create mode 100644 Persephone/State/ServerState.swift diff --git a/Persephone.xcodeproj/project.pbxproj b/Persephone.xcodeproj/project.pbxproj index f4af39f..96795c0 100644 --- a/Persephone.xcodeproj/project.pbxproj +++ b/Persephone.xcodeproj/project.pbxproj @@ -121,6 +121,9 @@ E4F26F7723411AE300D45FF9 /* ArtistListActions.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F26F7623411AE300D45FF9 /* ArtistListActions.swift */; }; E4F26F7923411B1500D45FF9 /* ArtistReducer.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F26F7823411B1500D45FF9 /* ArtistReducer.swift */; }; E4F26F7B23411D5400D45FF9 /* MPDClient+Artist.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F26F7A23411D5400D45FF9 /* MPDClient+Artist.swift */; }; + E4F2EFEE24076A2700198159 /* ServerState.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F2EFED24076A2700198159 /* ServerState.swift */; }; + E4F2EFF024076B0900198159 /* ServerActions.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F2EFEF24076B0900198159 /* ServerActions.swift */; }; + E4F2EFF224076B5E00198159 /* ServerReducer.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F2EFF124076B5E00198159 /* ServerReducer.swift */; }; E4F6B460221E119B00ACF42A /* QueueDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F6B45F221E119B00ACF42A /* QueueDataSource.swift */; }; E4F6B463221E125900ACF42A /* QueueItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F6B462221E125900ACF42A /* QueueItem.swift */; }; E4F6B467221E233200ACF42A /* AlbumDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4F6B466221E233200ACF42A /* AlbumDataSource.swift */; }; @@ -322,6 +325,9 @@ E4F26F7623411AE300D45FF9 /* ArtistListActions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArtistListActions.swift; sourceTree = ""; }; E4F26F7823411B1500D45FF9 /* ArtistReducer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArtistReducer.swift; sourceTree = ""; }; E4F26F7A23411D5400D45FF9 /* MPDClient+Artist.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MPDClient+Artist.swift"; sourceTree = ""; }; + E4F2EFED24076A2700198159 /* ServerState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerState.swift; sourceTree = ""; }; + E4F2EFEF24076B0900198159 /* ServerActions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerActions.swift; sourceTree = ""; }; + E4F2EFF124076B5E00198159 /* ServerReducer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ServerReducer.swift; sourceTree = ""; }; E4F6B45F221E119B00ACF42A /* QueueDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QueueDataSource.swift; sourceTree = ""; }; E4F6B462221E125900ACF42A /* QueueItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QueueItem.swift; sourceTree = ""; }; E4F6B466221E233200ACF42A /* AlbumDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlbumDataSource.swift; sourceTree = ""; }; @@ -662,6 +668,7 @@ E4B11B60226A4BFF0075461B /* PlayerReducer.swift */, E4FF718F227601B400D4C412 /* PreferencesReducer.swift */, E4B11B74226CC4D30075461B /* QueueReducer.swift */, + E4F2EFF124076B5E00198159 /* ServerReducer.swift */, E440519D227BB0720090CD6F /* UIReducer.swift */, ); path = Reducers; @@ -678,6 +685,7 @@ E4B11B65226A4F830075461B /* PlayerState.swift */, E4FF718D2276010E00D4C412 /* PreferencesState.swift */, E4B11B67226A4FA00075461B /* QueueState.swift */, + E4F2EFED24076A2700198159 /* ServerState.swift */, E440519F227BB0AB0090CD6F /* UIState.swift */, ); path = State; @@ -691,6 +699,7 @@ E4B11BBD2275EDAA0075461B /* PlayerActions.swift */, E4FF71912276029000D4C412 /* PreferencesActions.swift */, E4B11BBF2275EE150075461B /* QueueActions.swift */, + E4F2EFEF24076B0900198159 /* ServerActions.swift */, E440519B227BAF2E0090CD6F /* UIActions.swift */, ); path = Actions; @@ -977,6 +986,7 @@ E450AD7E222620A10091BED3 /* Album.swift in Sources */, E4DA820623D6236200C1EE58 /* NSSize.swift in Sources */, E408D3B6220DD8970006D9BE /* Notification.swift in Sources */, + E4F2EFEE24076A2700198159 /* ServerState.swift in Sources */, E4B46F8F2402E89800152157 /* MPDError.swift in Sources */, E43AC1F822C7065A001E483C /* AlbumCoverButton.swift in Sources */, E45962C62241A78500FC1A1E /* MPDCommand.swift in Sources */, @@ -995,6 +1005,7 @@ E4FF71922276029000D4C412 /* PreferencesActions.swift in Sources */, E489E39922B85D0400CA8CBD /* NSPasteboard.swift in Sources */, E43AC1F122C68E6A001E483C /* NSPasteboardItem.swift in Sources */, + E4F2EFF224076B5E00198159 /* ServerReducer.swift in Sources */, E465049A21E94DF500A70F4C /* WindowController.swift in Sources */, E41B22C621FB932700D544F6 /* MPDClient.swift in Sources */, E47E2FDD2220A6D100F747E6 /* Time.swift in Sources */, @@ -1025,6 +1036,7 @@ E41EA46C221636AF0068EF46 /* GeneralPrefsViewController.swift in Sources */, E4E8CC902204EC7F0024217A /* Delegate.swift in Sources */, E47E2FD5222071FD00F747E6 /* AlbumViewItem.swift in Sources */, + E4F2EFF024076B0900198159 /* ServerActions.swift in Sources */, E408D3BE220E03EE0006D9BE /* RawRepresentable.swift in Sources */, E4B11B66226A4F830075461B /* PlayerState.swift in Sources */, E4B11BBE2275EDAA0075461B /* PlayerActions.swift in Sources */, diff --git a/Persephone/Components/Shared/Extensions/Notification.swift b/Persephone/Components/Shared/Extensions/Notification.swift index 45a5169..c17c7fc 100644 --- a/Persephone/Components/Shared/Extensions/Notification.swift +++ b/Persephone/Components/Shared/Extensions/Notification.swift @@ -12,4 +12,5 @@ extension Notification.Name { static let didConnect = Notification.Name("MPDClientDidConnect") static let willDisconnect = Notification.Name("MPDClientWillDisconnect") static let didReloadAlbumArt = Notification.Name("MPDDidReloadAlbumArt") + static let didRaiseError = Notification.Name("MPDDidRaiseError") } diff --git a/Persephone/Components/Shared/MPDServerDelegate.swift b/Persephone/Components/Shared/MPDServerDelegate.swift index 0dabdb2..8de86fb 100644 --- a/Persephone/Components/Shared/MPDServerDelegate.swift +++ b/Persephone/Components/Shared/MPDServerDelegate.swift @@ -11,31 +11,23 @@ import AppKit class MPDServerDelegate: MPDClientDelegate { func didConnect(mpdClient: MPDClient) { NotificationCenter.default.post(name: .didConnect, object: nil) + DispatchQueue.main.async { + App.store.dispatch(UpdateConnectedState(connected: true)) + } } func willDisconnect(mpdClient: MPDClient) { NotificationCenter.default.post(name: .willDisconnect, object: nil) + DispatchQueue.main.async { + App.store.dispatch(UpdateConnectedState(connected: false)) + } } - func didRaiseError(mpdClient: MPDClient, error: MPDClient.MPDError) { - DispatchQueue.main.async { - let alert = NSAlert(error: error) - switch error { - case .success: - break; - case .outOfMemory(let message), - .argument(let message), - .state(let message), - .timeout(let message), - .system(let message), - .resolver(let message), - .malformed(let message), - .closed(let message), - .server(let message): - alert.messageText = message - alert.runModal() - } - } + func didRaiseError( + mpdClient: MPDClient, + error: MPDClient.MPDError + ) { + NotificationCenter.default.post(name: .didRaiseError, object: error) } func didUpdateStatus(mpdClient: MPDClient, status: MPDClient.MPDStatus) { diff --git a/Persephone/Components/Window/WindowController.swift b/Persephone/Components/Window/WindowController.swift index 499db98..6cb1af6 100644 --- a/Persephone/Components/Window/WindowController.swift +++ b/Persephone/Components/Window/WindowController.swift @@ -38,13 +38,14 @@ class WindowController: NSWindowController { App.store.subscribe(self) { $0.select { - ($0.playerState, $0.uiState) + ($0.serverState, $0.playerState, $0.uiState) } } App.store.dispatch(MainWindowDidOpenAction()) NotificationCenter.default.addObserver(self, selector: #selector(willDisconnect), name: .willDisconnect, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(reportError), name: .didRaiseError, object: nil) trackProgress.font = .timerFont trackRemaining.font = .timerFont @@ -76,9 +77,18 @@ class WindowController: NSWindowController { } } - func setShuffleRepeatState(_ state: PlayerState) { - shuffleState.state = state.shuffleState ? .on : .off - repeatState.state = state.repeatState ? .on : .off + func setShuffleRepeatState( + _ serverState: ServerState, + _ playerState: PlayerState + ) { + shuffleState.isEnabled = serverState.connected + repeatState.isEnabled = serverState.connected + shuffleState.state = playerState.shuffleState ? .on : .off + repeatState.state = playerState.repeatState ? .on : .off + } + + func setSearchState(_ serverState: ServerState) { + searchQuery.isEnabled = serverState.connected } func setTrackProgressControls(_ playerState: PlayerState) { @@ -144,10 +154,26 @@ class WindowController: NSWindowController { @objc func willDisconnect() { DispatchQueue.main.async { - App.store.dispatch(SetSearchQuery(searchQuery: "")) + App.store.dispatch(ResetStatusAction()) self.searchQuery.stringValue = "" } } + + @objc func reportError(_ notification: NSNotification) { + guard let error = notification.object as? MPDClient.MPDError + else { return } + + DispatchQueue.main.async { + let alert = NSAlert(error: error) + alert.informativeText = error.message + + alert.alertStyle = error.recovered ? .warning : .critical + + guard let window = NSApplication.shared.mainWindow + else { return } + alert.beginSheetModal(for: window) { _ in } + } + } // TODO: Refactor this using a gesture recognizer @IBAction func changeTrackProgress(_ sender: NSSlider) { @@ -224,12 +250,15 @@ extension WindowController: NSWindowDelegate { } extension WindowController: StoreSubscriber { - typealias StoreSubscriberStateType = (playerState: PlayerState, uiState: UIState) + typealias StoreSubscriberStateType = ( + serverState: ServerState, playerState: PlayerState, uiState: UIState + ) func newState(state: StoreSubscriberStateType) { DispatchQueue.main.async { self.setTransportControlState(state.playerState) - self.setShuffleRepeatState(state.playerState) + self.setShuffleRepeatState(state.serverState, state.playerState) + self.setSearchState(state.serverState) self.setTrackProgressControls(state.playerState) self.setDatabaseUpdatingIndicator(state.uiState) self.setVolumeControlIcon(state.playerState) diff --git a/Persephone/MPDClient/Extensions/MPDClient+Command.swift b/Persephone/MPDClient/Extensions/MPDClient+Command.swift index 0358974..5f8afdb 100644 --- a/Persephone/MPDClient/Extensions/MPDClient+Command.swift +++ b/Persephone/MPDClient/Extensions/MPDClient+Command.swift @@ -157,7 +157,9 @@ extension MPDClient { let commandOperation = BlockOperation() { [unowned self] in self.sendCommand(command: command, userData: userData) - self.idle(forceIdle) + if self.checkError() { + self.idle(forceIdle) + } } commandOperation.queuePriority = priority diff --git a/Persephone/MPDClient/Extensions/MPDClient+Connection.swift b/Persephone/MPDClient/Extensions/MPDClient+Connection.swift index 39cbb80..48e0e6c 100644 --- a/Persephone/MPDClient/Extensions/MPDClient+Connection.swift +++ b/Persephone/MPDClient/Extensions/MPDClient+Connection.swift @@ -15,7 +15,7 @@ extension MPDClient { let error = mpd_connection_get_error(connection) guard error == MPD_ERROR_SUCCESS else { - handleError(mpdError: error) + _ = handleError(mpdError: error) return } diff --git a/Persephone/MPDClient/Extensions/MPDClient+Error.swift b/Persephone/MPDClient/Extensions/MPDClient+Error.swift index 44b13b9..68f28c7 100644 --- a/Persephone/MPDClient/Extensions/MPDClient+Error.swift +++ b/Persephone/MPDClient/Extensions/MPDClient+Error.swift @@ -10,14 +10,32 @@ import Foundation import mpdclient extension MPDClient { - func handleError(mpdError: mpd_error) { + func checkError() -> Bool { + let error = mpd_connection_get_error(connection) + + if error != MPD_ERROR_SUCCESS { + return handleError(mpdError: error) + } + + return true + } + + func handleError(mpdError: mpd_error) -> Bool { guard let errorMessage = mpd_connection_get_error_message(connection) - else { return } + else { return true } let message = String(cString: errorMessage) - let error = MPDError(mpdError: mpdError, message: message) + let recovered = mpd_connection_clear_error(connection) + + let error = MPDError( + mpdError: mpdError, + recovered: recovered, + message: message + ) delegate?.didRaiseError(mpdClient: self, error: error) + + return recovered } func getLastErrorMessage() -> String? { diff --git a/Persephone/MPDClient/Models/MPDError.swift b/Persephone/MPDClient/Models/MPDError.swift index 3324bb7..40ed519 100644 --- a/Persephone/MPDClient/Models/MPDError.swift +++ b/Persephone/MPDClient/Models/MPDError.swift @@ -10,41 +10,9 @@ import Foundation import mpdclient extension MPDClient { - enum MPDError: Error { - init(mpdError: mpd_error, message: String) { - switch mpdError { - case MPD_ERROR_OOM: - self = .outOfMemory(message) - case MPD_ERROR_ARGUMENT: - self = .argument(message) - case MPD_ERROR_STATE: - self = .state(message) - case MPD_ERROR_TIMEOUT: - self = .timeout(message) - case MPD_ERROR_SYSTEM: - self = .system(message) - case MPD_ERROR_RESOLVER: - self = .resolver(message) - case MPD_ERROR_MALFORMED: - self = .malformed(message) - case MPD_ERROR_CLOSED: - self = .closed(message) - case MPD_ERROR_SERVER: - self = .server(message) - default: - self = .success - } - } - - case success - case outOfMemory(String) - case argument(String) - case state(String) - case timeout(String) - case system(String) - case resolver(String) - case malformed(String) - case closed(String) - case server(String) + struct MPDError: Error { + let mpdError: mpd_error + let recovered: Bool + let message: String } } diff --git a/Persephone/State/Actions/PlayerActions.swift b/Persephone/State/Actions/PlayerActions.swift index a879506..fd4a74f 100644 --- a/Persephone/State/Actions/PlayerActions.swift +++ b/Persephone/State/Actions/PlayerActions.swift @@ -6,7 +6,6 @@ // Copyright © 2019 Dan Barber. All rights reserved. // -import AppKit import ReSwift struct UpdateCurrentSongAction: Action { @@ -17,6 +16,8 @@ struct UpdateElapsedTimeAction: Action { var elapsedTimeMs: UInt = 0 } +struct ResetStatusAction: Action {} + struct UpdateStatusAction: Action { var status: MPDClient.MPDStatus } diff --git a/Persephone/State/Actions/ServerActions.swift b/Persephone/State/Actions/ServerActions.swift new file mode 100644 index 0000000..27c4948 --- /dev/null +++ b/Persephone/State/Actions/ServerActions.swift @@ -0,0 +1,13 @@ +// +// ServerActions.swift +// Persephone +// +// Created by Daniel Barber on 2/26/20. +// Copyright © 2020 Dan Barber. All rights reserved. +// + +import ReSwift + +struct UpdateConnectedState: Action { + var connected: Bool +} diff --git a/Persephone/State/AppState.swift b/Persephone/State/AppState.swift index fdc56d9..c83895d 100644 --- a/Persephone/State/AppState.swift +++ b/Persephone/State/AppState.swift @@ -9,6 +9,7 @@ import ReSwift struct AppState: StateType { + var serverState = ServerState() var playerState = PlayerState() var queueState = QueueState() var albumListState = AlbumListState() diff --git a/Persephone/State/PlayerState.swift b/Persephone/State/PlayerState.swift index e576430..4d69db9 100644 --- a/Persephone/State/PlayerState.swift +++ b/Persephone/State/PlayerState.swift @@ -6,7 +6,6 @@ // Copyright © 2019 Dan Barber. All rights reserved. // -import AppKit import ReSwift struct PlayerState: StateType { diff --git a/Persephone/State/Reducers/AppReducer.swift b/Persephone/State/Reducers/AppReducer.swift index f30a77a..09a3b61 100644 --- a/Persephone/State/Reducers/AppReducer.swift +++ b/Persephone/State/Reducers/AppReducer.swift @@ -10,6 +10,7 @@ import ReSwift func appReducer(action: Action, state: AppState?) -> AppState { return AppState( + serverState: serverReducer(action: action, state: state?.serverState), playerState: playerReducer(action: action, state: state?.playerState), queueState: queueReducer(action: action, state: state?.queueState), albumListState: albumListReducer(action: action, state: state?.albumListState), diff --git a/Persephone/State/Reducers/PlayerReducer.swift b/Persephone/State/Reducers/PlayerReducer.swift index 2e168c8..b6cccf2 100644 --- a/Persephone/State/Reducers/PlayerReducer.swift +++ b/Persephone/State/Reducers/PlayerReducer.swift @@ -13,6 +13,14 @@ func playerReducer(action: Action, state: PlayerState?) -> PlayerState { var state = state ?? PlayerState() switch action { + case is ResetStatusAction: + state.state = .unknown + state.totalTime = 0 + state.elapsedTimeMs = 0 + state.shuffleState = false + state.repeatState = false + state.volume = -1 + case let action as UpdateStatusAction: state.status = action.status state.state = action.status.state diff --git a/Persephone/State/Reducers/ServerReducer.swift b/Persephone/State/Reducers/ServerReducer.swift new file mode 100644 index 0000000..6d01c67 --- /dev/null +++ b/Persephone/State/Reducers/ServerReducer.swift @@ -0,0 +1,23 @@ +// +// ServerReducer.swift +// Persephone +// +// Created by Daniel Barber on 2/26/20. +// Copyright © 2020 Dan Barber. All rights reserved. +// + +import ReSwift + +func serverReducer(action: Action, state: ServerState?) -> ServerState { + var state = state ?? ServerState() + + switch action { + case let action as UpdateConnectedState: + state.connected = action.connected + + default: + break + } + + return state +} diff --git a/Persephone/State/ServerState.swift b/Persephone/State/ServerState.swift new file mode 100644 index 0000000..b275c36 --- /dev/null +++ b/Persephone/State/ServerState.swift @@ -0,0 +1,13 @@ +// +// ServerState.swift +// Persephone +// +// Created by Daniel Barber on 2/26/20. +// Copyright © 2020 Dan Barber. All rights reserved. +// + +import ReSwift + +struct ServerState: StateType { + var connected: Bool = false +}