-
Notifications
You must be signed in to change notification settings - Fork 92
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
Merge EA to public #1035
Merge EA to public #1035
Conversation
WalkthroughThis update brings a myriad of enhancements across the software, focusing on GPU interaction, process and device management, and UI improvements. Notable changes include label adjustments in issue templates, extensive updates to controllers and devices, and new functionalities in process handling and settings. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (17)
Files not summarized due to errors (1)
Additional Context UsedGitHub Check Runs (1)
Additional comments not posted (37)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 10
@@ -34,7 +34,7 @@ private void CheckDrivers() | |||
bool HasXboxDriversInstalled = PlatformManager.Steam.HasXboxDriversInstalled(); | |||
|
|||
// UI thread (async) | |||
Application.Current.Dispatcher.BeginInvoke(() => | |||
Application.Current.Dispatcher.Invoke(() => |
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.
Switching from BeginInvoke
to Invoke
may block the UI thread.
Consider using BeginInvoke
if responsiveness is critical, or ensure that Invoke
does not lead to UI freezes by testing with scenarios of high computational load.
protected override byte[] MapColorValues(int zone, Color color) | ||
{ | ||
if (!DynamicLightingCapabilities.HasFlag(level)) | ||
return false; | ||
|
||
color = MainColor; | ||
|
||
switch (level) | ||
switch (zone) | ||
{ | ||
case LEDLevel.SolidColor: | ||
SetLEDGroupColor(LEDGroup.AYAButton, AYA_ZONES, color); | ||
SetLEDGroupColor(LEDGroup.JoystickBoth, STICK_ZONES, color); | ||
break; | ||
case 1: | ||
return [color.G, color.R, color.B]; | ||
case 2: | ||
return [color.G, color.B, color.R]; | ||
case 3: | ||
return [color.B, color.R, color.G]; | ||
case 4: | ||
return [color.B, color.G, color.R]; | ||
default: | ||
return [color.R, color.G, color.B]; |
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.
New MapColorValues
method for LED color mapping.
The method returns an array with incorrect syntax. Use curly braces {}
instead of square brackets []
for array initialization.
- return [color.G, color.R, color.B];
+ return new byte[] {color.G, color.R, color.B};
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
protected override byte[] MapColorValues(int zone, Color color) | |
{ | |
if (!DynamicLightingCapabilities.HasFlag(level)) | |
return false; | |
color = MainColor; | |
switch (level) | |
switch (zone) | |
{ | |
case LEDLevel.SolidColor: | |
SetLEDGroupColor(LEDGroup.AYAButton, AYA_ZONES, color); | |
SetLEDGroupColor(LEDGroup.JoystickBoth, STICK_ZONES, color); | |
break; | |
case 1: | |
return [color.G, color.R, color.B]; | |
case 2: | |
return [color.G, color.B, color.R]; | |
case 3: | |
return [color.B, color.R, color.G]; | |
case 4: | |
return [color.B, color.G, color.R]; | |
default: | |
return [color.R, color.G, color.B]; | |
protected override byte[] MapColorValues(int zone, Color color) | |
{ | |
switch (zone) | |
{ | |
case 1: | |
return new byte[] {color.G, color.R, color.B}; | |
case 2: | |
return new byte[] {color.G, color.B, color.R}; | |
case 3: | |
return new byte[] {color.B, color.R, color.G}; | |
case 4: | |
return new byte[] {color.B, color.G, color.R}; | |
default: | |
return new byte[] {color.R, color.G, color.B}; |
@@ -28,7 +28,7 @@ private void Steam_Updated(PlatformStatus status) | |||
bool DesktopProfileApplied = PlatformManager.Steam.HasDesktopProfileApplied(); | |||
|
|||
// UI thread (async) | |||
Application.Current.Dispatcher.BeginInvoke(() => | |||
Application.Current.Dispatcher.Invoke(() => |
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.
Switching from BeginInvoke
to Invoke
may block the UI thread.
Consider using BeginInvoke
if responsiveness is critical, or ensure that Invoke
does not lead to UI freezes by testing with scenarios of high computational load.
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterBattery, Properties.Resources.PowerProfileROGAllyBetterBatteryDesc) | ||
{ | ||
Default = true, | ||
DeviceDefault = true, | ||
OEMPowerMode = (int)AsusMode.Silent, | ||
OSPowerMode = OSPowerMode.BetterBattery, | ||
Guid = new("961cc777-2547-4f9d-8174-7d86181b8a7a") | ||
CPUBoostLevel = CPUBoostLevel.Disabled, | ||
Guid = new("961cc777-2547-4f9d-8174-7d86181b8a7a"), | ||
TDPOverrideEnabled = true, | ||
TDPOverrideValues = new[] { 10.0d, 10.0d, 10.0d } | ||
}); | ||
|
||
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyPerformanceName, Properties.Resources.PowerProfileROGAllyPerformanceDescription) | ||
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterPerformance, Properties.Resources.PowerProfileROGAllyBetterPerformanceDesc) | ||
{ | ||
Default = true, | ||
DeviceDefault = true, | ||
OEMPowerMode = (int)AsusMode.Performance, | ||
OSPowerMode = OSPowerMode.BetterPerformance, | ||
Guid = new("3af9B8d9-7c97-431d-ad78-34a8bfea439f") | ||
Guid = new("3af9B8d9-7c97-431d-ad78-34a8bfea439f"), | ||
TDPOverrideEnabled = true, | ||
TDPOverrideValues = new[] { 15.0d, 15.0d, 15.0d } | ||
}); | ||
|
||
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyTurboName, Properties.Resources.PowerProfileROGAllyTurboDescription) | ||
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBestPerformance, Properties.Resources.PowerProfileROGAllyBestPerformanceDesc) | ||
{ | ||
Default = true, | ||
DeviceDefault = true, | ||
OEMPowerMode = (int)AsusMode.Turbo, | ||
OSPowerMode = OSPowerMode.BestPerformance, | ||
Guid = new("ded574b5-45a0-4f42-8737-46345c09c238") | ||
Guid = new("ded574b5-45a0-4f42-8737-46345c09c238"), | ||
TDPOverrideEnabled = true, | ||
TDPOverrideValues = new[] { 25.0d, 25.0d, 25.0d } |
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.
Consider refactoring the power profiles to avoid duplication of default values across profiles, enhancing maintainability.
- Default = true,
- DeviceDefault = true,
+ Default = this.IsFirstProfile(),
+ DeviceDefault = this.IsFirstProfile(),
Add a method IsFirstProfile()
to determine if a profile is the first one being added, thus maintaining the intent but reducing redundancy.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterBattery, Properties.Resources.PowerProfileROGAllyBetterBatteryDesc) | |
{ | |
Default = true, | |
DeviceDefault = true, | |
OEMPowerMode = (int)AsusMode.Silent, | |
OSPowerMode = OSPowerMode.BetterBattery, | |
Guid = new("961cc777-2547-4f9d-8174-7d86181b8a7a") | |
CPUBoostLevel = CPUBoostLevel.Disabled, | |
Guid = new("961cc777-2547-4f9d-8174-7d86181b8a7a"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 10.0d, 10.0d, 10.0d } | |
}); | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyPerformanceName, Properties.Resources.PowerProfileROGAllyPerformanceDescription) | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterPerformance, Properties.Resources.PowerProfileROGAllyBetterPerformanceDesc) | |
{ | |
Default = true, | |
DeviceDefault = true, | |
OEMPowerMode = (int)AsusMode.Performance, | |
OSPowerMode = OSPowerMode.BetterPerformance, | |
Guid = new("3af9B8d9-7c97-431d-ad78-34a8bfea439f") | |
Guid = new("3af9B8d9-7c97-431d-ad78-34a8bfea439f"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 15.0d, 15.0d, 15.0d } | |
}); | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyTurboName, Properties.Resources.PowerProfileROGAllyTurboDescription) | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBestPerformance, Properties.Resources.PowerProfileROGAllyBestPerformanceDesc) | |
{ | |
Default = true, | |
DeviceDefault = true, | |
OEMPowerMode = (int)AsusMode.Turbo, | |
OSPowerMode = OSPowerMode.BestPerformance, | |
Guid = new("ded574b5-45a0-4f42-8737-46345c09c238") | |
Guid = new("ded574b5-45a0-4f42-8737-46345c09c238"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 25.0d, 25.0d, 25.0d } | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterBattery, Properties.Resources.PowerProfileROGAllyBetterBatteryDesc) | |
{ | |
Default = this.IsFirstProfile(), | |
DeviceDefault = this.IsFirstProfile(), | |
OEMPowerMode = (int)AsusMode.Silent, | |
OSPowerMode = OSPowerMode.BetterBattery, | |
CPUBoostLevel = CPUBoostLevel.Disabled, | |
Guid = new("961cc777-2547-4f9d-8174-7d86181b8a7a"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 10.0d, 10.0d, 10.0d } | |
}); | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBetterPerformance, Properties.Resources.PowerProfileROGAllyBetterPerformanceDesc) | |
{ | |
Default = this.IsFirstProfile(), | |
DeviceDefault = this.IsFirstProfile(), | |
OEMPowerMode = (int)AsusMode.Performance, | |
OSPowerMode = OSPowerMode.BetterPerformance, | |
Guid = new("3af9B8d9-7c97-431d-ad78-34a8bfea439f"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 15.0d, 15.0d, 15.0d } | |
}); | |
DevicePowerProfiles.Add(new(Properties.Resources.PowerProfileROGAllyBestPerformance, Properties.Resources.PowerProfileROGAllyBestPerformanceDesc) | |
{ | |
Default = this.IsFirstProfile(), | |
DeviceDefault = this.IsFirstProfile(), | |
OEMPowerMode = (int)AsusMode.Turbo, | |
OSPowerMode = OSPowerMode.BestPerformance, | |
Guid = new("ded574b5-45a0-4f42-8737-46345c09c238"), | |
TDPOverrideEnabled = true, | |
TDPOverrideValues = new[] { 25.0d, 25.0d, 25.0d } |
HandheldCompanion/DSU/DSUServer.cs
Outdated
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4); | ||
outIdx += 4; | ||
// accelYG | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer.Y), 0, outputData, outIdx, 4); | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4); | ||
outIdx += 4; | ||
// accelZG | ||
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Accelerometer.Z), 0, outputData, outIdx, 4); | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4); | ||
outIdx += 4; | ||
|
||
// Gyroscope | ||
// angVelPitch | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope.X), 0, outputData, outIdx, 4); | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4); | ||
outIdx += 4; | ||
// angVelYaw | ||
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope.Y), 0, outputData, outIdx, 4); | ||
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4); | ||
outIdx += 4; | ||
// angVelRoll | ||
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope.Z), 0, outputData, outIdx, 4); | ||
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4); |
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.
Refactor to use a method for accessing Accelerometer
and Gyroscope
data to reduce duplication and improve maintainability.
- Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4);
- Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4);
- Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4);
- Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4);
- Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4);
- Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4);
+ private void CopySensorDataToOutput(byte[] outputData, ref int outIdx, SensorData sensorData)
+ {
+ Array.Copy(BitConverter.GetBytes(sensorData.X), 0, outputData, outIdx, 4);
+ outIdx += 4;
+ Array.Copy(BitConverter.GetBytes(sensorData.Y), 0, outputData, outIdx, 4);
+ outIdx += 4;
+ Array.Copy(BitConverter.GetBytes(sensorData.Z), 0, outputData, outIdx, 4);
+ outIdx += 4;
+ }
+ CopySensorDataToOutput(outputData, ref outIdx, Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU]);
+ CopySensorDataToOutput(outputData, ref outIdx, Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU]);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
// accelYG | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer.Y), 0, outputData, outIdx, 4); | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
// accelZG | |
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Accelerometer.Z), 0, outputData, outIdx, 4); | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
// Gyroscope | |
// angVelPitch | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope.X), 0, outputData, outIdx, 4); | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].X), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
// angVelYaw | |
Array.Copy(BitConverter.GetBytes(Inputs.GyroState.Gyroscope.Y), 0, outputData, outIdx, 4); | |
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Y), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
// angVelRoll | |
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope.Z), 0, outputData, outIdx, 4); | |
Array.Copy(BitConverter.GetBytes(-Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU].Z), 0, outputData, outIdx, 4); | |
private void CopySensorDataToOutput(byte[] outputData, ref int outIdx, SensorData sensorData) | |
{ | |
Array.Copy(BitConverter.GetBytes(sensorData.X), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
Array.Copy(BitConverter.GetBytes(sensorData.Y), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
Array.Copy(BitConverter.GetBytes(sensorData.Z), 0, outputData, outIdx, 4); | |
outIdx += 4; | |
} | |
CopySensorDataToOutput(outputData, ref outIdx, Inputs.GyroState.Accelerometer[GyroState.SensorState.DSU]); | |
CopySensorDataToOutput(outputData, ref outIdx, Inputs.GyroState.Gyroscope[GyroState.SensorState.DSU]); |
HandheldCompanion.iss
Outdated
//TODO - Add firewall entry | ||
end; |
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.
Add the necessary firewall entry as indicated by the TODO comment.
Would you like assistance in implementing the firewall entry code? If so, I can help draft the necessary script or configuration.
@@ -664,7 +671,7 @@ 19. Triggers | |||
SendHidControlWrite(flushBufferWriteChanges); // 16 | |||
|
|||
// Choose the appropriate mapping based on the 'Remap' flag | |||
SendHidControlWrite(Remap ? M1F17M2F18 : M1M2Default); // Step 17 | |||
SendHidControlWrite(Remap ? M1F18M2F17 : M1M2Default); // Step 17 |
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.
Tip
Codebase Verification
The 'Remap' flag is minimally documented in the codebase, as indicated by a single comment describing its use in HID control writes. However, there is no evidence of dedicated testing for this flag, which is a concern for ensuring its correct implementation and functionality. It is recommended to add comprehensive tests to verify the behavior of the 'Remap' flag under various conditions.
- Documentation found:
HandheldCompanion/Devices/ASUS/ROGAlly.cs
(Line 674) - Testing: No test cases involving the 'Remap' flag were found.
Analysis chain
Ensure the 'Remap' flag is well-documented and tested to confirm its impact on device configuration.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of 'Remap' flag in the codebase to ensure it's handled correctly.
rg --type cs 'bool Remap' --context 5
Length of output: 691
Script:
#!/bin/bash
# Search for comments related to the 'Remap' flag in the codebase
rg --type cs '//.*Remap.*' --context 2
# Search for test cases involving the 'Remap' flag
rg --type cs 'Remap' --files-with-matches | grep 'Tests'
Length of output: 517
protected FontFamily GlyphFontFamily = new("PromptFont"); | ||
public static readonly string defaultGlyph = "\u2753"; |
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.
Tip
Codebase Verification
Several files in the codebase reference the font "PromptFont," but it does not appear to be included in the project resources based on the search results in XML and JSON files. This could potentially lead to runtime errors or fallback to default fonts as the font might not be available at runtime.
HandheldCompanion/Managers/Hotkeys/InputsHotkey.cs
HandheldCompanion/Devices/IDevice.cs
HandheldCompanion/Controllers/IController.xaml.cs
Analysis chain
Ensure that the font "PromptFont" is included in the project resources or is a system font. Missing font resources can lead to runtime errors or fallback to default fonts.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the font "PromptFont" is included in the project resources
rg --type cs "PromptFont"
Length of output: 402
Script:
#!/bin/bash
# Search for resource files or configurations that might include font definitions
rg --type xml --type json "PromptFont"
Length of output: 38
protected double VibrationStrength = 1.0d; | ||
private byte _UserIndex = 255; | ||
private readonly int MaxUserIndex = 10; |
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.
Tip
Codebase Verification
The review comment raises a concern about the handling of _UserIndex
initialized to 255, which is used as a sentinel value. The script output confirms that _UserIndex
is accessed and modified within its class but does not show any special handling or conditions for the sentinel value of 255. This could potentially lead to unintended behavior if other parts of the code rely on this value without proper checks.
HandheldCompanion/Controllers/IController.xaml.cs
: The_UserIndex
is used in getter and setter methods without conditions checking for the sentinel value.
Analysis chain
The _UserIndex
is initialized to 255, which seems to be used as a sentinel value. Ensure that this value is handled appropriately in all relevant parts of the code to avoid unintended behavior.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of `_UserIndex` to ensure it's handled properly
rg --type cs "_UserIndex"
Length of output: 285
ProgressBarWarning.Visibility = value ? Visibility.Visible : Visibility.Collapsed; | ||
}); | ||
|
||
switch (value) | ||
{ | ||
case false: | ||
{ | ||
// kill working thread | ||
if (workingThread is not null) | ||
{ | ||
workingThreadRunning = false; | ||
// Ensure the thread has finished execution | ||
if (workingThread.IsAlive) | ||
workingThread.Join(); | ||
workingThread = null; |
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 thread management logic here is risky. Specifically, the use of Thread.Join
without a timeout can lead to a deadlock if the thread does not terminate as expected. Consider using Thread.Join(TimeSpan)
with a reasonable timeout.
- workingThread.Join();
+ workingThread.Join(TimeSpan.FromSeconds(5)); // Timeout after 5 seconds
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
ProgressBarWarning.Visibility = value ? Visibility.Visible : Visibility.Collapsed; | |
}); | |
switch (value) | |
{ | |
case false: | |
{ | |
// kill working thread | |
if (workingThread is not null) | |
{ | |
workingThreadRunning = false; | |
// Ensure the thread has finished execution | |
if (workingThread.IsAlive) | |
workingThread.Join(); | |
workingThread = null; | |
ProgressBarWarning.Visibility = value ? Visibility.Visible : Visibility.Collapsed; | |
}); | |
switch (value) | |
{ | |
case false: | |
{ | |
// kill working thread | |
if (workingThread is not null) | |
{ | |
workingThreadRunning = false; | |
// Ensure the thread has finished execution | |
if (workingThread.IsAlive) | |
workingThread.Join(TimeSpan.FromSeconds(5)); // Timeout after 5 seconds | |
workingThread = null |
* Disable the RealTimeStylus for WPF Applications * Update MainWindow.xaml.cs * test ?
…#158) * Start implementing IGCL (wip) * More work - Implemented GPUManager - Implemented GPU-specific classes (AMDGPU, IntelGPU) - Implemented IGCLBackend (wip) - SystemManager renamed to MultimediaManager - PowerManager renamed to SystemManager * more work on IGCL * prevent crash on null MainThread * prevent useless SetResolution() calls * more work on IGCL * add missing sharpness check * implement ctl_device_adapter_properties_t (wip) * what if the issue was deviceIdx all along... * Update IGCL_Wrapper.dll * fix remaining implementations * implement IntegerScalingType (Intel only) * make sure to use defaultGPU (idx: 0) We need to find a proper way to guess which one is used for 3D rendering I guess or linked to main screen.. * fix ctl_device_adapter_properties_t Marshalling * implemented some form of logic to pick the first available external GPU (if any) * improve GPUManager - add support for Manufacturer: "Advanced Micro Devices, Inc." - improve GPUManager and GPU Start() and Stop() logics - prevent Task Execution within Tasks on AMDGPU * fix a crash when UpdateTimer is null
* Implement new UI classes - UISounds to manage UI sounds on interaction. - UIGamepad to manage gamepad interactions. - Audio files from https://kenney.nl/assets/ui-audio. - Add support for TextBox and RepeatButton selection via gamepad. * Update HandheldCompanion/UI/UISounds.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix PlayOggFile refs * removed unused audio files * Add UI Sounds toggle on SettingsPage (default Off) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Migrate everything ADLX related to ADLX_Wrapper() * update IGCL logic - Implemented Terminate() and Initialize() as well as GetTelemetryData() * debug functions on both IGCL and ADLX backends * Update ADLX_Wrapper and fix Initialize() calls on GPU classes * add Telemetry Timer as part of GPU class * Implement GPU GetLoad() and GetPower()
Fixing the rare case of soft-brick issue on uninstallation caused by a race condition
…e#186) * early wip * more work * improve DesktopScreen GetResolution() logic * implement GPUManager Hooked event * prevent crash on null gpu variable * improve performance manager locks logic * more work on ADLX - Implemented displayIdx logic - Renamed a few ADLXBackend arguments for clarity - Leveraging WindowsDisplayAPI library to link a display DisplayName with its FriendlyName (which is reported by ADLX - Now storing FriendlyName and DevicePath on DesktopScreen * add new functions to MultimediaManager - GetDisplayFriendlyName() - GetDisplayPath() - GetDisplayTarget()
* move a few calls away from MainWindow * improve main window loading experience * halt and resume GPU manager on sleep/resume * suspend/resume LibreHardwareMonitor with system * check IGCL/ADLX status before trying to terminate() * migrate GPU wait to GPUManager * mark GPU as Halting on system stop/sleep Prevents any further ADLX/IGCL calls while GPU is halting * Fixes Valkirie#990 * misc edit to VirtualManager and vTargets * Improved management of controller-specific parameters - Implemented IController UpdateSettings() - Implemented UpdateSettings() supports over LegionController and SteamController * improve lock logic on GPU * revert GPU halting logic I need to learn about Semaphore here to lock and release from different thread. * implement CrossThreadLock class - used by GPU * delay system until we're done ! * fix usage of CrossThreadLock within GPU * fix DesktopScreen GetResolution() That's dirty :'( * Support for Ayaneo Slide Support for Ayaneo Slide --------- Co-authored-by: DevL0rd <dmhzmxn@gmail.com>
* MVVM Rework - Batch 1 * Fix merge mistake. * update IController - make TargetButtons, TargetAxis, SourceAxis, SourceButtons non static * feed SourceButtons, SourceAxis on IController creation * Corrected inconsistencies in the use of Environment.ProcessorCount and MotherboardInfo.NumberOfCores --------- Co-authored-by: Matthias Seys <matthiasseys@gmail.com> Co-authored-by: Lesueur Benjamin <benjamin.lesueur@live.com>
* Testing left gyro * start implementing per-controller gyro support * improve the automatic sensor switching logic - when a controller is targeted, use its sensors if available - when unplugged, if controller had sensors, pick next available - if a controller is plugged and doesn't have sensors, pick next available * implement LegionControllerGyroIndex on DevicePage * fix type on gZ
* Testing left gyro * start implementing per-controller gyro support * improve the automatic sensor switching logic - when a controller is targeted, use its sensors if available - when unplugged, if controller had sensors, pick next available - if a controller is plugged and doesn't have sensors, pick next available * implement LegionControllerGyroIndex on DevicePage * fix type on gZ * migrate sensor filtering * improve filtering logic * Tentative * it's working * remove deprecated filtering * sorting all axis * more work on axis, UI and calibration - still getting awkward results when testing PadTest (Cemuhook) * remove deprecated FilterMotion() function * more work on gyro aiming - implemented local space, player space and world space * add missing glyph for MotionInput.LocalSpace * fix inclination * use GetGravity() instead of GetProcessedAcceleration() * restore normal inclination now that we're using GetGravity instead of GetProcessedAcceleration * fix legion controller gyro/accelero computation * improve overall motion experience - remove hardcoded motion delta, use TimerManager event to pass double delta value. - implement SensorsManager ProcessReport() function * remove useless argument from SensorsManager UpdateReport() * Make TimerManager delta float - One cast instead of several. * remove MadgwickAHRS - migrate ToEulerAngles to InputUtils * compile GamepadMotionHelper in Release * rename Legion Controller * implement IMUCalibration - SensorsManager can store and retrieve calibration offset data from GamepadMotion objects * fix incorrect GamepadMotion variable when using Internal IMU * Implement device IMU calibration * set calibration button style to accent * prevent crash on null gamepadMotion * move motion processing to Motion Manager * remove redundant switch * more work on motion manager - fixed motion invert (horizontal, vertical) - fixed steering - now properly swapping (Roll, Yaw) on LocalSpace
* InnoSetup Update Update InnoSetup with the following: - Install dependencies only when not installed or newer version comes with installer - Show optional reboot screen only when required - After installing HidHide, remove desktop icon - Option to open the application after install * update HidHide and RTSS redist Files were already pushed to mainline * update NewHidHideVersion --------- Co-authored-by: Lesueur Benjamin <benjamin.lesueur@live.com>
* Support for MSI Claw Adds support for Claw and Quick Settings buttons Adds orientation for IMU Adds TDP and clock information Updated PromptFont with MSI Claw glyphs * fix gyro axis * force device to XInput mode on startup (poc) * fix CPU power reading on Intel CPUs * fix incorrect accelerometer axis * add default power profiles * implementing hint (WIP) * implement hint * prevent crash if ContentFrame can't go back on power profile delete * fix MSI power profiles naming * rename OEM power presets variable names * change profile CPUBoostLevel default value to 1:Enabled --------- Co-authored-by: romracer <romracer@localhost> Co-authored-by: Lesueur Benjamin <benjamin.lesueur@live.com>
- if main sensor is internal/external, swap axis to Yaw. - if main sensor is controller, compare accelZ and accelY to swap axis.
* prevent GPU Execute() overlapping * prevent WinEventProc (FOREGROUND) from locking UI thread
This is the same code as the previous PR (Valkirie#236) but I noticed the filesize was bigger. Inspecting it revealed it was a DLL (GUI) (whatever that is) versus a DLL (console). I was also getting access violation exceptions that I would not expect. I re-compiled it from the VS Developer Command Prompt instead of the GUI. The filesize now more closely matches the previous file and it no longer reports as DLL (GUI). I have also not been able to reproduce the access violation exceptions..
This syncs the DLL with Valkirie/ADLX-SDK-Wrapper#6 Specifically, it reverts the strncpy change from Valkirie/ADLX-SDK-Wrapper#5
* Actuall set CurrentTDP * Actually set CurrentTDP at some point
* Use correct type for custom osd level settings * fix Settings.Designer.cs We need to make sure we're using our custom SettingsProvider --------- Co-authored-by: Lesueur Benjamin <benjamin.lesueur@live.com>
* Hold RGB control while running * Remove Ayaneo PowerStatusChanged, control makes it obsolete --------- Co-authored-by: CasperH2O <CasperH2O@users.noreply.github.com>
* Cache properties Only ProcessorMaxTurboSpeed is not cached now, but its queried from IDevice anyway. * Use separate caches for easier deserialization * Additional MotherboardInfo changes * Fix duplicate cache keys * simplify MotherboardInfo caching logic * prevent crash on null processor --------- Co-authored-by: Lesueur Benjamin <benjamin.lesueur@live.com>
* Update path and check for newer HidHide versions * Update the version string for RTSS and flag it as reboot required
d5be87d
to
789f750
Compare
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.
Actionable comments posted: 6
// avoid locking UI thread | ||
new Thread(() => { ForegroundCallback(); }).Start(); |
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.
Consider using Task.Run
instead of new Thread
for better resource management.
Using Task.Run
instead of new Thread
can lead to better thread pool management and potentially improved performance. Task.Run
uses the .NET thread pool, which is optimized for short-running compute-bound operations and can efficiently handle a large number of small tasks by maintaining a pool of threads that are reused.
- new Thread(() => { ForegroundCallback(); }).Start();
+ Task.Run(() => { ForegroundCallback(); });
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// avoid locking UI thread | |
new Thread(() => { ForegroundCallback(); }).Start(); | |
// avoid locking UI thread | |
Task.Run(() => { ForegroundCallback(); }); |
{ | ||
processor.SetTDPLimit(type, TDP); | ||
CurrentTDP[idx] = TDP; | ||
} |
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.
Ensure thread safety when updating CurrentTDP
array.
The code modifies the CurrentTDP
array within a loop that potentially runs on multiple threads due to the asynchronous nature of cpuWatchdog_Elapsed
. This could lead to race conditions. Consider using thread-safe operations or synchronization mechanisms when updating shared data.
if (processor is not null && processor.IsInitialized) | ||
{ | ||
processor.StatusChanged += Processor_StatusChanged; | ||
processor.Initialize(); | ||
} | ||
|
||
// deprecated | ||
/* | ||
processor.ValueChanged += Processor_ValueChanged; | ||
processor.LimitChanged += Processor_LimitChanged; | ||
processor.MiscChanged += Processor_MiscChanged; | ||
*/ | ||
else | ||
{ | ||
ProcessorStatusChanged?.Invoke(false, false); | ||
} |
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.
Handle potential null reference in processor initialization.
The method Start
checks if processor
is not null and is initialized before attaching event handlers and calling Initialize
. However, there's a potential for processor
to be null or uninitialized when ProcessorStatusChanged
is invoked immediately after. This could lead to a null reference exception. Ensure that processor
is properly checked before invoking any methods or accessing its properties.
private void ProfileManager_Deleted(Profile profile) | ||
{ | ||
// update current process | ||
currentProcess = processEx; | ||
|
||
// update real profile | ||
realProfile = ProfileManager.GetProfileFromPath(processEx.Path, true); | ||
// this shouldn't happen, someone removed the currently applied profile | ||
if (selectedProfile == profile) | ||
ProcessManager_ForegroundChanged(currentProcess, null); | ||
} |
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.
Remove duplicate method definition for ProfileManager_Deleted
.
- private void ProfileManager_Deleted(Profile profile)
- {
- // this shouldn't happen, someone removed the currently applied profile
- if (selectedProfile == profile)
- ProcessManager_ForegroundChanged(currentProcess, null);
- }
This method is defined twice in the file, which will cause a compilation error. The duplicate definition should be removed to resolve this issue.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
private void ProfileManager_Deleted(Profile profile) | |
{ | |
// update current process | |
currentProcess = processEx; | |
// update real profile | |
realProfile = ProfileManager.GetProfileFromPath(processEx.Path, true); | |
// this shouldn't happen, someone removed the currently applied profile | |
if (selectedProfile == profile) | |
ProcessManager_ForegroundChanged(currentProcess, null); | |
} |
} | ||
|
||
// no code from the cases inside this function will be called on program start | ||
private async void OnSystemStatusChanged(PowerManager.SystemStatus status, PowerManager.SystemStatus prevStatus) |
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.
Add the necessary using directive or reference for PowerManager
to resolve the compilation error.
@@ -49,7 +49,8 @@ | |||
#define NewDirectXVersion "9.29.1974" | |||
#define NewViGemVersion "1.22.0.0" | |||
#define NewHidHideVersion "1.5.212" | |||
#define NewRtssVersion "7.3.6" | |||
; RTSS 7.3.6 | |||
#define NewRtssVersion "7.3.5.28010" |
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 RTSS version number is inconsistent with the defined constant.
- #define NewRtssVersion "7.3.5.28010"
+ #define NewRtssVersion "7.3.6"
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
#define NewRtssVersion "7.3.5.28010" | |
#define NewRtssVersion "7.3.6" |
Summary by CodeRabbit
ADLXBackend
.