Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 123 additions & 22 deletions AquaMai.Mods/GameSystem/AdxHidInput.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.IO.Pipes;
using System.Linq;
Expand Down Expand Up @@ -31,22 +31,121 @@ public class AdxHidInput
private static byte[,] inputBufPending = new byte[2, 32];
private static double[] td = [0, 0];
private static bool tdEnabled, keyEnabled, pipeEnabled;
private static bool[] hidThreadRunning = [false, false];

private static bool TryConnectDevice(int p)
{
var device = p == 0
? HidDevices.Enumerate(0x2E3C, [0x5750, 0x5767]).FirstOrDefault(it => !it.DevicePath.EndsWith("kbd"))
: HidDevices.Enumerate(0x2E4C, 0x5750).Concat(HidDevices.Enumerate(0x2E3C, 0x5768)).FirstOrDefault(it => !it.DevicePath.EndsWith("kbd"));
Comment on lines +38 to +40

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Vendor IDs (e.g., 0x2E3C) and Product IDs (e.g., 0x5750) are hardcoded as magic numbers. Using named constants for these values would significantly improve code readability and maintainability. I recommend defining constants for these IDs at the class level and using them here and in the NeedsButtonInput method.


if (device == null) return false;

adxController[p] = device;
device.OpenDevice();
MelonLogger.Msg($"[HidInput] Device {p + 1}P connected");

return true;
}

private static bool IsDeviceAvailable(int p)
{
var device = adxController[p];
if (device == null) return false;

try
{
return device.IsConnected;
}
catch
{
return false;
}
Comment on lines +56 to +63

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Catching a generic Exception and silently returning false can hide unexpected errors and make debugging difficult. It's better to catch a more specific exception if the HidLibrary provides one. If not, at least log the exception to aid in troubleshooting.

        try
        {
            return device.IsConnected;
        }
        catch (Exception e)
        {
            MelonLogger.Warning($"[HidInput] Error checking device {p + 1}P connection: {e.Message}");
            return false;
        }

}

private static void DisconnectDevice(int p)
{
var device = adxController[p];
if (device == null) return;

try
{
device.CloseDevice();
}
catch
{
// ignore
}
Comment on lines +71 to +78

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to other try-catch blocks in this file, catching a generic Exception is not ideal. Even if the intention is to ignore the error (as the device is likely disconnected), logging the exception can be valuable for debugging unexpected behavior.

        try
        {
            device.CloseDevice();
        }
        catch (Exception e)
        {
            // It's likely the device was already disconnected, but we log it just in case.
            MelonLogger.Warning($"[HidInput] Error closing device {p + 1}P: {e.Message}");
        }


adxController[p] = null;

for (int i = 0; i < 32; i++)
{
inputBuf[p, i] = 0;
inputBufPending[p, i] = 0;
}

MelonLogger.Msg($"[HidInput] Device {p + 1}P disconnected");
}

private static bool NeedsButtonInput(int p)
{
var device = adxController[p];
if (device == null) return false;
try
{
return device.Attributes.ProductId is not (0x5767 or 0x5768);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These hardcoded product IDs (0x5767, 0x5768) should be replaced with named constants for better readability and maintainability, as mentioned in the comment for the TryConnectDevice method.

}
catch
{
return false;
}
Comment on lines +95 to +102

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This try-catch block catches a generic Exception and silently returns false, which can mask underlying problems. It's better to log the exception for debugging purposes, similar to the suggestions for IsDeviceAvailable and DisconnectDevice.

        try
        {
            return device.Attributes.ProductId is not (0x5767 or 0x5768);
        }
        catch (Exception e)
        {
            MelonLogger.Warning($"[HidInput] Error reading attributes for device {p + 1}P: {e.Message}");
            return false;
        }

}

private static void HidInputThread(int p)
{
while (true)
{
if (adxController[p] == null) return;
var report1P = adxController[p].Read();
if (report1P.Status != HidDeviceData.ReadStatus.Success || report1P.Data.Length <= 13) continue;
for (int i = 0; i < 14; i++)
while (!IsDeviceAvailable(p))
{
Thread.Sleep(500);
TryConnectDevice(p);
}

if (!NeedsButtonInput(p))
{
var newState = report1P.Data[i];
if (newState == 1 && inputBuf[p, i] == 0)
Thread.Sleep(500);
continue;
}
Comment on lines +115 to +119

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When a connected device does not require button input, the thread enters a polling loop, sleeping for 500ms and then re-checking the device status. This is inefficient and consumes CPU resources unnecessarily. Consider increasing the sleep duration to reduce the polling frequency and lower the performance impact.

            if (!NeedsButtonInput(p))
            {
                Thread.Sleep(2000); // Increase sleep time to reduce CPU usage
                continue;
            }


var device = adxController[p];
if (device == null) continue;

try
{
var report = device.Read();

if (report.Status != HidDeviceData.ReadStatus.Success)
{
DisconnectDevice(p);
continue;
}

if (report.Data.Length <= 13) continue;

for (int i = 0; i < 14; i++)
{
inputBufPending[p, i] = 1;
var newState = report.Data[i];
if (newState == 1 && inputBuf[p, i] == 0)
{
inputBufPending[p, i] = 1;
}
inputBuf[p, i] = newState;
}
inputBuf[p, i] = newState;
}
catch
{
DisconnectDevice(p);
}
}
}
Expand Down Expand Up @@ -99,28 +198,30 @@ private static void TdInit(int p)

public static void OnBeforeEnableCheck()
{
adxController[0] = HidDevices.Enumerate(0x2E3C, [0x5750, 0x5767]).FirstOrDefault(it => !it.DevicePath.EndsWith("kbd"));
adxController[1] = HidDevices.Enumerate(0x2E4C, 0x5750).Concat(HidDevices.Enumerate(0x2E3C, 0x5768)).FirstOrDefault(it => !it.DevicePath.EndsWith("kbd"));
TryConnectDevice(0);
TryConnectDevice(1);

if (adxController[0] != null)
for (int i = 0; i < 2; i++)
{
MelonLogger.Msg("[HidInput] Open HID 1P OK");
if (adxController[i] != null)
{
TdInit(i);
}
}

if (adxController[1] != null)
{
MelonLogger.Msg("[HidInput] Open HID 2P OK");
}
if (disableButtons) return;

for (int i = 0; i < 2; i++)
{
if (adxController[i] == null) continue;
TdInit(i);
if (adxController[i].Attributes.ProductId is 0x5767 or 0x5768) continue;
if (disableButtons) continue;
if (hidThreadRunning[i]) continue;

keyEnabled = true;
hidThreadRunning[i] = true;
var p = i;
Thread hidThread = new Thread(() => HidInputThread(p));
var hidThread = new Thread(() => HidInputThread(p))
{
IsBackground = true
};
hidThread.Start();
}
}
Expand Down