Update ControllerManager.cs #872
Conversation
WalkthroughThe update involves modifying the Changes
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 X ? 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: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- HandheldCompanion/Managers/ControllerManager.cs (5 hunks)
Additional comments: 3
HandheldCompanion/Managers/ControllerManager.cs (3)
59-62: The
Startmethod has been updated to return aTask. Ensure that all invocations ofStartare updated to handle the new return type, especially if they are awaited.371-386: The block of code that sets
settings.playerNumberandjoyShockTypeis only executed ifjoyShockIdis not-1. This is a good use of a guard clause to prevent further execution if no JoyShock device is found. However, ensure that there is appropriate handling or logging if no device is found (joyShockIdremains-1).388-407: The creation of controller instances based on
joyShockTypeis done within the UI thread usingApplication.Current.Dispatcher.Invoke. This is a good practice as it ensures that UI-related operations are performed on the UI thread. However, ensure that thecontrollervariable is correctly updated and accessible outside the dispatcher invoke block.
| int connectedJoys = -1; | ||
| int joyShockId = -1; | ||
| JOY_SETTINGS settings = new(); | ||
|
|
||
| DateTime timeout = DateTime.Now.Add(TimeSpan.FromSeconds(4)); |
There was a problem hiding this comment.
Initialization of connectedJoys to -1 and the subsequent loop to connect devices may result in an infinite loop if JslConnectDevices() throws an exception that is not caught. Ensure that exceptions are handled properly to avoid an infinite loop.
| } | ||
| } | ||
|
|
||
| if (connectedJoys >= 0) | ||
| if (connectedJoys > 0) | ||
| { | ||
| int[] joysHandle = new int[connectedJoys]; | ||
| JslGetConnectedDeviceHandles(joysHandle, connectedJoys); | ||
|
|
||
| // scroll handles until we find matching device path | ||
| int joyShockId = -1; | ||
| JOY_SETTINGS settings = new(); | ||
|
|
||
| foreach (int i in joysHandle) | ||
| { | ||
| settings = JslGetControllerInfoAndSettings(i); |
There was a problem hiding this comment.
Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [350-369]
The check if (connectedJoys > 0) ensures that the following logic is only executed if there are connected devices. However, there is no else block to handle the case when there are no connected devices. Consider adding logic to handle this case appropriately.
Summary by CodeRabbit