fix issues with Rog Ally buttons #800
Conversation
WalkthroughThe changes primarily focus on enhancing the LED control capabilities of the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- HandheldCompanion/Devices/ASUS/ROGAlly.cs (5} hunks)
Additional comments: 8
HandheldCompanion/Devices/ASUS/ROGAlly.cs (8)
1-4: The
usingstatements have been changed to non-aliased versions. This change is minor and does not affect the functionality of the code. However, it's important to ensure that the rest of the codebase is updated to reflect this change if necessary.36-48: New constants and byte arrays have been introduced for LED initialization and message setting. These constants and byte arrays are used in the new methods for LED control. The changes are consistent with the PR's objective of adding LED control capabilities.
99-99: The
DeviceCapabilities.LEDControlcapability has been added to theCapabilitiesvariable. This change is consistent with the PR's objective of adding LED control capabilities.220-240: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [223-247]
The
HandleEventmethod has been modified to handle different key events. The changes improve the responsiveness and versatility of theROGAllyclass. However, it's important to ensure that these changes do not introduce any unexpected behavior or side effects.
249-260: The
SetLedStatusmethod has been introduced to set the LED status based on the provided boolean value. The method uses theSetLedBrightnessandSetLedColormethods to set the LED brightness and color respectively. The changes are consistent with the PR's objective of adding LED control capabilities.307-319: The
SetLedColormethod has been introduced to set the LED color. The method uses theApplyColormethod to apply the color settings to the device. The changes are consistent with the PR's objective of adding LED control capabilities.321-338: The
ApplyColormethod has been introduced to apply the color settings to the device. The method uses theHidDeviceclass to write the color settings to the device. The changes are consistent with the PR's objective of adding LED control capabilities.340-357: The
AuraMessagemethod has been introduced to create a byte array representing an Aura message. The method is used in theApplyColormethod to create the message to be written to the device. The changes are consistent with the PR's objective of adding LED control capabilities.
| public override bool SetLedBrightness(int brightness) | ||
| { | ||
| //ROG ALly brightness range is: 0 - 3 range, 0 is off, convert from 0 - 100 % range | ||
| brightness = (int)Math.Round(brightness / 33.33); | ||
|
|
||
| default: | ||
| Task.Run(async () => | ||
| { | ||
| byte[] msg = { AURA_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | ||
| byte[] msgBackup = { INPUT_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | ||
|
|
||
| IEnumerable<HidDevice> devices = GetHidDevices(_vid, _pid); | ||
| foreach (HidDevice device in devices) | ||
| { | ||
| if (device is null || !device.IsConnected) | ||
| return false; | ||
|
|
||
| device.OpenDevice(); | ||
|
|
||
| if (device.ReadFeatureData(out byte[] data, AURA_HID_ID)) | ||
| { | ||
| Task.Run(async () => | ||
| { | ||
| KeyPress(button); | ||
| await Task.Delay(20); | ||
| KeyRelease(button); | ||
| }); | ||
| device.WriteFeatureData(msg); | ||
| } | ||
| break; | ||
| else | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (device.ReadFeatureData(out byte[] dataBackkup, INPUT_HID_ID)) | ||
| { | ||
| device.WriteFeatureData(msgBackup); | ||
| } | ||
| else | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| device.CloseDevice(); | ||
| } | ||
|
|
||
| return true; | ||
| }); | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
The SetLedBrightness method has been introduced to set the LED brightness. The method uses the HidDevice class to write the brightness value to the device. The changes are consistent with the PR's objective of adding LED control capabilities. However, the method returns false at the end, which means it will always return false regardless of whether the operation was successful or not. This should be corrected.
- return false;
+ return true;Committable suggestion (Beta)
| public override bool SetLedBrightness(int brightness) | |
| { | |
| //ROG ALly brightness range is: 0 - 3 range, 0 is off, convert from 0 - 100 % range | |
| brightness = (int)Math.Round(brightness / 33.33); | |
| default: | |
| Task.Run(async () => | |
| { | |
| byte[] msg = { AURA_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | |
| byte[] msgBackup = { INPUT_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | |
| IEnumerable<HidDevice> devices = GetHidDevices(_vid, _pid); | |
| foreach (HidDevice device in devices) | |
| { | |
| if (device is null || !device.IsConnected) | |
| return false; | |
| device.OpenDevice(); | |
| if (device.ReadFeatureData(out byte[] data, AURA_HID_ID)) | |
| { | |
| Task.Run(async () => | |
| { | |
| KeyPress(button); | |
| await Task.Delay(20); | |
| KeyRelease(button); | |
| }); | |
| device.WriteFeatureData(msg); | |
| } | |
| break; | |
| else | |
| { | |
| return false; | |
| } | |
| if (device.ReadFeatureData(out byte[] dataBackkup, INPUT_HID_ID)) | |
| { | |
| device.WriteFeatureData(msgBackup); | |
| } | |
| else | |
| { | |
| return false; | |
| } | |
| device.CloseDevice(); | |
| } | |
| return true; | |
| }); | |
| return false; | |
| } | |
| public override bool SetLedBrightness(int brightness) | |
| { | |
| //ROG ALly brightness range is: 0 - 3 range, 0 is off, convert from 0 - 100 % range | |
| brightness = (int)Math.Round(brightness / 33.33); | |
| Task.Run(async () => | |
| { | |
| byte[] msg = { AURA_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | |
| byte[] msgBackup = { INPUT_HID_ID, 0xba, 0xc5, 0xc4, (byte)brightness }; | |
| IEnumerable<HidDevice> devices = GetHidDevices(_vid, _pid); | |
| foreach (HidDevice device in devices) | |
| { | |
| if (device is null || !device.IsConnected) | |
| return false; | |
| device.OpenDevice(); | |
| if (device.ReadFeatureData(out byte[] data, AURA_HID_ID)) | |
| { | |
| device.WriteFeatureData(msg); | |
| } | |
| else | |
| { | |
| return false; | |
| } | |
| if (device.ReadFeatureData(out byte[] dataBackkup, INPUT_HID_ID)) | |
| { | |
| device.WriteFeatureData(msgBackup); | |
| } | |
| else | |
| { | |
| return false; | |
| } | |
| device.CloseDevice(); | |
| } | |
| return true; | |
| }); | |
| return true; | |
| } |
Summary by CodeRabbit