diff --git a/Sources/WLKit/BridgeController.swift b/Sources/WLKit/BridgeController.swift index bf18e02..b7ce554 100644 --- a/Sources/WLKit/BridgeController.swift +++ b/Sources/WLKit/BridgeController.swift @@ -193,12 +193,19 @@ public final class BridgeController: ObservableObject { lastError = nil } catch { deviceConnected = false - let message = error.localizedDescription - if message.contains("0xE00002C1") || message.contains("Input Monitoring") { + // Ask the API that knows about the grant rather than reading the + // error code, which says "not permitted" for a wedged pad too. + let granted = IOHIDCheckAccess(kIOHIDRequestTypeListenEvent) == kIOHIDAccessTypeGranted + switch DeviceOpenFailure.classify(accessGranted: granted, message: error.localizedDescription) { + case .permissionMissing: permissionDenied = true - if !warnedPermission { warnedPermission = true; lastError = message } - } else { - lastError = message + if !warnedPermission { + warnedPermission = true + lastError = DeviceOpenFailure.permissionMissing.message + } + case .deviceUnavailable(let underlying): + permissionDenied = false + lastError = DeviceOpenFailure.deviceUnavailable(underlying).message } scheduleReopen() return diff --git a/Sources/WLKit/DeviceOpenFailure.swift b/Sources/WLKit/DeviceOpenFailure.swift new file mode 100644 index 0000000..a670d65 --- /dev/null +++ b/Sources/WLKit/DeviceOpenFailure.swift @@ -0,0 +1,33 @@ +import Foundation + +/// Why the pad would not open, in terms someone can act on. +/// +/// `IOHIDDeviceOpen` returns `kIOReturnNotPermitted` (0xE00002E2) for a missing +/// Input Monitoring grant *and* for a device that is already held or has got +/// itself wedged, so reading the code alone gets it wrong half the time. The +/// original check only recognised the older 0xE00002C1, so on current systems a +/// permissions failure fell through and the menu simply said it was looking for +/// the pad, which sends you hunting for a cable when the answer is a checkbox. +public enum DeviceOpenFailure: Equatable { + case permissionMissing + case deviceUnavailable(String) + + /// `accessGranted` comes from `IOHIDCheckAccess`, the only caller that + /// actually knows the state of the grant. An error that names a permission + /// outright is still believed, in case the check itself is stale. + public static func classify(accessGranted: Bool, message: String) -> DeviceOpenFailure { + if message.contains("0xE00002C1") || message.contains("Input Monitoring") { + return .permissionMissing + } + return accessGranted ? .deviceUnavailable(message) : .permissionMissing + } + + public var message: String { + switch self { + case .permissionMissing: + return "Input Monitoring is not granted. System Settings › Privacy & Security › Input Monitoring, then switch the manager off and on." + case .deviceUnavailable(let underlying): + return "The pad did not open. Something else may be holding it, or it needs a reconnect - unplug it, wait a moment, plug it back in. (\(underlying))" + } + } +} diff --git a/Sources/WLKit/IconSymbols.swift b/Sources/WLKit/IconSymbols.swift new file mode 100644 index 0000000..ba965af --- /dev/null +++ b/Sources/WLKit/IconSymbols.swift @@ -0,0 +1,26 @@ +import Foundation + +/// The SF Symbols the menu-bar icon draws. +/// +/// Gathered here rather than written inline at the draw site so a test can +/// check they all still exist. A symbol name that is not on the running system +/// resolves to nil, and the drawing code has nowhere to go but a blank image, +/// so the icon quietly loses its glyph on exactly the machines where the symbol +/// was withdrawn or never shipped. +public enum IconSymbols { + + /// Running, nothing to report. + public static let idle = "keyboard" + + /// Agents are doing something worth a colour. + public static let active = "keyboard.fill" + + /// No pad, or no permission to reach it. + /// + /// Not `keyboard.badge.exclamationmark`, which reads better but is not on + /// every system - where it is absent the icon drew as a bare dot with no + /// keyboard at all, in the two states where you most need to see it. + public static let problem = "keyboard.badge.ellipsis" + + public static let all = [idle, active, problem] +} diff --git a/Sources/WLMicroManager/MenuBarIcon.swift b/Sources/WLMicroManager/MenuBarIcon.swift index 45e2a6e..2765539 100644 --- a/Sources/WLMicroManager/MenuBarIcon.swift +++ b/Sources/WLMicroManager/MenuBarIcon.swift @@ -36,10 +36,10 @@ enum MenuBarIcon { var symbolName: String { switch self { - case .off: return "keyboard" - case .permissionDenied, .deviceMissing: return "keyboard.badge.exclamationmark" - case .idle: return "keyboard" - case .allIdle, .working, .blocked: return "keyboard.fill" + case .off: return IconSymbols.idle + case .permissionDenied, .deviceMissing: return IconSymbols.problem + case .idle: return IconSymbols.idle + case .allIdle, .working, .blocked: return IconSymbols.active } } @@ -71,7 +71,12 @@ enum MenuBarIcon { static func image(for state: State) -> NSImage { let symbolSize = NSSize(width: 18, height: 18) let config = NSImage.SymbolConfiguration(pointSize: 14, weight: .regular) - let symbol = NSImage(systemSymbolName: state.symbolName, accessibilityDescription: state.help)? + // Falling back to the plain keyboard rather than to a blank image: a + // symbol the running system does not have would otherwise leave the + // menu bar with a status dot floating on its own, which reads as a + // broken app rather than as whatever the state actually is. + let symbol = (NSImage(systemSymbolName: state.symbolName, accessibilityDescription: state.help) + ?? NSImage(systemSymbolName: IconSymbols.idle, accessibilityDescription: state.help))? .withSymbolConfiguration(config) ?? NSImage(size: symbolSize) diff --git a/Tests/WLKitTests/DeviceOpenFailureTests.swift b/Tests/WLKitTests/DeviceOpenFailureTests.swift new file mode 100644 index 0000000..dd5a557 --- /dev/null +++ b/Tests/WLKitTests/DeviceOpenFailureTests.swift @@ -0,0 +1,48 @@ +import XCTest +@testable import WLKit + +/// Telling "you have not granted permission" apart from "the pad is not +/// reachable", which look identical from the error code alone. +/// +/// `IOHIDDeviceOpen` answers `kIOReturnNotPermitted` (0xE00002E2) both when the +/// Input Monitoring grant is missing and when the device is already held or +/// wedged, so the code cannot be the thing that decides. `IOHIDCheckAccess` is +/// the only source that actually knows about the grant, so it casts the vote. +final class DeviceOpenFailureTests: XCTestCase { + + private let notPermitted = "IOHIDDeviceOpen failed (0xE00002E2)" + + func testMissingGrantIsReportedAsAPermissionProblem() { + let failure = DeviceOpenFailure.classify(accessGranted: false, message: notPermitted) + XCTAssertEqual(failure, .permissionMissing) + } + + /// The case that sent us in circles: the same error code with the grant + /// already in place is a device problem, and calling it a permission + /// problem sends someone back to System Settings for nothing. + func testSameErrorWithTheGrantInPlaceIsADeviceProblem() { + let failure = DeviceOpenFailure.classify(accessGranted: true, message: notPermitted) + XCTAssertEqual(failure, .deviceUnavailable(notPermitted)) + } + + /// Whatever we say next has to name the thing to try, because the error + /// code on its own has already proved useless to a reader. + func testTheDeviceMessageSuggestsSomethingToDo() { + let advice = DeviceOpenFailure.deviceUnavailable(notPermitted).message + XCTAssertTrue(advice.contains("reconnect") || advice.contains("unplug"), + "a wedged pad is fixed by reseating it, so say so: \(advice)") + XCTAssertTrue(advice.contains(notPermitted), "keep the raw error for anyone diagnosing") + } + + func testThePermissionMessageNamesTheSettingToLookFor() { + let advice = DeviceOpenFailure.permissionMissing.message + XCTAssertTrue(advice.contains("Input Monitoring"), advice) + } + + /// Older systems phrase the denial differently, and that reading should + /// still win over a stale access check. + func testAnExplicitPermissionErrorIsBelievedRegardless() { + let legacy = "IOHIDDeviceOpen failed (0xE00002C1)" + XCTAssertEqual(DeviceOpenFailure.classify(accessGranted: true, message: legacy), .permissionMissing) + } +} diff --git a/Tests/WLKitTests/IconSymbolsTests.swift b/Tests/WLKitTests/IconSymbolsTests.swift new file mode 100644 index 0000000..57f8e31 --- /dev/null +++ b/Tests/WLKitTests/IconSymbolsTests.swift @@ -0,0 +1,30 @@ +import XCTest +import AppKit +@testable import WLKit + +/// Guards the menu-bar glyph against a failure that is invisible until it is +/// embarrassing: `NSImage(systemSymbolName:)` returns nil for a name the running +/// system does not have, the drawing code falls back to an empty image, and the +/// icon becomes a bare status dot with no keyboard at all. +/// +/// Worse, the states that need the glyph most are the error ones, so the icon +/// vanishes precisely when someone is trying to work out what is wrong. +final class IconSymbolsTests: XCTestCase { + + func testEverySymbolTheIconDrawsExistsOnThisSystem() { + for name in IconSymbols.all { + XCTAssertNotNil( + NSImage(systemSymbolName: name, accessibilityDescription: nil), + "\(name) is not an SF Symbol on this system, so the icon would draw blank" + ) + } + } + + /// The glyph is how the icon is found in a crowded menu bar, so every state + /// keeps the keyboard rather than swapping to a generic warning mark. + func testEveryStateKeepsTheKeyboardShape() { + for name in IconSymbols.all { + XCTAssertTrue(name.hasPrefix("keyboard"), "\(name) would not read as this app") + } + } +}