-
Notifications
You must be signed in to change notification settings - Fork 0
Add toggle command to switch between two input sources #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ntation and examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a new toggle command to the ddcswitch utility, enabling users to automatically switch a monitor between two specified input sources. The command detects the current input and switches to the alternate one, making it ideal for hotkey automation and scripting scenarios.
- Added
ToggleCommandwith input detection, validation, and switching logic - Extended JSON serialization support with
ToggleInputResponserecord - Updated documentation across README, EXAMPLES, and help output
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| DDCSwitch/Commands/ToggleCommand.cs | Implements the new toggle command with comprehensive error handling and both JSON and console output formatting |
| DDCSwitch/Commands/CommandRouter.cs | Integrates the toggle command into the command routing logic |
| DDCSwitch/JsonContext.cs | Adds ToggleInputResponse record for JSON serialization support |
| DDCSwitch/Commands/HelpCommand.cs | Updates help text to document the toggle command with usage examples |
| README.md | Documents toggle command usage, behavior, and use cases for hotkeys/automation |
| EXAMPLES.md | Provides detailed examples including PowerShell scripts and AutoHotkey integration |
| CHANGELOG.md | Documents the new toggle feature for version 1.0.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| else | ||
| { | ||
| ConsoleOutputFormatter.WriteError(errorMessage); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should include a period at the end for consistency with other commands. In GetCommand and SetCommand, the console error output for this message includes a period: "Monitor '{monitorId}' not found." The JSON output correctly omits the period to match the pattern used throughout the codebase.
| return PerformInputSwitch(monitor, currentInput, targetInput, input1Arg, input2Arg, hasWarning, warningMessage, jsonOutput); | ||
| } | ||
|
|
||
| private static int PerformInputSwitch(Monitor monitor, uint currentInput, uint targetInput, string input1Arg, string input2Arg, bool hasWarning, string? warningMessage, bool jsonOutput) |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters input1Arg and input2Arg are unused in this method. The method uses InputSource.GetName(currentInput) and InputSource.GetName(targetInput) to get the display names instead. These parameters should be removed from the method signature.
| } | ||
|
|
||
| // Perform the input switch | ||
| return PerformInputSwitch(monitor, currentInput, targetInput, input1Arg, input2Arg, hasWarning, warningMessage, jsonOutput); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters input1Arg and input2Arg are unused by the PerformInputSwitch method. These should be removed from this call and from the method signature.
| return 1; | ||
| } | ||
|
|
||
| private static int ExecuteToggle(Monitor monitor, uint input1Value, uint input2Value, string input1Arg, string input2Arg, bool jsonOutput) |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters input1Arg and input2Arg are only passed through to PerformInputSwitch where they are unused. Since the method uses InputSource.GetName() with the numeric values instead, these string parameters should be removed from this method signature as well.
| } | ||
|
|
||
| // Execute the toggle operation | ||
| int result = ExecuteToggle(monitor, input1Value, input2Value, input1Arg, input2Arg, jsonOutput); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters input1Arg and input2Arg are unused by ExecuteToggle and its downstream methods. These should be removed from this call and from the method signature.
| } | ||
| else | ||
| { | ||
| ConsoleOutputFormatter.WriteError(errorMessage); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should include a period at the end for consistency with other commands. In GetCommand, SetCommand, and VcpScanCommand, the console error output for this message includes a period: "No DDC/CI capable monitors found." The JSON output correctly omits the period to match the pattern used throughout the codebase.
… better readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach (var feature in features.Values) | ||
| { | ||
| if (feature.IsSupported) | ||
| { | ||
| supportedFeatures.Add(feature); | ||
| } | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (string mfgKey in displayRoot.GetSubKeyNames()) | ||
| { | ||
| using var mfgSubKey = displayRoot.OpenSubKey(mfgKey); | ||
| if (mfgSubKey == null) continue; | ||
|
|
||
| // Try each instance under this manufacturer | ||
| foreach (string instanceKey in mfgSubKey.GetSubKeyNames()) | ||
| { | ||
| using var instanceSubKey = mfgSubKey.OpenSubKey(instanceKey); | ||
| if (instanceSubKey == null) continue; | ||
|
|
||
| // Check if this is an active device | ||
| using var deviceParams = instanceSubKey.OpenSubKey("Device Parameters"); | ||
| if (deviceParams == null) continue; | ||
|
|
||
| // Read EDID data | ||
| var edidData = deviceParams.GetValue("EDID") as byte[]; | ||
| if (edidData != null && edidData.Length >= 128) | ||
| { | ||
| // Validate EDID header | ||
| if (EdidParser.ValidateHeader(edidData)) | ||
| { | ||
| edidList.Add(edidData); | ||
| } | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (string instanceKey in mfgSubKey.GetSubKeyNames()) | ||
| { | ||
| using var instanceSubKey = mfgSubKey.OpenSubKey(instanceKey); | ||
| if (instanceSubKey == null) continue; | ||
|
|
||
| // Check if this is an active device | ||
| using var deviceParams = instanceSubKey.OpenSubKey("Device Parameters"); | ||
| if (deviceParams == null) continue; | ||
|
|
||
| // Read EDID data | ||
| var edidData = deviceParams.GetValue("EDID") as byte[]; | ||
| if (edidData != null && edidData.Length >= 128) | ||
| { | ||
| // Validate EDID header | ||
| if (EdidParser.ValidateHeader(edidData)) | ||
| { | ||
| edidList.Add(edidData); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| if (edidData != null && edidData.Length >= 128) | ||
| { | ||
| // Validate EDID header | ||
| if (EdidParser.ValidateHeader(edidData)) | ||
| { | ||
| edidList.Add(edidData); | ||
| } | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 'if' statements can be combined.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <param name="monitor">Monitor to read current input from</param> | ||
| /// <param name="jsonOutput">Whether to format output as JSON</param> | ||
| /// <param name="currentInput">The detected current input value</param> | ||
| /// <returns>0 on success, 1 on failure</returns> |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation comment states this function returns 0 on success or 1 on failure, but the implementation returns 2 for warning conditions. Update the documentation comment to reflect that it can return 0 (success), 1 (hard error), or 2 (warning with fallback).
| /// <returns>0 on success, 1 on failure</returns> | |
| /// <returns> | |
| /// 0 on success, | |
| /// 1 on hard error, | |
| /// or 2 on warning with fallback. | |
| /// </returns> |
| // Calculate approximate month from week number | ||
| // Week 1-4: January, Week 5-8: February, etc. | ||
| string? monthName = week switch | ||
| { | ||
| >= 1 and <= 4 => "January", | ||
| >= 5 and <= 8 => "February", | ||
| >= 9 and <= 13 => "March", | ||
| >= 14 and <= 17 => "April", | ||
| >= 18 and <= 22 => "May", | ||
| >= 23 and <= 26 => "June", | ||
| >= 27 and <= 30 => "July", | ||
| >= 31 and <= 35 => "August", | ||
| >= 36 and <= 39 => "September", | ||
| >= 40 and <= 43 => "October", | ||
| >= 44 and <= 48 => "November", | ||
| >= 49 and <= 53 => "December", | ||
| _ => null | ||
| }; |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The week-to-month conversion is inaccurate. This logic assumes 4 weeks per month, but months have different numbers of weeks. For example, week 13 would be in late March, not the end of March as implied by "March". ISO 8601 week dates don't align with calendar months. Consider either removing the month approximation and just showing the week number, or using a more accurate calculation based on the actual calendar.
| if (edidData != null && edidData.Length >= 128) | ||
| { | ||
| // Validate EDID header | ||
| if (EdidParser.ValidateHeader(edidData)) | ||
| { | ||
| edidList.Add(edidData); | ||
| } |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 'if' statements can be combined.
| if (edidData != null && edidData.Length >= 128) | |
| { | |
| // Validate EDID header | |
| if (EdidParser.ValidateHeader(edidData)) | |
| { | |
| edidList.Add(edidData); | |
| } | |
| if (edidData != null && edidData.Length >= 128 && EdidParser.ValidateHeader(edidData)) | |
| { | |
| // Valid EDID header | |
| edidList.Add(edidData); |
This pull request adds a new
togglecommand to theddcswitchtool, allowing users to easily switch a monitor between two specified input sources. The documentation and help output are updated to reflect this new feature, including usage examples and integration tips for automation and hotkeys.New Feature: Toggle Command
togglecommand to the command router, enabling users to flip a monitor between two input sources automatically. (CommandRouter.cs)ToggleInputResponserecord for structured JSON output, and registered it in theJsonContextfor serialization. (JsonContext.cs) [1] [2]Documentation Updates
CHANGELOG.mdto document the addition of the toggle command for version 1.0.3.HelpCommand.cs) to include the new toggle command and added example usage lines. [1] [2]EXAMPLES.md.README.mdwith toggle command examples, use cases, and integration scenarios for hotkeys and scripts. [1] [2]