From 7cb59ce4ba343e0b205e8f37968b0dc84babfc8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Fri, 26 Sep 2025 13:09:03 +0100 Subject: [PATCH 1/5] Add proposed fix by PR #2245 --- .../com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs index 3e7354584b..1fdc43ad6d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs @@ -257,6 +257,7 @@ internal static unsafe HIDDeviceDescriptor ReadHIDDeviceDescriptor(ref InputDevi // Update the descriptor on the device with the information we got. deviceDescription.capabilities = hidDeviceDescriptor.ToJson(); + Debug.Log($"Parsing HID descriptor from JSON for device '{deviceDescription.capabilities}'"); } else { @@ -385,7 +386,7 @@ public InputControlLayout Build() var yElementParameters = yElement.DetermineParameters(); builder.AddControl(stickName + "/x") - .WithFormat(xElement.isSigned ? InputStateBlock.FormatSBit : InputStateBlock.FormatBit) + .WithFormat(xElement.DetermineFormat()) .WithByteOffset((uint)(xElement.reportOffsetInBits / 8 - byteOffset)) .WithBitOffset((uint)(xElement.reportOffsetInBits % 8)) .WithSizeInBits((uint)xElement.reportSizeInBits) @@ -394,7 +395,7 @@ public InputControlLayout Build() .WithProcessors(xElement.DetermineProcessors()); builder.AddControl(stickName + "/y") - .WithFormat(yElement.isSigned ? InputStateBlock.FormatSBit : InputStateBlock.FormatBit) + .WithFormat(yElement.DetermineFormat()) .WithByteOffset((uint)(yElement.reportOffsetInBits / 8 - byteOffset)) .WithBitOffset((uint)(yElement.reportOffsetInBits % 8)) .WithSizeInBits((uint)yElement.reportSizeInBits) From 9a5f397a0f3de786ece4003c22eec249394cd6f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Fri, 26 Sep 2025 13:10:17 +0100 Subject: [PATCH 2/5] Fix parsing signed bytes and incorrect byte reading for 4 bytes --- .../InputSystem/Plugins/HID/HIDParser.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs index efcd1e87c0..051b9bd1d6 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs @@ -267,7 +267,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr) { if (currentPtr >= endPtr) return 0; - return *currentPtr; + return (sbyte)*currentPtr; } // Read short. @@ -291,7 +291,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr) var data3 = *(currentPtr + 2); var data4 = *(currentPtr + 3); - return (data4 << 24) | (data3 << 24) | (data2 << 8) | data1; + return (data4 << 24) | (data3 << 16) | (data2 << 8) | data1; } Debug.Assert(false, "Should not reach here"); From bd804af9dc159dc208ecd2345a4325cf03a6d5e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Fri, 26 Sep 2025 13:12:15 +0100 Subject: [PATCH 3/5] Add test to validate stick values Also creates a simple layout struct for a device with only a single analog stick where the HID report descriptor states that the x and y values are between -127 and 127. --- Assets/Tests/InputSystem/Plugins/HIDTests.cs | 139 ++++++++++++++++--- 1 file changed, 118 insertions(+), 21 deletions(-) diff --git a/Assets/Tests/InputSystem/Plugins/HIDTests.cs b/Assets/Tests/InputSystem/Plugins/HIDTests.cs index a36018f22b..e744c24c8e 100644 --- a/Assets/Tests/InputSystem/Plugins/HIDTests.cs +++ b/Assets/Tests/InputSystem/Plugins/HIDTests.cs @@ -218,27 +218,9 @@ public void Devices_CanCreateGenericHID_FromDeviceWithBinaryReportDescriptor() // The HID report descriptor is fetched from the device via an IOCTL. var deviceId = runtime.AllocateDeviceId(); - unsafe - { - runtime.SetDeviceCommandCallback(deviceId, - (id, commandPtr) => - { - if (commandPtr->type == HID.QueryHIDReportDescriptorSizeDeviceCommandType) - return reportDescriptor.Length; - if (commandPtr->type == HID.QueryHIDReportDescriptorDeviceCommandType - && commandPtr->payloadSizeInBytes >= reportDescriptor.Length) - { - fixed(byte* ptr = reportDescriptor) - { - UnsafeUtility.MemCpy(commandPtr->payloadPtr, ptr, reportDescriptor.Length); - return reportDescriptor.Length; - } - } + SetDeviceCommandCallbackToReturnReportDescriptor(deviceId, reportDescriptor); - return InputDeviceCommand.GenericFailure; - }); - } // Report device. runtime.ReportNewInputDevice( new InputDeviceDescription @@ -309,6 +291,111 @@ public void Devices_CanCreateGenericHID_FromDeviceWithBinaryReportDescriptor() ////TODO: check hat switch } + // This is used to mock out the IOCTL the HID device driver would use to return + // the report descriptor and its size. + unsafe void SetDeviceCommandCallbackToReturnReportDescriptor(int deviceId, byte[] reportDescriptor) + { + runtime.SetDeviceCommandCallback(deviceId, + (id, commandPtr) => + { + if (commandPtr->type == HID.QueryHIDReportDescriptorSizeDeviceCommandType) + return reportDescriptor.Length; + + if (commandPtr->type == HID.QueryHIDReportDescriptorDeviceCommandType + && commandPtr->payloadSizeInBytes >= reportDescriptor.Length) + { + fixed(byte* ptr = reportDescriptor) + { + UnsafeUtility.MemCpy(commandPtr->payloadPtr, ptr, reportDescriptor.Length); + return reportDescriptor.Length; + } + } + + return InputDeviceCommand.GenericFailure; + }); + } + + [Test] + [Category("HID Devices")] + public void Devices_CanCrateGenericHID_WithSignedLogicalMinAndMaxSticks() + { + // This is a HID report descriptor for a simple device with two analog sticks; + // Similar to a user that reported an issue in Discussions: + // https://discussions.unity.com/t/input-system-reading-invalid-values-from-hall-effect-keyboards/1684840/3 + var reportDescriptor = new byte[] + { + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x05, // Usage (Game Pad) + 0xA1, 0x01, // Collection (Application) + 0x85, 0x01, // Report ID (1) + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x30, // Usage (X) + 0x09, 0x31, // Usage (Y) + 0x15, 0x81, // Logical Minimum (-127) + 0x25, 0x7F, // Logical Maximum (127) + 0x75, 0x08, // Report Size (8) + 0x95, 0x02, // Report Count (2) + 0x81, 0x02, // Input (Data,Var,Abs) + 0xC0, // End Collection + }; + + // The HID report descriptor is fetched from the device via an IOCTL. + var deviceId = runtime.AllocateDeviceId(); + + // Callback to return the desired report descriptor. + SetDeviceCommandCallbackToReturnReportDescriptor(deviceId, reportDescriptor); + + // Report device. + runtime.ReportNewInputDevice( + new InputDeviceDescription + { + interfaceName = HID.kHIDInterface, + manufacturer = "TestVendor", + product = "TestHID", + capabilities = new HID.HIDDeviceDescriptor + { + vendorId = 0x321, + productId = 0x432 + }.ToJson() + }.ToJson(), deviceId); + + InputSystem.Update(); + + var device = (Joystick)InputSystem.GetDeviceById(deviceId); + Assert.That(device, Is.Not.Null); + Assert.That(device, Is.TypeOf()); + + // Stick vector 2 should be centered at (0,0). + Assert.That(device.stick.ReadValue(), Is.EqualTo(new Vector2(0f, 0f)).Using(Vector2EqualityComparer.Instance)); + + // Queue event with stick pushed to bottom. We assume Y axis is inverted by default in HID devices. + // See HID.HIDElementDescriptor.DetermineParameters() + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickByte { reportId = 1, x = 0, y = 127 }); + InputSystem.Update(); + + Assert.That(device.stick.ReadValue() , Is.EqualTo(new Vector2(0f, -1f)).Using(Vector2EqualityComparer.Instance)); + + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickByte { reportId = 1, x = 0, y = -127 }); + InputSystem.Update(); + + Assert.That(device.stick.ReadValue(), Is.EqualTo(new Vector2(0f, 1f)).Using(Vector2EqualityComparer.Instance)); + + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickByte { reportId = 1, x = 127, y = 0 }); + InputSystem.Update(); + + Assert.That(device.stick.ReadValue() , Is.EqualTo(new Vector2(1f, 0f)).Using(Vector2EqualityComparer.Instance)); + + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickByte { reportId = 1, x = -127, y = 0 }); + InputSystem.Update(); + + Assert.That(device.stick.ReadValue(), Is.EqualTo(new Vector2(-1f, 0f)).Using(Vector2EqualityComparer.Instance)); + + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickByte { reportId = 1, x = 127, y = 127 }); + InputSystem.Update(); + + Assert.That(device.stick.ReadValue(), Is.EqualTo(new Vector2(0.7071f, -0.7071f)).Using(Vector2EqualityComparer.Instance)); + } + [Test] [Category("Devices")] public void Devices_CanCreateGenericHID_FromDeviceWithParsedReportDescriptor() @@ -1026,7 +1113,7 @@ public void Devices_GenericHIDConvertsXAndYUsagesToStickControl() } [StructLayout(LayoutKind.Explicit)] - struct SimpleJoystickLayout : IInputStateTypeInfo + struct SimpleJoystickLayoutWithStickUshort : IInputStateTypeInfo { [FieldOffset(0)] public byte reportId; [FieldOffset(1)] public ushort x; @@ -1035,6 +1122,16 @@ struct SimpleJoystickLayout : IInputStateTypeInfo public FourCC format => new FourCC('H', 'I', 'D'); } + [StructLayout(LayoutKind.Explicit)] + struct SimpleJoystickLayoutWithStickByte : IInputStateTypeInfo + { + [FieldOffset(0)] public byte reportId; + [FieldOffset(1)] public sbyte x; + [FieldOffset(2)] public sbyte y; + + public FourCC format => new FourCC('H', 'I', 'D'); + } + [Test] [Category("Devices")] public void Devices_GenericHIDXAndYDrivesStickControl() @@ -1069,7 +1166,7 @@ public void Devices_GenericHIDXAndYDrivesStickControl() Assert.That(device, Is.TypeOf()); Assert.That(device["Stick"], Is.TypeOf()); - InputSystem.QueueStateEvent(device, new SimpleJoystickLayout { reportId = 1, x = ushort.MaxValue, y = ushort.MinValue }); + InputSystem.QueueStateEvent(device, new SimpleJoystickLayoutWithStickUshort { reportId = 1, x = ushort.MaxValue, y = ushort.MinValue }); InputSystem.Update(); Assert.That(device["stick"].ReadValueAsObject(), From 8ddd35ee9c093daab697ee53e1b4558468b2d56b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Sat, 27 Sep 2025 15:47:25 +0300 Subject: [PATCH 4/5] Cast short when concatenating bytes --- .../com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs index 051b9bd1d6..5317277ee1 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDParser.cs @@ -277,7 +277,7 @@ private unsafe static int ReadData(int itemSize, byte* currentPtr, byte* endPtr) return 0; var data1 = *currentPtr; var data2 = *(currentPtr + 1); - return (data2 << 8) | data1; + return (short)(data2 << 8) | data1; } // Read int. From 637e5eb8fcf5b62df2d44e0811cbfea961ebc5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Freire?= Date: Mon, 29 Sep 2025 10:38:19 +0300 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Assets/Tests/InputSystem/Plugins/HIDTests.cs | 2 +- Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Assets/Tests/InputSystem/Plugins/HIDTests.cs b/Assets/Tests/InputSystem/Plugins/HIDTests.cs index e744c24c8e..bec950f159 100644 --- a/Assets/Tests/InputSystem/Plugins/HIDTests.cs +++ b/Assets/Tests/InputSystem/Plugins/HIDTests.cs @@ -317,7 +317,7 @@ unsafe void SetDeviceCommandCallbackToReturnReportDescriptor(int deviceId, byte[ [Test] [Category("HID Devices")] - public void Devices_CanCrateGenericHID_WithSignedLogicalMinAndMaxSticks() + public void Devices_CanCreateGenericHID_WithSignedLogicalMinAndMaxSticks() { // This is a HID report descriptor for a simple device with two analog sticks; // Similar to a user that reported an issue in Discussions: diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs index 1fdc43ad6d..0a304dc068 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HID.cs @@ -257,7 +257,6 @@ internal static unsafe HIDDeviceDescriptor ReadHIDDeviceDescriptor(ref InputDevi // Update the descriptor on the device with the information we got. deviceDescription.capabilities = hidDeviceDescriptor.ToJson(); - Debug.Log($"Parsing HID descriptor from JSON for device '{deviceDescription.capabilities}'"); } else {