From 519ac70842f7e0763461b80441d798972c15cdce Mon Sep 17 00:00:00 2001 From: Zoran Olujic Date: Fri, 3 Jul 2026 14:52:45 +0200 Subject: [PATCH 1/2] fix: address display control review findings --- Dimmerly/Services/BrightnessManager.swift | 5 +- Dimmerly/Services/ScheduleManager.swift | 74 ++++-- Dimmerly/Views/KeyboardShortcutRecorder.swift | 3 + Dimmerly/Views/MenuBarPanel.swift | 2 +- Dimmerly/Views/SettingsView.swift | 240 ++++++++++++------ DimmerlyTests/AppSettingsTests.swift | 179 +++++++++++++ DimmerlyTests/BrightnessManagerTests.swift | 35 +++ DimmerlyTests/MenuBarPanelTests.swift | 45 ++++ DimmerlyTests/ScheduleManagerTests.swift | 39 +++ 9 files changed, 517 insertions(+), 105 deletions(-) diff --git a/Dimmerly/Services/BrightnessManager.swift b/Dimmerly/Services/BrightnessManager.swift index bf949af..c4615bf 100644 --- a/Dimmerly/Services/BrightnessManager.swift +++ b/Dimmerly/Services/BrightnessManager.swift @@ -891,8 +891,9 @@ class BrightnessManager { return true } - let mode = HardwareBrightnessManager.shared.controlMode - return mode != .softwareOnly && HardwareBrightnessManager.shared.supportsDDC(for: display.id) + let hardwareManager = HardwareBrightnessManager.shared + let mode = hardwareManager.controlMode + return hardwareManager.isEnabled && mode != .softwareOnly && hardwareManager.supportsDDC(for: display.id) } private func setExternalHardwareBrightness(for displayID: CGDirectDisplayID, to value: Double) { diff --git a/Dimmerly/Services/ScheduleManager.swift b/Dimmerly/Services/ScheduleManager.swift index 0178c37..724ad59 100644 --- a/Dimmerly/Services/ScheduleManager.swift +++ b/Dimmerly/Services/ScheduleManager.swift @@ -61,6 +61,14 @@ class ScheduleManager { /// Cached enabled state to detect changes and avoid redundant timer restarts. private var lastEnabled: Bool? + /// A concrete schedule trigger that falls inside a polling catch-up window. + private struct ScheduleFireCandidate { + let triggerDate: Date + let scheduleIndex: Int + let schedule: DimmingSchedule + let dayString: String + } + /// UserDefaults key for persisting schedules as JSON. private static let schedulesKey = "dimmerlyDimmingSchedules" @@ -135,24 +143,10 @@ class ScheduleManager { let todayString = Self.dateString(for: now) let previousCheck = effectivePreviousCheckDate(for: now) - for schedule in schedules where schedule.isEnabled { - // Skip if already fired today (prevents firing same schedule multiple times per day) - if firedToday[schedule.id] == todayString { - continue - } - - // Resolve the trigger to a concrete date/time for today - // (sunrise/sunset times vary by date, so we must resolve each check) - guard let triggerDate = resolveTriggerDate(schedule.trigger, on: now) else { - continue - } - - // Fire once if the trigger time falls within the check window (previousCheck, now] - // This half-open interval ensures we fire exactly once when crossing the trigger time - if triggerDate > previousCheck, triggerDate <= now { - firedToday[schedule.id] = todayString - onScheduleTriggered?(schedule.presetID) - } + for candidate in fireCandidates(previousCheck: previousCheck, now: now) { + guard firedToday[candidate.schedule.id] != candidate.dayString else { continue } + firedToday[candidate.schedule.id] = candidate.dayString + onScheduleTriggered?(candidate.schedule.presetID) } // Clean up stale entries from firedToday (schedules fired on previous days) @@ -180,6 +174,50 @@ class ScheduleManager { return lastCheckDate } + /// Builds all schedule triggers crossed in `(previousCheck, now]`, sorted by trigger time. + private func fireCandidates(previousCheck: Date, now: Date) -> [ScheduleFireCandidate] { + let daysToCheck = Self.daysInRange(from: previousCheck, through: now) + var candidates: [ScheduleFireCandidate] = [] + + for (scheduleIndex, schedule) in schedules.enumerated() where schedule.isEnabled { + for day in daysToCheck { + let dayString = Self.dateString(for: day) + guard firedToday[schedule.id] != dayString else { continue } + guard let triggerDate = resolveTriggerDate(schedule.trigger, on: day) else { continue } + guard triggerDate > previousCheck, triggerDate <= now else { continue } + + candidates.append(ScheduleFireCandidate( + triggerDate: triggerDate, + scheduleIndex: scheduleIndex, + schedule: schedule, + dayString: dayString + )) + } + } + + return candidates.sorted { + if $0.triggerDate == $1.triggerDate { + return $0.scheduleIndex < $1.scheduleIndex + } + return $0.triggerDate < $1.triggerDate + } + } + + private static func daysInRange(from start: Date, through end: Date) -> [Date] { + let calendar = Calendar.current + var day = calendar.startOfDay(for: start) + let endDay = calendar.startOfDay(for: end) + var days: [Date] = [] + + while day <= endDay { + days.append(day) + guard let nextDay = calendar.date(byAdding: .day, value: 1, to: day) else { break } + day = nextDay + } + + return days + } + /// Converts a ScheduleTrigger to a concrete Date for a given day. /// /// Trigger types: diff --git a/Dimmerly/Views/KeyboardShortcutRecorder.swift b/Dimmerly/Views/KeyboardShortcutRecorder.swift index 5eef455..2e25072 100644 --- a/Dimmerly/Views/KeyboardShortcutRecorder.swift +++ b/Dimmerly/Views/KeyboardShortcutRecorder.swift @@ -122,6 +122,9 @@ private struct ShortcutRecorderView: NSViewRepresentable { if let captureView = nsView as? ShortcutCaptureView { captureView.isActive = isRecording } + if isRecording { + nsView.window?.makeFirstResponder(nsView) + } } } diff --git a/Dimmerly/Views/MenuBarPanel.swift b/Dimmerly/Views/MenuBarPanel.swift index 2461932..6a8aefb 100644 --- a/Dimmerly/Views/MenuBarPanel.swift +++ b/Dimmerly/Views/MenuBarPanel.swift @@ -1116,7 +1116,7 @@ struct DisplayBrightnessRow: View { guard !isBuiltIn else { return self } var copy = self - let hasDDC = hardwareManager.supportsDDC(for: displayID) + let hasDDC = hardwareManager.isEnabled && hardwareManager.supportsDDC(for: displayID) copy.hasDDC = hasDDC if hasDDC { diff --git a/Dimmerly/Views/SettingsView.swift b/Dimmerly/Views/SettingsView.swift index e87795b..5551e4e 100644 --- a/Dimmerly/Views/SettingsView.swift +++ b/Dimmerly/Views/SettingsView.swift @@ -37,26 +37,99 @@ func applyLaunchAtLoginChange( } #if !APPSTORE + @MainActor + func isDDCControlModeAvailable( + _ mode: DDCControlMode, + hardwareManager: HardwareBrightnessManager + ) -> Bool { + switch mode { + case .softwareOnly: + true + case .hardwareOnly, .combined: + hardwareManager.isEnabled + && hardwareManager.capabilities.values.contains { $0.supportsDDC && $0.supportsBrightness } + } + } + + func ddcFeatureLabels(for cap: HardwareDisplayCapability) -> String { + var labels: [String] = [] + if cap.supportsBrightness { labels.append("Brightness") } + if cap.supportsContrast { labels.append("Contrast") } + if cap.supportsVolume { labels.append("Volume") } + if cap.supportsInputSource { labels.append("Input") } + return labels.joined(separator: ", ") + } + + func ddcDisplayStatusSymbolName(for cap: HardwareDisplayCapability) -> String { + if cap.supportsDDC, cap.supportsBrightness { + return "checkmark.circle.fill" + } + if cap.supportsDDC { + return "exclamationmark.circle" + } + return "tv.and.mediabox" + } + + func ddcDisplayStatusText(for cap: HardwareDisplayCapability) -> String { + guard cap.supportsDDC else { + return "Software brightness" + } + + let features = ddcFeatureLabels(for: cap) + guard cap.supportsBrightness else { + return features.isEmpty ? "No hardware brightness" : "No hardware brightness (\(features))" + } + + return features.isEmpty ? "Hardware brightness" : features + } + + func ddcDisplayAccessibilityStatus(for cap: HardwareDisplayCapability) -> String { + guard cap.supportsDDC else { + return "using software brightness" + } + + let features = ddcFeatureLabels(for: cap) + guard cap.supportsBrightness else { + return features.isEmpty + ? "DDC available, no hardware brightness" + : "DDC available, no hardware brightness, \(features)" + } + + return features.isEmpty + ? "hardware brightness available" + : "hardware brightness available, \(features)" + } + + @MainActor + func applyDDCRuntimeSettings( + settings: AppSettings, + hardwareManager: HardwareBrightnessManager + ) { + hardwareManager.applyRuntimeSettings( + controlMode: settings.ddcControlMode, + pollingInterval: settings.ddcPollingInterval, + writeDelayMilliseconds: settings.ddcWriteDelay + ) + } + @MainActor func applyDDCEnabledChange( _ newValue: Bool, settings: AppSettings, - hardwareManager: HardwareBrightnessManager + hardwareManager: HardwareBrightnessManager, + brightnessManager: BrightnessManager = .shared ) { settings.ddcEnabled = newValue if newValue { hardwareManager.isEnabled = true - hardwareManager.applyRuntimeSettings( - controlMode: settings.ddcControlMode, - pollingInterval: settings.ddcPollingInterval, - writeDelayMilliseconds: settings.ddcWriteDelay - ) + applyDDCRuntimeSettings(settings: settings, hardwareManager: hardwareManager) hardwareManager.probeAllDisplays() hardwareManager.startPolling() } else { hardwareManager.isEnabled = false hardwareManager.stopPolling() + brightnessManager.reapplyAll() } } #endif @@ -447,31 +520,49 @@ struct DisplaySettingsTab: View { ) } )) - .help("Control monitor brightness, volume, and input via DDC/CI protocol") + .help("DDC/CI controls compatible external displays over the display cable. " + + "Built-in HDMI on Apple Silicon Macs, DisplayLink adapters, and most TVs may not work; " + + "unsupported displays use software brightness automatically.") if settings.ddcEnabled { - Picker("Control Mode:", selection: $settings.ddcControlMode) { + let selectedControlMode = isDDCControlModeAvailable( + settings.ddcControlMode, + hardwareManager: hardwareManager + ) ? settings.ddcControlMode : .softwareOnly + let hardwareControlModesAvailable = isDDCControlModeAvailable( + .hardwareOnly, + hardwareManager: hardwareManager + ) + + Picker("Control Mode:", selection: Binding( + get: { selectedControlMode }, + set: { settings.ddcControlMode = $0 } + )) { ForEach(DDCControlMode.allCases) { mode in - Text(mode.displayName).tag(mode) + Text(mode.displayName) + .tag(mode) + .disabled(!isDDCControlModeAvailable(mode, hardwareManager: hardwareManager)) } } .pickerStyle(.radioGroup) .help("Choose how Dimmerly controls display output") .onChange(of: settings.ddcControlMode) { - hardwareManager.applyRuntimeSettings( - controlMode: settings.ddcControlMode, - pollingInterval: settings.ddcPollingInterval, - writeDelayMilliseconds: settings.ddcWriteDelay - ) + applyDDCRuntimeSettings(settings: settings, hardwareManager: hardwareManager) brightnessManager.reapplyAll() } - Text(settings.ddcControlMode.description) + Text(selectedControlMode.description) .font(.caption) .foregroundStyle(.secondary) - Stepper(value: $settings.ddcPollingInterval, in: 1 ... 30) { - Text( + if !hardwareControlModesAvailable { + Text("Hardware modes require a DDC-capable display with brightness control.") + .font(.caption) + .foregroundStyle(.secondary) + } + + DisclosureGroup("Advanced") { + ddcAdvancedStepperRow( String( format: NSLocalizedString( "Poll every %d seconds", @@ -479,24 +570,22 @@ struct DisplaySettingsTab: View { ), settings.ddcPollingInterval ) - ) - } - .accessibilityLabel("DDC polling interval") - .accessibilityValue("\(settings.ddcPollingInterval) seconds") - .help("How often to read hardware values from the monitor") - .onChange(of: settings.ddcPollingInterval) { - hardwareManager.applyRuntimeSettings( - controlMode: settings.ddcControlMode, - pollingInterval: settings.ddcPollingInterval, - writeDelayMilliseconds: settings.ddcWriteDelay - ) - if settings.ddcEnabled { - hardwareManager.startPolling() + ) { + Stepper("", value: $settings.ddcPollingInterval, in: 1 ... 30) + .labelsHidden() + .controlSize(.small) + .accessibilityLabel("DDC polling interval") + .accessibilityValue("\(settings.ddcPollingInterval) seconds") + .help("How often to read hardware values from the monitor") + .onChange(of: settings.ddcPollingInterval) { + applyDDCRuntimeSettings(settings: settings, hardwareManager: hardwareManager) + if settings.ddcEnabled { + hardwareManager.startPolling() + } + } } - } - Stepper(value: $settings.ddcWriteDelay, in: 20 ... 200, step: 10) { - Text( + ddcAdvancedStepperRow( String( format: NSLocalizedString( "Write delay: %d ms", @@ -504,17 +593,17 @@ struct DisplaySettingsTab: View { ), settings.ddcWriteDelay ) - ) - } - .accessibilityLabel("DDC write delay") - .accessibilityValue("\(settings.ddcWriteDelay) milliseconds") - .help("Minimum delay between DDC writes (increase if monitor is unresponsive)") - .onChange(of: settings.ddcWriteDelay) { - hardwareManager.applyRuntimeSettings( - controlMode: settings.ddcControlMode, - pollingInterval: settings.ddcPollingInterval, - writeDelayMilliseconds: settings.ddcWriteDelay - ) + ) { + Stepper("", value: $settings.ddcWriteDelay, in: 20 ... 200, step: 10) + .labelsHidden() + .controlSize(.small) + .accessibilityLabel("DDC write delay") + .accessibilityValue("\(settings.ddcWriteDelay) milliseconds") + .help("Minimum delay between DDC writes (increase if monitor is unresponsive)") + .onChange(of: settings.ddcWriteDelay) { + applyDDCRuntimeSettings(settings: settings, hardwareManager: hardwareManager) + } + } } // Per-display DDC status @@ -531,71 +620,54 @@ struct DisplaySettingsTab: View { } } } - - Text( - "DDC/CI controls the monitor's hardware directly over the display cable. " - + "Not all monitors support DDC — built-in HDMI on Apple Silicon Macs, " - + "DisplayLink adapters, and most TVs do not work. " - + "Unsupported displays use software brightness automatically." - ) - .font(.caption) - .foregroundStyle(.secondary) } } header: { Label("Hardware Control", systemImage: "cable.connector.horizontal") } } + private func ddcAdvancedStepperRow( + _ label: String, + @ViewBuilder control: () -> some View + ) -> some View { + HStack(alignment: .center, spacing: 8) { + Text(label) + .lineLimit(1) + .layoutPriority(1) + + control() + } + .frame(maxWidth: .infinity, alignment: .leading) + } + private func ddcDisplayStatusRow( displayID: CGDirectDisplayID, capability: HardwareDisplayCapability ) -> some View { let displayName = brightnessManager.displays.first(where: { $0.id == displayID })?.name ?? "Display \(displayID)" - let features = capability.supportsDDC ? ddcFeatureLabels(for: capability) : "" + let statusText = ddcDisplayStatusText(for: capability) + let supportsHardwareBrightness = capability.supportsDDC && capability.supportsBrightness return HStack(spacing: 6) { - if capability.supportsDDC { - Image(systemName: "checkmark.circle.fill") - .foregroundStyle(.green) - .font(.caption) - } else { - Image(systemName: "tv.and.mediabox") - .foregroundStyle(.secondary) - .font(.caption) - } + Image(systemName: ddcDisplayStatusSymbolName(for: capability)) + .foregroundStyle(supportsHardwareBrightness ? Color.green : Color.secondary) + .font(.caption) Text(displayName) .font(.caption) Spacer() - if capability.supportsDDC { - Text(features) - .font(.caption2) - .foregroundStyle(.secondary) - } else { - Text("Software brightness") - .font(.caption2) - .foregroundStyle(.secondary) - } + Text(statusText) + .font(.caption2) + .foregroundStyle(.secondary) } .accessibilityElement(children: .combine) .accessibilityLabel( - capability.supportsDDC - ? "\(displayName), DDC supported, \(features)" - : "\(displayName), using software brightness" + "\(displayName), \(ddcDisplayAccessibilityStatus(for: capability))" ) } - - private func ddcFeatureLabels(for cap: HardwareDisplayCapability) -> String { - var labels: [String] = [] - if cap.supportsBrightness { labels.append("Brightness") } - if cap.supportsContrast { labels.append("Contrast") } - if cap.supportsVolume { labels.append("Volume") } - if cap.supportsInputSource { labels.append("Input") } - return labels.joined(separator: ", ") - } #endif } diff --git a/DimmerlyTests/AppSettingsTests.swift b/DimmerlyTests/AppSettingsTests.swift index 879fce0..4ec099f 100644 --- a/DimmerlyTests/AppSettingsTests.swift +++ b/DimmerlyTests/AppSettingsTests.swift @@ -91,6 +91,14 @@ final class AppSettingsTests: XCTestCase { settings.ddcPollingInterval = 13 settings.ddcWriteDelay = 120 let manager = HardwareBrightnessManager(forTesting: true, ddcInterface: MockDDCInterface()) + manager.capabilities[25] = HardwareDisplayCapability( + displayID: 25, + supportsDDC: true, + supportedCodes: [.brightness], + maxBrightness: 100, + maxContrast: 0, + maxVolume: 0 + ) applyDDCEnabledChange(true, settings: settings, hardwareManager: manager) manager.stopPolling() @@ -101,6 +109,177 @@ final class AppSettingsTests: XCTestCase { XCTAssertEqual(manager.pollingInterval, 13) XCTAssertEqual(manager.minimumWriteInterval, 0.12, accuracy: 0.001) } + + func testSettingsHardwareModesAreDisabledWhenUnavailable() throws { + let repositoryURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + let settingsViewSourceURL = repositoryURL.appendingPathComponent("Dimmerly/Views/SettingsView.swift") + let source = try String(contentsOf: settingsViewSourceURL, encoding: .utf8) + + XCTAssertTrue( + source.contains(".disabled(!isDDCControlModeAvailable(mode, hardwareManager: hardwareManager))"), + "Hardware and Combined DDC mode picker rows should be disabled when hardware brightness is unavailable" + ) + } + + func testSettingsExplainsWhyHardwareModesAreUnavailable() throws { + let repositoryURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + let settingsViewSourceURL = repositoryURL.appendingPathComponent("Dimmerly/Views/SettingsView.swift") + let source = try String(contentsOf: settingsViewSourceURL, encoding: .utf8) + + XCTAssertTrue( + source.contains("Hardware modes require a DDC-capable display with brightness control."), + "Unavailable Hardware and Combined picker rows should have a nearby explanation" + ) + } + + func testSettingsKeepsAdvancedDDCControlsBehindDisclosure() throws { + let repositoryURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + let settingsViewSourceURL = repositoryURL.appendingPathComponent("Dimmerly/Views/SettingsView.swift") + let source = try String(contentsOf: settingsViewSourceURL, encoding: .utf8) + + XCTAssertTrue( + source.contains("DisclosureGroup(\"Advanced\")"), + "DDC polling and write timing controls should be grouped as advanced settings" + ) + XCTAssertTrue( + source.contains(".help(\"DDC/CI controls"), + "Detailed DDC compatibility caveats should live in help text instead of always-visible copy" + ) + XCTAssertFalse( + source.contains("Text(\n \"DDC/CI controls"), + "The hardware section should avoid a long always-visible DDC compatibility paragraph" + ) + } + + func testAdvancedDDCSteppersUseCompactRows() throws { + let repositoryURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + let settingsViewSourceURL = repositoryURL.appendingPathComponent("Dimmerly/Views/SettingsView.swift") + let source = try String(contentsOf: settingsViewSourceURL, encoding: .utf8) + + XCTAssertTrue( + source.contains("ddcAdvancedStepperRow("), + "Advanced DDC steppers should use compact rows so controls stay visually attached to their labels" + ) + XCTAssertTrue( + source.contains(".labelsHidden()"), + "Compact DDC stepper rows should hide the empty native Stepper label" + ) + XCTAssertTrue( + source.contains(".frame(maxWidth: .infinity, alignment: .leading)"), + "Compact DDC stepper rows should align their contents to the leading edge " + + "instead of the far trailing edge" + ) + } + + func testDDCDisplayStatusPresentationRequiresBrightnessForSuccess() { + let brightnessCapable = HardwareDisplayCapability( + displayID: 25, + supportsDDC: true, + supportedCodes: [.brightness, .volume], + maxBrightness: 100, + maxContrast: 0, + maxVolume: 100 + ) + let partialDDC = HardwareDisplayCapability( + displayID: 26, + supportsDDC: true, + supportedCodes: [.volume, .inputSource], + maxBrightness: 0, + maxContrast: 0, + maxVolume: 100 + ) + let unsupported = HardwareDisplayCapability.notSupported(displayID: 27) + + XCTAssertEqual(ddcDisplayStatusSymbolName(for: brightnessCapable), "checkmark.circle.fill") + XCTAssertEqual(ddcDisplayStatusText(for: brightnessCapable), "Brightness, Volume") + XCTAssertEqual( + ddcDisplayAccessibilityStatus(for: brightnessCapable), + "hardware brightness available, Brightness, Volume" + ) + + XCTAssertEqual(ddcDisplayStatusSymbolName(for: partialDDC), "exclamationmark.circle") + XCTAssertEqual(ddcDisplayStatusText(for: partialDDC), "No hardware brightness (Volume, Input)") + XCTAssertEqual( + ddcDisplayAccessibilityStatus(for: partialDDC), + "DDC available, no hardware brightness, Volume, Input" + ) + + XCTAssertEqual(ddcDisplayStatusSymbolName(for: unsupported), "tv.and.mediabox") + XCTAssertEqual(ddcDisplayStatusText(for: unsupported), "Software brightness") + XCTAssertEqual(ddcDisplayAccessibilityStatus(for: unsupported), "using software brightness") + } + + func testDDCControlModeAvailabilityRequiresEnabledHardwareBrightness() { + let manager = HardwareBrightnessManager(forTesting: true, ddcInterface: MockDDCInterface()) + + XCTAssertTrue(isDDCControlModeAvailable(.softwareOnly, hardwareManager: manager)) + XCTAssertFalse(isDDCControlModeAvailable(.hardwareOnly, hardwareManager: manager)) + XCTAssertFalse(isDDCControlModeAvailable(.combined, hardwareManager: manager)) + + manager.isEnabled = true + manager.capabilities[25] = HardwareDisplayCapability( + displayID: 25, + supportsDDC: true, + supportedCodes: [.volume], + maxBrightness: 0, + maxContrast: 0, + maxVolume: 100 + ) + + XCTAssertFalse(isDDCControlModeAvailable(.hardwareOnly, hardwareManager: manager)) + XCTAssertFalse(isDDCControlModeAvailable(.combined, hardwareManager: manager)) + + manager.capabilities[25] = HardwareDisplayCapability( + displayID: 25, + supportsDDC: true, + supportedCodes: [.brightness], + maxBrightness: 100, + maxContrast: 0, + maxVolume: 0 + ) + + XCTAssertTrue(isDDCControlModeAvailable(.hardwareOnly, hardwareManager: manager)) + XCTAssertTrue(isDDCControlModeAvailable(.combined, hardwareManager: manager)) + + manager.isEnabled = false + XCTAssertFalse(isDDCControlModeAvailable(.hardwareOnly, hardwareManager: manager)) + XCTAssertFalse(isDDCControlModeAvailable(.combined, hardwareManager: manager)) + } + + func testApplyDDCEnabledChangeReappliesSoftwareGammaWhenTurningOff() { + let hardwareManager = HardwareBrightnessManager(forTesting: true) + let brightnessManager = BrightnessManager(forTesting: true) + settings.ddcEnabled = true + hardwareManager.isEnabled = true + brightnessManager.displays = [ + ExternalDisplay(id: 91, name: "External", brightness: 0.42, warmth: 0.0, contrast: 0.5), + ] + var reapplyCount = 0 + brightnessManager.applyGammaHook = { displayID, brightness, _, _ in + guard displayID == 91 else { return } + reapplyCount += 1 + XCTAssertEqual(brightness, 0.42, accuracy: 0.001) + } + + applyDDCEnabledChange( + false, + settings: settings, + hardwareManager: hardwareManager, + brightnessManager: brightnessManager + ) + + XCTAssertFalse(settings.ddcEnabled) + XCTAssertFalse(hardwareManager.isEnabled) + XCTAssertEqual(reapplyCount, 1) + } #endif /// Tests that keyboard shortcut changes are tracked by Observation diff --git a/DimmerlyTests/BrightnessManagerTests.swift b/DimmerlyTests/BrightnessManagerTests.swift index b228a73..c3da633 100644 --- a/DimmerlyTests/BrightnessManagerTests.swift +++ b/DimmerlyTests/BrightnessManagerTests.swift @@ -18,6 +18,7 @@ final class BrightnessManagerTests: XCTestCase { #if !APPSTORE HardwareBrightnessManager.shared.capabilities.removeAll() HardwareBrightnessManager.shared.controlMode = .combined + HardwareBrightnessManager.shared.isEnabled = true #endif } @@ -25,6 +26,7 @@ final class BrightnessManagerTests: XCTestCase { #if !APPSTORE HardwareBrightnessManager.shared.capabilities.removeAll() HardwareBrightnessManager.shared.controlMode = .combined + HardwareBrightnessManager.shared.isEnabled = false #endif bm = nil } @@ -407,6 +409,7 @@ final class BrightnessManagerTests: XCTestCase { maxVolume: 0 ) HardwareBrightnessManager.shared.controlMode = .combined + HardwareBrightnessManager.shared.isEnabled = true let hardwareSynced = expectation(description: "External DDC brightness is updated after animation") let finalGammaApplied = expectation( @@ -454,6 +457,7 @@ final class BrightnessManagerTests: XCTestCase { maxVolume: 0 ) HardwareBrightnessManager.shared.controlMode = .combined + HardwareBrightnessManager.shared.isEnabled = true let gammaApplied = expectation( description: "Warmth update preserves neutral gamma brightness on DDC display" @@ -470,6 +474,37 @@ final class BrightnessManagerTests: XCTestCase { wait(for: [gammaApplied], timeout: 0.1) } + + func testDisabledDDCFallsBackToSoftwareGammaForExternalBrightness() throws { + var external = ExternalDisplay(id: 4, name: "External", brightness: 1.0, warmth: 0.0, contrast: 0.5) + external.supportsDDC = true + bm.displays = [external] + + HardwareBrightnessManager.shared.capabilities[4] = HardwareDisplayCapability( + displayID: 4, + supportsDDC: true, + supportedCodes: [.brightness], + maxBrightness: 100, + maxContrast: 100, + maxVolume: 0 + ) + HardwareBrightnessManager.shared.controlMode = .combined + HardwareBrightnessManager.shared.isEnabled = false + + var hardwareWriteCount = 0 + var gammaBrightness: Double? + bm.setExternalHardwareBrightnessHook = { _, _ in + hardwareWriteCount += 1 + } + bm.applyGammaHook = { _, brightness, _, _ in + gammaBrightness = brightness + } + + bm.setBrightness(for: 4, to: 0.4) + + XCTAssertEqual(hardwareWriteCount, 0) + XCTAssertEqual(try XCTUnwrap(gammaBrightness), 0.4, accuracy: 0.001) + } #endif // MARK: - buildTable diff --git a/DimmerlyTests/MenuBarPanelTests.swift b/DimmerlyTests/MenuBarPanelTests.swift index 3d38bd7..d3a1a1b 100644 --- a/DimmerlyTests/MenuBarPanelTests.swift +++ b/DimmerlyTests/MenuBarPanelTests.swift @@ -122,6 +122,51 @@ final class MenuBarPanelTests: XCTestCase { XCTAssertTrue(gate.shouldPropagateChange()) } + #if !APPSTORE + @MainActor + func testDDCControlsAreHiddenWhenHardwareManagerIsDisabled() { + let displayID: CGDirectDisplayID = 42 + let manager = HardwareBrightnessManager(forTesting: true) + manager.isEnabled = false + manager.capabilities[displayID] = HardwareDisplayCapability( + displayID: displayID, + supportsDDC: true, + supportedCodes: [.brightness, .volume, .audioMute, .inputSource], + maxBrightness: 100, + maxContrast: 100, + maxVolume: 100 + ) + + let row = DisplayBrightnessRow( + display: ExternalDisplay(id: displayID, name: "External", brightness: 0.6), + isBlanked: false, + onChange: { _ in }, + onWarmthChange: { _ in }, + onContrastChange: { _ in }, + onToggleBlank: {} + ) + let wired = row.ddcControls(hardwareManager: manager, displayID: displayID) + + XCTAssertFalse(wired.hasDDC) + XCTAssertNil(wired.onVolumeChange) + XCTAssertNil(wired.onMuteToggle) + XCTAssertNil(wired.onInputSourceChange) + } + #endif + + func testMainShortcutRecorderRequestsFirstResponderWhenRecording() throws { + let sourceURL = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("Dimmerly/Views/KeyboardShortcutRecorder.swift") + let source = try String(contentsOf: sourceURL, encoding: .utf8) + + XCTAssertTrue( + source.contains("nsView.window?.makeFirstResponder(nsView)"), + "Main shortcut recording should make its capture view first responder when recording starts" + ) + } + @MainActor private func drainMainRunLoop(iterations: Int = 12) { for _ in 0 ..< iterations { diff --git a/DimmerlyTests/ScheduleManagerTests.swift b/DimmerlyTests/ScheduleManagerTests.swift index 6664271..9442534 100644 --- a/DimmerlyTests/ScheduleManagerTests.swift +++ b/DimmerlyTests/ScheduleManagerTests.swift @@ -133,6 +133,45 @@ final class ScheduleManagerTests: XCTestCase { XCTAssertEqual(triggerCount, 1, "Should fire once after gap crossing trigger time") } + func testCheckSchedulesCatchesUpAcrossMidnight() { + var firedPresets: [UUID] = [] + manager.onScheduleTriggered = { firedPresets.append($0) } + + let presetID = UUID() + let schedule = makeSchedule(hour: 22, minute: 0, presetID: presetID) + manager.addSchedule(schedule) + + let beforeSleep = makeDate(day: 1, hour: 21, minute: 30) + manager.checkSchedules(now: beforeSleep) + XCTAssertTrue(firedPresets.isEmpty) + + let afterWake = makeDate(day: 2, hour: 7, minute: 0) + manager.checkSchedules(now: afterWake) + + XCTAssertEqual(firedPresets, [presetID], "Should fire yesterday's missed trigger after overnight sleep") + } + + func testCheckSchedulesFiresMissedSchedulesInTriggerOrder() { + var firedPresets: [UUID] = [] + manager.onScheduleTriggered = { firedPresets.append($0) } + + let earlierPreset = UUID() + let laterPreset = UUID() + manager.addSchedule(makeSchedule(name: "Later", hour: 22, minute: 0, presetID: laterPreset)) + manager.addSchedule(makeSchedule(name: "Earlier", hour: 20, minute: 0, presetID: earlierPreset)) + + let before = makeDate(hour: 19, minute: 0) + manager.checkSchedules(now: before) + let after = makeDate(hour: 23, minute: 0) + manager.checkSchedules(now: after) + + XCTAssertEqual( + firedPresets, + [earlierPreset, laterPreset], + "Missed schedules should apply in trigger-time order so the final preset is the latest one" + ) + } + func testDisabledScheduleDoesNotFire() { var triggerCount = 0 manager.onScheduleTriggered = { _ in triggerCount += 1 } From 39791d71b8d31f69230855f2ab1aea4eba6593eb Mon Sep 17 00:00:00 2001 From: Zoran Olujic Date: Fri, 3 Jul 2026 15:05:29 +0200 Subject: [PATCH 2/2] docs: update unreleased changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3256664..f459642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ Semantic Versioning. ## [Unreleased] +### Changed +- Refined Hardware Control settings so unavailable hardware control modes are disabled with clearer fallback guidance. +- Clarified per-display DDC/CI support status and tightened the advanced polling and write-delay controls. + +### Fixed +- Fixed hardware brightness control fallback so DDC/CI control is only used when hardware control is enabled and available. +- Fixed display schedules so missed events between app checks are applied in chronological order. +- Fixed keyboard shortcut recording so the recorder reliably receives focus before capturing keys. + ## [1.0.2] - 2026-07-01 ### Fixed