Skip to content

Commit

Permalink
FIX: GetHapticCapabilitiesCommand always failing to execute (#1776)
Browse files Browse the repository at this point in the history
* Fixed GetHapticCapabilitiesCommand always failing to execute and added missing haptic capabilities properties

* Added setter to make migration better if the user was setting the old field

* Fixed API_MinorVersionsHaveNoBreakingChanges by reverting renames

The names won't match the UnityEngine.XR and IUnityXRInput.h names, but this is needed to avoid a breaking change. Renamed the optimal size field to more closely match the naming of maxBufferSize for consistency within Input System.

* Reverted redundant explicit field initialization since it was causing API_MinorVersionsHaveNoBreakingChanges to fail. The value remains the same.

The test could be updated in the future to recognize the value is the same, but for now it's easier to just revert the change.

* Added xml-doc to public fields and methods

---------

Co-authored-by: James McGill <[email protected]>
  • Loading branch information
chris-massie and jamesmcgill authored Nov 8, 2023
1 parent b19cf30 commit 317d7cc
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 9 deletions.
15 changes: 15 additions & 0 deletions Assets/Tests/InputSystem/Plugins/XRTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,7 @@ public void Controls_OptimizedControls_PoseControl_IsOptimized()

// ISXB-405
[Test]
[Category("Devices")]
public void Devices_AddingUnusualDevice_ShouldntCrashTheSystem()
{
var deviceDescr =
Expand All @@ -1153,5 +1154,19 @@ public void Devices_AddingUnusualDevice_ShouldntCrashTheSystem()

Assert.That(device, Is.Not.Null);
}

[Test]
[Category("Commands")]
public void Commands_GetHapticCapabilitiesCommand_UsesCorrectPayloadSize()
{
unsafe
{
// Check that the payload of the command matches the low-level struct defined in IUnityXRInput.h (UnityXRHapticCapabilities)
// and used in XRInputSubsystem by checking the size. The sizes are required to match for the event to be
// sent to the device.
Assert.That(sizeof(UnityEngine.InputSystem.XR.Haptics.HapticCapabilities), Is.EqualTo(sizeof(UnityEngine.XR.HapticCapabilities)));
Assert.That(sizeof(UnityEngine.InputSystem.XR.Haptics.GetHapticCapabilitiesCommand) - InputDeviceCommand.BaseCommandSize, Is.EqualTo(sizeof(UnityEngine.XR.HapticCapabilities)));
}
}
}
#endif
2 changes: 2 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ however, it has to be formatted properly to pass verification tests.
- Fixed issue of visual elements being null during editing project-wide actions in project settings which prompted console errors.
- Fixed case ISX-1436 (UI TK Input Action Asset Editor - Error deleting Bindings with DeleteKey on Windows).
- Fixed issue with UI Toolkit based Input Action Editor not restoring it's selected items after Domain Reload.
- Fixed the [`GetHapticCapabilitiesCommand`](xref:UnityEngine.InputSystem.XR.Haptics.GetHapticCapabilitiesCommand) always failing to execute due to a mismatch in the size in bytes of the payload and the size expected by XR devices. Changed [`HapticCapabilities`](xref:UnityEngine.InputSystem.XR.Haptics.HapticCapabilities) to include all properties returned by the XR input subsystem. This makes Input System match the functionality provided by the [XR](https://docs.unity3d.com/Manual/com.unity.modules.xr.html) module's [`InputDevice.TryGetHapticCapabilities`](https://docs.unity3d.com/ScriptReference/XR.InputDevice.TryGetHapticCapabilities.html) and [`HapticCapabilities`](https://docs.unity3d.com/ScriptReference/XR.HapticCapabilities.html).


## [1.8.0-pre.1] - 2023-09-04

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public class Vector2Composite : InputBindingComposite<Vector2>
/// </code>
/// </example>
/// </remarks>
public Mode mode = Mode.DigitalNormalized;
public Mode mode;

/// <inheritdoc />
public override Vector2 ReadValue(ref InputBindingCompositeContext context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ namespace UnityEngine.InputSystem.LowLevel
/// </summary>
public interface IInputDeviceCommandInfo
{
/// <summary>
/// The data format identifier of the device command as a <see cref="FourCC"/> code.
/// </summary>
FourCC typeStatic { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,147 @@

namespace UnityEngine.InputSystem.XR.Haptics
{
/// <summary>
/// Describes the haptic capabilities of a specific device.
/// </summary>
public struct HapticCapabilities
{
public HapticCapabilities(uint numChannels, uint frequencyHz, uint maxBufferSize)
/// <summary>
/// Initializes and returns an instance of <see cref="HapticCapabilities"/>.
/// </summary>
/// <param name="numChannels">The number of haptic channels available on this device.</param>
/// <param name="supportsImpulse">This device supports sending a haptic impulse.</param>
/// <param name="supportsBuffer">This device supports sending a haptic buffer.</param>
/// <param name="frequencyHz">The buffer frequency the device operates at in Hertz.</param>
/// <param name="maxBufferSize">The max amount of buffer data that can be stored by the device.</param>
/// <param name="optimalBufferSize">The optimal size of a device's buffer, taking into account frequency and latency.</param>
public HapticCapabilities(uint numChannels, bool supportsImpulse, bool supportsBuffer, uint frequencyHz, uint maxBufferSize, uint optimalBufferSize)
{
this.numChannels = numChannels;
this.supportsImpulse = supportsImpulse;
this.supportsBuffer = supportsBuffer;
this.frequencyHz = frequencyHz;
this.maxBufferSize = maxBufferSize;
this.optimalBufferSize = optimalBufferSize;
}

public uint numChannels { get; private set; }
public uint frequencyHz { get; private set; }
public uint maxBufferSize { get; private set; }
/// <summary>
/// Deprecated. Use <see cref="HapticCapabilities(uint, bool, bool, uint, uint, uint)"/> instead.
/// This constructor did not match the native haptic capabilities struct and was missing properties.
/// </summary>
/// <param name="numChannels">The number of haptic channels available on this device.</param>
/// <param name="frequencyHz">The buffer frequency the device operates at in Hertz.</param>
/// <param name="maxBufferSize">The max amount of buffer data that can be stored by the device.</param>
public HapticCapabilities(uint numChannels, uint frequencyHz, uint maxBufferSize)
: this(numChannels, false, false, frequencyHz, maxBufferSize, 0U)
{
}

/// <summary>
/// The number of haptic channels available on this device.
/// </summary>
public uint numChannels { get; }

/// <summary>
/// This device supports sending a haptic impulse.
/// </summary>
/// <seealso cref="SendHapticImpulseCommand"/>
public bool supportsImpulse { get; }

/// <summary>
/// This device supports sending a haptic buffer.
/// </summary>
/// <seealso cref="SendBufferedHapticCommand"/>
public bool supportsBuffer { get; }

/// <summary>
/// The buffer frequency the device operates at in Hertz. This impacts how fast the device consumes buffered haptic data.
/// </summary>
/// <remarks>
/// This value is greater than 0 if <see cref="supportsBuffer"/> is <see langword="true"/>, and 0 otherwise.
/// </remarks>
public uint frequencyHz { get; }

/// <summary>
/// The max amount of buffer data that can be stored by the device.
/// </summary>
public uint maxBufferSize { get; }

/// <summary>
/// The optimal size of a device's buffer, taking into account frequency and latency.
/// </summary>
public uint optimalBufferSize { get; }
}

/// <summary>
/// Input device command struct for retrieving the haptic capabilities of a device.
/// </summary>
[StructLayout(LayoutKind.Explicit, Size = kSize)]
public struct GetHapticCapabilitiesCommand : IInputDeviceCommandInfo
{
static FourCC Type => new FourCC('X', 'H', 'C', '0');

const int kSize = InputDeviceCommand.kBaseCommandSize + sizeof(uint) * 3;
// 20 bytes of data from uint(4) + bool(1) + bool(1) + padding + uint(4) + uint(4) + uint(4)
const int kSize = InputDeviceCommand.kBaseCommandSize + 20;

/// <inheritdoc />
public FourCC typeStatic => Type;

[FieldOffset(0)]
InputDeviceCommand baseCommand;

/// <summary>
/// The number of haptic channels available on this device.
/// </summary>
[FieldOffset(InputDeviceCommand.kBaseCommandSize)]
public uint numChannels;

[FieldOffset(InputDeviceCommand.kBaseCommandSize + sizeof(uint))]
/// <summary>
/// This device supports sending a haptic impulse.
/// </summary>
/// <seealso cref="SendHapticImpulseCommand"/>
[FieldOffset(InputDeviceCommand.kBaseCommandSize + 4)]
public bool supportsImpulse;

/// <summary>
/// This device supports sending a haptic buffer.
/// </summary>
/// <seealso cref="SendBufferedHapticCommand"/>
[FieldOffset(InputDeviceCommand.kBaseCommandSize + 5)]
public bool supportsBuffer;

/// <summary>
/// The buffer frequency the device operates at in Hertz. This impacts how fast the device consumes buffered haptic data.
/// </summary>
/// <remarks>
/// This value is greater than 0 if <see cref="supportsBuffer"/> is <see langword="true"/>, and 0 otherwise.
/// </remarks>
[FieldOffset(InputDeviceCommand.kBaseCommandSize + 8)]
public uint frequencyHz;

[FieldOffset(InputDeviceCommand.kBaseCommandSize + (sizeof(uint) * 2))]
/// <summary>
/// The max amount of buffer data that can be stored by the device.
/// </summary>
[FieldOffset(InputDeviceCommand.kBaseCommandSize + 12)]
public uint maxBufferSize;

public HapticCapabilities capabilities => new HapticCapabilities(numChannels, frequencyHz, maxBufferSize);
/// <summary>
/// The optimal size of a device's buffer, taking into account frequency and latency.
/// </summary>
[FieldOffset(InputDeviceCommand.kBaseCommandSize + 16)]
public uint optimalBufferSize;

/// <summary>
/// The haptic capabilities of the device, populated after this command is executed.
/// </summary>
public HapticCapabilities capabilities => new HapticCapabilities(numChannels, supportsImpulse, supportsBuffer, frequencyHz, maxBufferSize, optimalBufferSize);

/// <summary>
/// Creates and returns a new initialized input device command struct for retrieving
/// the haptic capabilities of a device when executed.
/// </summary>
/// <returns>Returns a new command struct with the data header initialized, making it ready to execute.</returns>
/// <seealso cref="InputDevice.ExecuteCommand{TCommand}(ref TCommand)"/>
public static GetHapticCapabilitiesCommand Create()
{
return new GetHapticCapabilitiesCommand
Expand Down

0 comments on commit 317d7cc

Please sign in to comment.