integrate ddcctl submodule and switch to direct DDC#2
integrate ddcctl submodule and switch to direct DDC#2reed-rob wants to merge 1 commit intobase-integrate-ddcctl-r-11722390from
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| let value = abs(self.prefs.integer(forKey: "-b-\(d.serial)") - 1); | ||
|
|
||
| self.prefs.setValue(value, forKey: "-b-\(monitor)"); | ||
| self.prefs.setValue(value, forKey: "-b-\(d.serial))"); |
There was a problem hiding this comment.
Extra closing parenthesis corrupts UserDefaults key
High Severity
The prefs.setValue call on this line uses the key "-b-\(d.serial))" with an extra ) inside the string literal. This means brightness-decrease writes to a different key than brightness-decrease reads from (line 239 uses the correct key "-b-\(d.serial)"), and also mismatches the key used by the brightness slider. The result is that the keyboard shortcut for decreasing brightness will never persist properly — the stored value is never read back.
| brightnessLabelKeyCode.isBordered = false; | ||
| brightnessLabelKeyCode.isBezeled = false; | ||
| brightnessLabelKeyCode.isHidden = i != 1; | ||
| brightnessLabelKeyCode.isHidden = firstDisplay == nil; |
There was a problem hiding this comment.
Inverted condition hides hotkey labels on default display
Medium Severity
The condition firstDisplay == nil is inverted compared to the original i != 1 logic. Since firstDisplay is still nil for the first display (it's assigned at the end of the loop body), this causes keyboard shortcut labels to be hidden on the default display and shown on non-default displays, and the "Default" button to be enabled instead of disabled. The condition needs to be firstDisplay != nil.
Additional Locations (2)
| precondition(false, "Unknown command: \(command)") | ||
| } | ||
|
|
||
| var wrcmd = DDCWriteCommand(control_id: UInt8(cmd), new_value: UInt8(value)) |
There was a problem hiding this comment.
Unbounded hotkey values crash on UInt8 conversion
Medium Severity
The hotkey handlers increment values without clamping (abs(prefs.integer(...) + 1)) and store them back to UserDefaults. These unclamped Int values are passed to ddcctl, which converts them via UInt8(value). Swift's UInt8 initializer traps on overflow, so once the stored value exceeds 255 (e.g., from repeated key presses with key-repeat), the app crashes. The old string-based CLI path had no such crash risk.
Note
Medium Risk
Touches core hardware-control paths (DDC writes) and introduces a native C/ObjC dependency via submodule/bridging; failures could break monitor control or mis-target the wrong display (also note a likely typo in one brightness preference key).
Overview
Switches the macOS app from invoking the external
/usr/local/bin/ddcctlprocess to writing DDC commands directly via the vendoredddcctlcode, wired into Swift through a newBridging-Header.h.Updates display handling to enumerate external monitors via
NSScreen+ EDID, label menu items with the detected display name, and persist brightness/contrast/volume per-monitor using EDID serials (and routes global hotkeys to the first detected external display).Written by Cursor Bugbot for commit b3d900e. This will update automatically on new commits. Configure here.