Skip to content

Commit 6b81f4c

Browse files
PlayStation: bypass HidSharp overlapped I/O on Windows via direct WriteFile
Field reports against the USB transport showed lightbar updating once and then freezing. Root cause: HidSharp's HidStream opens its handle with FILE_FLAG_OVERLAPPED and runs an asynchronous WriteFile + GetOverlappedResult dance. The PlayStation HID minidriver returns failure on the second and subsequent overlapped writes, which HidSharp surfaces as IOException — the queue's catch handler then suspends the queue. Reintroduced HidRawWriter (synchronous Win32 WriteFile on a separate kernel handle, BOOL return — never throws). On Windows, both DS4 and DS5 queues prefer the raw writer; on non-Windows the queues fall back to HidStream.Write since HidSharp's macOS/Linux paths don't share the overlapped Windows code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 45c9ce9 commit 6b81f4c

5 files changed

Lines changed: 252 additions & 48 deletions

File tree

RGB.NET.Devices.PlayStation/DualSense/DualSenseUpdateQueue.cs

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ internal sealed class DualSenseUpdateQueue : UpdateQueue
6565
#region Properties & Fields
6666

6767
private readonly HidStream _stream;
68+
private readonly HidRawWriter? _rawWriter;
6869
private readonly PlayStationTransport _transport;
6970
private readonly byte[] _buffer;
7071
private readonly string _devicePath;
@@ -77,10 +78,11 @@ internal sealed class DualSenseUpdateQueue : UpdateQueue
7778

7879
#region Constructors
7980

80-
public DualSenseUpdateQueue(IDeviceUpdateTrigger trigger, HidStream stream, PlayStationTransport transport, string devicePath)
81+
public DualSenseUpdateQueue(IDeviceUpdateTrigger trigger, HidStream stream, HidRawWriter? rawWriter, PlayStationTransport transport, string devicePath)
8182
: base(trigger)
8283
{
8384
_stream = stream;
85+
_rawWriter = rawWriter;
8486
_transport = transport;
8587
_devicePath = devicePath ?? string.Empty;
8688
_buffer = new byte[transport == PlayStationTransport.Bluetooth ? 78 : 63];
@@ -132,21 +134,39 @@ protected override bool Update(ReadOnlySpan<(object key, Color color)> dataSet)
132134
// the lightbar at black instead of leaving uninitialised state.
133135
if (!gotLightbar) lightbar = new Color(0, 0, 0);
134136

137+
bool ok;
138+
lock (_writeLock)
139+
{
140+
Array.Clear(_buffer, 0, _buffer.Length);
141+
BuildReport(lightbar, playerLedBits);
142+
ok = WriteBuffer();
143+
}
144+
145+
if (!ok)
146+
{
147+
Trace.WriteLine("[RGB.NET.PlayStation] DualSense write failed, suspending queue.");
148+
_disposed = true;
149+
return false;
150+
}
151+
_firstReport = false;
152+
return true;
153+
}
154+
155+
// See DualShock4UpdateQueue.WriteBuffer for the rationale behind preferring
156+
// HidRawWriter over HidStream.Write on Windows.
157+
private bool WriteBuffer()
158+
{
159+
if (_rawWriter != null)
160+
return _rawWriter.TryWrite(_buffer);
161+
135162
try
136163
{
137-
lock (_writeLock)
138-
{
139-
Array.Clear(_buffer, 0, _buffer.Length);
140-
BuildReport(lightbar, playerLedBits);
141-
_stream.Write(_buffer);
142-
}
143-
_firstReport = false;
164+
_stream.Write(_buffer);
144165
return true;
145166
}
146167
catch (Exception ex)
147168
{
148-
Trace.WriteLine($"[RGB.NET.PlayStation] DualSense write failed, suspending queue: {ex.Message}");
149-
_disposed = true;
169+
Trace.WriteLine($"[RGB.NET.PlayStation] DualSense stream write threw: {ex.Message}");
150170
return false;
151171
}
152172
}
@@ -246,18 +266,13 @@ public void Shutdown(bool sendOffFrame = true)
246266
if (_disposed) return;
247267
_disposed = true;
248268
if (!sendOffFrame) return;
249-
try
250-
{
251-
lock (_writeLock)
252-
{
253-
Array.Clear(_buffer, 0, _buffer.Length);
254-
BuildReport(new Color(0, 0, 0), 0);
255-
_stream.Write(_buffer);
256-
}
257-
}
258-
catch
269+
// Best-effort — WriteBuffer returns false silently if the handle has
270+
// already been invalidated.
271+
lock (_writeLock)
259272
{
260-
// Best-effort.
273+
Array.Clear(_buffer, 0, _buffer.Length);
274+
BuildReport(new Color(0, 0, 0), 0);
275+
WriteBuffer();
261276
}
262277
}
263278

RGB.NET.Devices.PlayStation/DualShock4/DualShock4UpdateQueue.cs

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ internal sealed class DualShock4UpdateQueue : UpdateQueue
3434
#region Properties & Fields
3535

3636
private readonly HidStream _stream;
37+
private readonly HidRawWriter? _rawWriter;
3738
private readonly PlayStationTransport _transport;
3839
private readonly byte[] _buffer;
3940
private readonly string _devicePath;
@@ -44,10 +45,11 @@ internal sealed class DualShock4UpdateQueue : UpdateQueue
4445

4546
#region Constructors
4647

47-
public DualShock4UpdateQueue(IDeviceUpdateTrigger trigger, HidStream stream, PlayStationTransport transport, string devicePath)
48+
public DualShock4UpdateQueue(IDeviceUpdateTrigger trigger, HidStream stream, HidRawWriter? rawWriter, PlayStationTransport transport, string devicePath)
4849
: base(trigger)
4950
{
5051
_stream = stream;
52+
_rawWriter = rawWriter;
5153
_transport = transport;
5254
_devicePath = devicePath ?? string.Empty;
5355
_buffer = new byte[transport == PlayStationTransport.Bluetooth ? 78 : 32];
@@ -76,26 +78,46 @@ protected override bool Update(ReadOnlySpan<(object key, Color color)> dataSet)
7678
// colour we see — RGB.NET commits the painted colour for that LED each tick.
7779
Color color = dataSet[0].color;
7880

79-
try
81+
bool ok;
82+
lock (_writeLock)
8083
{
81-
lock (_writeLock)
82-
{
83-
Array.Clear(_buffer, 0, _buffer.Length);
84-
BuildReport(color);
85-
_stream.Write(_buffer);
86-
}
87-
return true;
84+
Array.Clear(_buffer, 0, _buffer.Length);
85+
BuildReport(color);
86+
ok = WriteBuffer();
8887
}
89-
catch (Exception ex)
88+
89+
if (!ok)
9090
{
9191
// Device went away mid-write or another tool grabbed exclusive access.
9292
// Suspend the queue so the next 30Hz tick short-circuits at the
9393
// `if (_disposed)` gate. The provider's hot-plug Reconcile will
9494
// RemoveDevice us shortly.
95-
Trace.WriteLine($"[RGB.NET.PlayStation] DualShock4 write failed, suspending queue: {ex.Message}");
95+
Trace.WriteLine("[RGB.NET.PlayStation] DualShock4 write failed, suspending queue.");
9696
_disposed = true;
9797
return false;
9898
}
99+
return true;
100+
}
101+
102+
// On Windows, prefer HidRawWriter (synchronous Win32 WriteFile) — HidSharp's
103+
// overlapped HidStream.Write fails on the second and subsequent USB writes
104+
// against the PlayStation HID minidriver. On non-Windows or if HidRawWriter
105+
// failed to open, fall back to HidStream.Write.
106+
private bool WriteBuffer()
107+
{
108+
if (_rawWriter != null)
109+
return _rawWriter.TryWrite(_buffer);
110+
111+
try
112+
{
113+
_stream.Write(_buffer);
114+
return true;
115+
}
116+
catch (Exception ex)
117+
{
118+
Trace.WriteLine($"[RGB.NET.PlayStation] DualShock4 stream write threw: {ex.Message}");
119+
return false;
120+
}
99121
}
100122

101123
private void BuildReport(Color color)
@@ -169,19 +191,14 @@ public void Shutdown(bool sendOffFrame = true)
169191
// last colour after we tear down. The controller's firmware restores
170192
// the OS-driven indicator (e.g. player number) shortly after we stop
171193
// sending reports anyway, but explicit black avoids the visible "stuck
172-
// on last colour" beat between shutdown and firmware reset.
173-
try
174-
{
175-
lock (_writeLock)
176-
{
177-
Array.Clear(_buffer, 0, _buffer.Length);
178-
BuildReport(new Color(0, 0, 0));
179-
_stream.Write(_buffer);
180-
}
181-
}
182-
catch
194+
// on last colour" beat between shutdown and firmware reset. Best-effort —
195+
// WriteBuffer returns false silently if the handle has already been
196+
// invalidated.
197+
lock (_writeLock)
183198
{
184-
// Best-effort — handle may already have been invalidated.
199+
Array.Clear(_buffer, 0, _buffer.Length);
200+
BuildReport(new Color(0, 0, 0));
201+
WriteBuffer();
185202
}
186203
}
187204

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
using System;
2+
using System.IO;
3+
using System.Runtime.InteropServices;
4+
using Microsoft.Win32.SafeHandles;
5+
6+
namespace RGB.NET.Devices.PlayStation;
7+
8+
// Direct Win32 WriteFile wrapper for HID output reports — Windows only.
9+
//
10+
// On Windows, HidSharp's HidStream opens its underlying handle with
11+
// FILE_FLAG_OVERLAPPED and runs an asynchronous WriteFile + GetOverlappedResult
12+
// dance internally. Field reports against the PlayStation USB minidriver
13+
// indicate that overlapped path can return non-ERROR_IO_PENDING failure on the
14+
// second and subsequent writes, which HidSharp surfaces as IOException — and
15+
// our queue's catch handler turns the exception into a queue suspension. End
16+
// result: lightbar updates once, then freezes.
17+
//
18+
// To sidestep the overlapped I/O path entirely we open OUR OWN kernel handle
19+
// alongside HidSharp's, with shared read/write access and dwFlagsAndAttributes
20+
// = 0 (synchronous I/O, no overlapped). WriteFile then returns BOOL — false on
21+
// failure surfaces as a return value, no exception. HidSharp keeps the handle
22+
// it opens during TryOpen (still useful for detecting exclusive-access
23+
// conflicts via DS4Windows / reWASD at open time). Two handles per controller
24+
// is fine — Sony HID gamepads accept shared writes by default on Windows.
25+
//
26+
// The class is intentionally minimal: open with shared read/write access,
27+
// write a buffer, dispose. No reads, no overlapped I/O, no internal locking
28+
// (the caller's UpdateQueue already serialises via its own write lock).
29+
//
30+
// Windows-only at runtime — the kernel32 P/Invokes will throw
31+
// DllNotFoundException on Linux/macOS. Construction is gated on
32+
// OperatingSystem.IsWindows() inside the provider; the class itself is not
33+
// platform-attributed to keep call sites readable across the assembly.
34+
internal sealed class HidRawWriter : IDisposable
35+
{
36+
#region Win32
37+
38+
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
39+
private static extern SafeFileHandle CreateFileW(
40+
string lpFileName,
41+
uint dwDesiredAccess,
42+
uint dwShareMode,
43+
IntPtr lpSecurityAttributes,
44+
uint dwCreationDisposition,
45+
uint dwFlagsAndAttributes,
46+
IntPtr hTemplateFile);
47+
48+
[DllImport("kernel32.dll", SetLastError = true)]
49+
[return: MarshalAs(UnmanagedType.Bool)]
50+
private static extern bool WriteFile(
51+
SafeFileHandle hFile,
52+
byte[] lpBuffer,
53+
uint nNumberOfBytesToWrite,
54+
out uint lpNumberOfBytesWritten,
55+
IntPtr lpOverlapped);
56+
57+
private const uint GENERIC_WRITE = 0x40000000;
58+
private const uint FILE_SHARE_READ = 0x00000001;
59+
private const uint FILE_SHARE_WRITE = 0x00000002;
60+
private const uint OPEN_EXISTING = 3;
61+
62+
#endregion
63+
64+
#region Properties & Fields
65+
66+
private readonly SafeFileHandle _handle;
67+
private volatile bool _closed;
68+
69+
#endregion
70+
71+
#region Constructors
72+
73+
public HidRawWriter(string devicePath)
74+
{
75+
if (string.IsNullOrEmpty(devicePath))
76+
throw new ArgumentException("Device path is required.", nameof(devicePath));
77+
78+
// Shared read/write so we coexist with HidSharp's handle and any other
79+
// app (Steam Input, game native lighting, etc.) that may also be
80+
// opening the device. dwFlagsAndAttributes = 0 → synchronous I/O. We
81+
// don't need overlapped — writes are small (78 bytes max) and our
82+
// caller is already on a dedicated trigger thread.
83+
_handle = CreateFileW(
84+
devicePath,
85+
GENERIC_WRITE,
86+
FILE_SHARE_READ | FILE_SHARE_WRITE,
87+
IntPtr.Zero,
88+
OPEN_EXISTING,
89+
0,
90+
IntPtr.Zero);
91+
92+
if (_handle == null || _handle.IsInvalid)
93+
{
94+
int err = Marshal.GetLastWin32Error();
95+
throw new IOException($"CreateFile failed for HID device ({devicePath}). Win32 error: {err}");
96+
}
97+
}
98+
99+
#endregion
100+
101+
#region Methods
102+
103+
/// <summary>
104+
/// True on successful write, false on any failure (handle invalid, device
105+
/// gone, partial write, etc.). Never throws — that's the whole point.
106+
/// Caller checks the return value and decides whether to log, retry, or
107+
/// self-suspend the queue.
108+
///
109+
/// The first byte of <paramref name="buffer"/> must be the HID report ID,
110+
/// matching the convention HidStream.Write uses.
111+
/// </summary>
112+
public bool TryWrite(byte[] buffer)
113+
{
114+
if (_closed) return false;
115+
if (buffer == null || buffer.Length == 0) return false;
116+
117+
try
118+
{
119+
if (_handle == null || _handle.IsClosed || _handle.IsInvalid)
120+
return false;
121+
122+
return WriteFile(_handle, buffer, (uint)buffer.Length, out _, IntPtr.Zero);
123+
}
124+
catch
125+
{
126+
// P/Invoke marshalling could conceivably fault on a pathological
127+
// handle state; swallow and report failure rather than escape the
128+
// contract.
129+
return false;
130+
}
131+
}
132+
133+
public void Dispose()
134+
{
135+
if (_closed) return;
136+
_closed = true;
137+
try { _handle?.Dispose(); } catch { /* best effort */ }
138+
}
139+
140+
#endregion
141+
}

0 commit comments

Comments
 (0)