Repository navigation
Conversation
OleRoss
changed the title
Contributor
Author
|
@codex review Comment generated by claude-opus-5-5 on behalf of Ole |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
Repo Coverage
PR Coverage1353/1527 changed lines covered (Lines: 88.61%, Branches: 75.11%) Darp.Ble.HciHost🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciHostBleManagerBuilderExtensions.cs — 11/49 changed lines covered (Lines: 22.45%, Branches: 12.5%)...
24 ⬛ {
25 🟥 ArgumentNullException.ThrowIfNull(builder);
26 🟥 var options = new SerialHciHostOptions();
27 🟥 configure?.Invoke(options);
28 🟥 var mapping = new Dictionary<(ushort VendorId, ushort ProductId), string>(options.DeviceNameMapping);
29 🟥 return builder.AddBackend(
30 🟥 new HciHostBackend(() =>
31 🟥 [
32 🟥 .. UsbPort
33 🟥 .GetPortInfos()
34 🟥 .Where(port => port.Port is not null && mapping.ContainsKey((port.VendorId, port.ProductId)))
35 🟥 .Select(port =>
36 🟥 SerialSource(
37 🟥 port.Port ?? "",
38 🟥 $"{mapping[(port.VendorId, port.ProductId)]} ({port.Port})",
39 🟥 options.RandomAddress,
40 🟥 options.Settings
41 🟥 )
42 🟥 ),
43 🟥 ]
44 🟥 )
45 🟥 );
46 ⬛ }
...
58 ⬛ {
59 🟥 ArgumentNullException.ThrowIfNull(builder);
60 🟥 ArgumentException.ThrowIfNullOrEmpty(portName);
61 🟥 var options = new HciHostOptions();
62 🟥 configure?.Invoke(options);
63 🟥 HciHostAdapterSource source = SerialSource(
64 🟥 portName,
65 🟥 options.Name ?? portName,
66 🟥 options.RandomAddress,
67 🟥 options.Settings
68 🟥 );
69 🟥 return builder.AddBackend(new HciHostBackend(() => [source]));
70 ⬛ }
...
85 🟩 var options = new HciHostOptions();
86 🟨 configure?.Invoke(options);
87 🟩 var source = new HciHostAdapterSource(
...
101 ⬛ ) =>
102 🟥 new(
103 🟥 name,
104 🟥 context => new H4TransportLayer(portName, context.LoggerFactory.CreateLogger<H4TransportLayer>()),
105 🟥 randomAddress,
106 🟥 settings
107 🟥 );
108 ⬛ }
109 ⬛ 🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gap/HciHostAdvertisingSetDriver.cs — 116/129 changed lines covered (Lines: 89.92%, Branches: 75%)...
82 🟩 CheckType(parameters);
83 🟨 return _device.RunAsync<
84 🟩 HciLeSetExtendedAdvertisingParametersV1Command,
...
136 🟩 CheckLength(data, _maxDataLength, nameof(data));
137 🟥 ReadOnlyMemory<byte> bytes = data.AsReadOnlyMemory();
138 🟥 return _device.RunAsync<HciLeSetExtendedScanResponseDataCommand, HciLeSetExtendedScanResponseDataResult>(
139 🟥 new HciLeSetExtendedScanResponseDataCommand
140 🟥 {
141 🟥 AdvertisingHandle = _handle,
142 🟥 Operation = 0x03, // Complete scan response data
143 🟥 FragmentPreference = 0x01, // The controller should not fragment
144 🟥 ScanResponseDataLength = (byte)bytes.Length,
145 🟥 ScanResponseData = bytes,
146 🟥 },
147 🟥 "set the scan response data",
148 🟥 cancellationToken
149 🟥 );
150 ⬛ }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Connections/HciHostConnectionDriver.cs — 135/143 changed lines covered (Lines: 94.41%, Branches: 84.09%)...
136 🟩 int max = _connection.AttMtu - 3;
137 🟨 if (value.Length > max)
138 🟥 throw new ArgumentOutOfRangeException(nameof(value), value.Length, $"The MTU allows at most {max} bytes");
139 🟩 }
...
142 ⬛ {
143 🟨 if (!_services.TryGetValue(start, out HciHostRemoteService? service))
144 ⬛ {
...
185 ⬛ // Notifications of characteristics nobody discovered have no reader
186 🟨 if (characteristic is not null)
187 🟩 _sink.OnNotification(characteristic, notification.Value.Span);
...
206 ⬛ {
207 🟨 if (!_down.TrySetResult(info))
208 🟥 return;
209 🟩 _sink.OnDisconnected(info);
...
219 ⬛ private void OnReadByTypeRequest(AttReadByTypeReq<ushort> request) =>
220 🟥 NotFound(request.OpCode, request.StartingHandle);
221 ⬛
222 ⬛ [MessageSink]
223 ⬛ private void OnFindByTypeValueRequest(AttFindByTypeValueReq request) =>
224 🟥 NotFound(request.OpCode, request.StartingHandle);
225 ⬛
226 ⬛ [MessageSink]
227 ⬛ private void OnFindInformationRequest(AttFindInformationReq request) =>
228 🟥 NotFound(request.OpCode, request.StartingHandle);
229 ⬛
230 ⬛ [MessageSink]
231 ⬛ private void OnReadRequest(AttReadReq request) =>
232 🟥 _connection.EnqueueGattErrorResponse(request.OpCode, request.AttributeHandle, AttErrorCode.InvalidHandle, null);
233 ⬛
234 ⬛ [MessageSink]
235 ⬛ private void OnWriteRequest(AttWriteReq request) =>
236 🟥 _connection.EnqueueGattErrorResponse(request.OpCode, request.AttributeHandle, AttErrorCode.InvalidHandle, null);
237 ⬛
...
271 ⬛ // Unknown Connection Identifier: the link went down meanwhile, and its event is on the way
272 🟨 if (status is not (HciCommandStatus.Success or HciCommandStatus.UnknownConnectionIdentifier))
273 🟥 throw HciCommands.Failed(status, "disconnect");
274 🟩 await _down
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Connections/HciHostCentralDriver.cs — 73/80 changed lines covered (Lines: 91.25%, Branches: 75%)...
42 🟩 TimeSpan supervision = parameters.ConnectionInterval * 6;
43 🟨 ushort supervisionTimeout = HciTiming.ToSupervisionTimeoutUnits(
44 🟩 supervision > MinSupervisionTimeout ? supervision : MinSupervisionTimeout,
...
54 ⬛ {
55 🟨 throw new BleConnectException(
56 🟩 status is HciCommandStatus.CommandDisallowed or HciCommandStatus.ConnectionAlreadyExists
...
71 ⬛ }
72 🟥 }
73 ⬛ finally
...
110 ⬛ }
111 🟥 catch (TimeoutException e)
112 ⬛ {
113 🟥 throw new BleConnectException(
114 🟥 BleConnectErrorKind.TimedOut,
115 🟥 "The controller did not answer the connect command",
116 🟥 innerException: e
117 🟥 );
118 ⬛ }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gap/HciHostBroadcasterDriver.cs — 32/37 changed lines covered (Lines: 86.49%, Branches: 75%)...
34 🟩 HciHostAdvertisingSetDriver.CheckLength(data, _maxDataLength, nameof(data));
35 🟨 if (scanResponseData is not null)
36 🟥 HciHostAdvertisingSetDriver.CheckLength(scanResponseData, _maxDataLength, nameof(scanResponseData));
37 🟩 byte handle = TakeHandle();
...
43 🟩 await set.SetDataAsync(data, cancellationToken).ConfigureAwait(false);
44 🟨 if (scanResponseData is not null)
45 🟥 await set.SetScanResponseDataAsync(scanResponseData, cancellationToken).ConfigureAwait(false);
46 🟩 return set;
47 ⬛ }
48 🟥 catch
49 ⬛ {
50 🟥 await set.DisposeAsync().ConfigureAwait(false);
51 🟥 throw;
52 ⬛ }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gap/HciHostObserverDriver.cs — 86/91 changed lines covered (Lines: 94.51%, Branches: 87.5%)...
59 🟩 }
60 🟥 catch
61 🟥 {
62 ⬛ lock (_lock)
63 🟥 _sink = null;
64 🟥 throw;
65 ⬛ }
...
93 🟩 sink = _sink;
94 🟨 if (sink is null)
95 🟥 return;
96 🟩 DateTimeOffset timestamp = _device.TimeProvider.GetUtcNow();
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Connections/ConnectAttempt.cs — 38/41 changed lines covered (Lines: 92.68%, Branches: 66.67%)...
52 ⬛ {
53 🟨 if (hciEvent.Role is not HciLeConnectionRole.Central || _result.Task.IsCompleted)
54 🟥 return;
55 🟩 if (hciEvent.Status is HciCommandStatus.UnknownConnectionIdentifier)
...
72 ⬛ // A link to another peer is not this attempt's
73 🟨 if (hciEvent.PeerAddress.ToUInt64() != _peer.Value.ToUInt64())
74 🟥 return;
75 🟨 if (!_device.TryGetConnection(hciEvent.ConnectionHandle, out AclConnection? connection))
76 🟥 return;
77 ⬛ #pragma warning disable CA2000 // The caller owns the driver; a cancelled caller disposes it
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gatt/PendingResponse.cs — 18/21 changed lines covered (Lines: 85.71%, Branches: 62.5%)...
32 ⬛ }
33 🟨 else if (typeof(T) == typeof(AttErrorRsp))
34 ⬛ {
35 🟩 AttErrorRsp error = Unsafe.As<T, AttErrorRsp>(ref pdu);
36 🟨 if (error.RequestOpCode != _requestOpCode)
37 🟥 return;
38 🟩 answer = AttResponse<TResponse>.Fail(error);
...
41 ⬛ {
42 🟥 return;
43 ⬛ }
44 🟨 if (_answer.Task.IsCompleted)
45 🟥 return;
46 🟩 _gattRequest.OnTransactionAnswered();
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gatt/HciHostRemoteCharacteristic.cs — 45/47 changed lines covered (Lines: 95.74%, Branches: 83.33%)...
50 🟩 throw new OperationCanceledException("Notifications were not changed; the request was abandoned");
51 🟨 HciHostRemoteDescriptor cccd =
52 🟩 descriptors.FirstOrDefault(descriptor => descriptor.Uuid == CccdUuid)
...
55 🟩 );
56 🟨 await _connection
57 🟩 .WriteAsync(cccd.Handle, enabled ? new byte[] { 0x01, 0x00 } : new byte[] { 0x00, 0x00 }, request)
...
62 ⬛ {
63 🟨 if (_descriptors is { } known)
64 🟥 return known;
65 🟩 List<HciHostRemoteDescriptor> descriptors = [];
...
79 🟩 ushort last = response.InformationData.Span[^1].Handle;
80 🟨 if (last >= _endHandle)
81 ⬛ break;
82 🟥 start = (ushort)(last + 1);
83 ⬛ }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciCommands.cs — 6/8 changed lines covered (Lines: 75%, Branches: 50%)...
22 🟩 .ConfigureAwait(false);
23 🟨 if (result.Status is not HciCommandStatus.Success)
24 🟥 throw Failed(result.Status, action);
25 🟩 return result;
...
28 ⬛ public static BleException Failed(HciCommandStatus status, string action) =>
29 🟥 new($"The controller could not {action}: {status}", false, (int)status, null);
30 ⬛ }
31 ⬛ 🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciHostAdapterSource.cs — 29/30 changed lines covered (Lines: 96.67%, Branches: 50%)...
24 🟩 cancellationToken.ThrowIfCancellationRequested();
25 🟨 BleAddress address = _randomAddress ?? BleAddress.NewRandomStaticAddress();
26 ⬛ #pragma warning disable CA2000 // The driver owns the device; a failed setup disposes it below
...
41 🟩 await device.DisposeAsync().ConfigureAwait(false);
42 🟨 if (e is OperationCanceledException && cancellationToken.IsCancellationRequested)
43 🟥 throw;
44 🟨 throw new BleOpenException(
45 🟩 BleOpenErrorKind.NotAvailable,
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gatt/AttClient.cs — 38/39 changed lines covered (Lines: 97.44%, Branches: 85.71%)...
47 🟩 return response.Value;
48 🟨 return response.Error.ErrorCode is AttErrorCode.AttributeNotFoundError
49 🟩 ? null
...
66 ⬛ {
67 🟨 if (_ended is not null)
68 🟥 throw new BleException("The link is down", _ended);
69 🟩 _pending = pending;
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/SerialHciHostOptions.cs — 0/1 changed lines covered (Lines: 0%, Branches: n/a)...
16 ⬛ public IDictionary<(ushort VendorId, ushort ProductId), string> DeviceNameMapping { get; } =
17 🟥 new Dictionary<(ushort VendorId, ushort ProductId), string> { [(0x2FE3, 0x0004)] = "nrf52840 dongle" };
18 ⬛ }
19 ⬛ 🟠 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciHostBackend.cs — 6/6 changed lines covered (Lines: 100%, Branches: 50%)...
15 ⬛ ) =>
16 🟨 cancellationToken.IsCancellationRequested
17 🟩 ? Task.FromCanceled<IReadOnlyList<IBleAdapterSource>>(cancellationToken)
...🟠 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciTiming.cs — 12/12 changed lines covered (Lines: 100%, Branches: 75%)...
24 🟩 long units = (long)Math.Round(value / unit, MidpointRounding.AwayFromZero);
25 🟨 if (units < min || units > max)
26 ⬛ {
...✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/Gatt/HciHostRemoteService.cs — 41/41 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.HciHost/HciHostAdapterDriver.cs — 41/41 changed lines covered Darp.Ble.Hci🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/AclPacketQueue.cs — 186/215 changed lines covered (Lines: 86.51%, Branches: 77.68%)...
93 🟩 link = Find(connectionHandle);
94 🟨 if (_closed is not null || link is null)
95 ⬛ {
96 🟥 ArrayPool<byte>.Shared.Return(frame);
97 🟥 return ValueTask.FromException(Closed(connectionHandle));
98 ⬛ }
99 🟨 if (cancellationToken.IsCancellationRequested || released.IsCancellationRequested)
100 ⬛ {
101 🟥 ArrayPool<byte>.Shared.Return(frame);
102 🟥 return ValueTask.FromCanceled(cancellationToken.IsCancellationRequested ? cancellationToken : released);
103 ⬛ }
...
149 🟩 return new ValueTask(handedOff.Task);
150 🟥 }
151 ⬛
...
175 🟩 Link? link = Find(connectionHandle);
176 🟨 if (_closed is not null || link is null)
177 ⬛ {
178 🟥 ArrayPool<byte>.Shared.Return(frame);
179 🟥 return;
180 ⬛ }
...
209 ⬛ {
210 🟨 if (Find(connectionHandle) is not { } link)
211 🟥 return;
212 🟩 int index = _links.IndexOf(link);
213 🟩 _links.RemoveAt(index);
214 🟨 if (_nextLink > index)
215 🟥 _nextLink--;
216 🟩 _freeBuffers = Math.Min(_maxPacketsInFlight, _freeBuffers + link.InFlight);
...
228 ⬛ {
229 🟨 if (_closed is not null)
230 🟥 return;
231 🟩 _closed = reason;
...
255 ⬛ {
256 🟨 if (_pumping)
257 🟥 return;
258 🟩 _pumping = true;
...
321 🟩 }
322 🟥 catch (Exception e)
323 ⬛ {
324 ⬛ // A broken transport fails the links; its owner learns it through the transport's own error
325 🟥 Close(e);
326 ⬛ }
...
331 ⬛ {
332 🟨 foreach (bool answers in (ReadOnlySpan<bool>)[true, false])
333 ⬛ {
...
345 ⬛ }
346 🟥 throw new InvalidOperationException("A frame was queued but none was found");
347 ⬛ }
...
375 ⬛ }
376 🟨 foreach (Frame frame in dropped)
377 🟥 ArrayPool<byte>.Shared.Return(frame.Bytes);
378 ⬛ // The rest of a frame that is half sent belongs to the link that is gone
379 🟨 if (_sending?.Link == link)
380 ⬛ {
381 🟥 Frame sending = _sending;
382 🟥 _sending = null;
383 🟥 sending.IsDropped = true;
384 🟥 if (!ReferenceEquals(_transmitting, sending))
385 🟥 ArrayPool<byte>.Shared.Return(sending.Bytes);
386 ⬛ }
...
391 ⬛ {
392 🟨 foreach (Frame frame in frames)
393 🟥 frame.Fail(reason);
394 🟩 }
...
397 ⬛ {
398 🟨 foreach (Link link in _links)
399 ⬛ {
...
402 ⬛ }
403 🟥 return null;
404 🟩 }
405 ⬛
406 ⬛ private HciException Closed(ushort connectionHandle) =>
407 🟥 _closed is not null
408 🟥 ? new HciException("The HCI transport is gone", _closed)
409 🟥 : new HciException($"The link 0x{connectionHandle:X3} is down");
410 ⬛
...
471 ⬛ {
472 🟥 Registration.Unregister();
473 🟥 ReleasedRegistration.Unregister();
474 🟥 _handedOff?.TrySetException(reason);
475 🟥 }
476 ⬛
...
479 🟩 if (_queue.TryRemove(this))
480 🟨 _handedOff?.TrySetCanceled(cancellationToken);
481 🟩 }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/Transport/H4TransportLayer.cs — 0/22 changed lines covered (Lines: 0%, Branches: 0%)...
31 ⬛ // Each packet is serialized into a pooled array when it is queued, so the caller may reuse its buffers at once
32 🟥 private readonly Channel<(byte[] Bytes, int Length)> _txQueue = Channel.CreateUnbounded<(byte[], int)>(
33 🟥 new UnboundedChannelOptions { SingleReader = true }
34 🟥 );
35 ⬛ private readonly CancellationTokenSource _cancelSource = new();
...
58 ⬛ _serialPort.Open();
59 🟥 _txTask = Task.Run(() => RunTx(onError), cancellationToken);
60 🟥 _rxTask = Task.Run(() => RunRx(onReceived, onError), cancellationToken);
61 ⬛ return ValueTask.CompletedTask;
...
87 ⬛
88 🟥 onError(exception);
89 ⬛ }
...
96 ⬛ {
97 🟥 (byte[] bytes, int length) = await _txQueue.Reader.ReadAsync(StopToken).ConfigureAwait(false);
98 ⬛ try
99 ⬛ {
100 🟥 await _serialPort.BaseStream.WriteAsync(bytes.AsMemory(0, length), StopToken).ConfigureAwait(false);
101 ⬛ }
...
115 ⬛ }
116 🟥 ReportTransportFailure(e, "Tx", onError);
117 ⬛ }
...
163 ⬛
164 🟥 ReportTransportFailure(e, "Rx", onError);
165 ⬛ }
...
195 ⬛ {
196 🟥 ArgumentNullException.ThrowIfNull(packet);
197 ⬛ ObjectDisposedException.ThrowIf(_isDisposing, this);
198 🟥 int length = 1 + packet.GetByteCount();
199 🟥 byte[] bytes = ArrayPool<byte>.Shared.Rent(length);
200 🟥 bytes[0] = (byte)packet.PacketType;
201 🟥 if (!packet.TryWriteLittleEndian(bytes.AsSpan(1, length - 1)))
202 ⬛ {
203 🟥 ArrayPool<byte>.Shared.Return(bytes);
204 🟥 _logger?.LogPacketSendingErrorEncoding(packet);
205 🟥 return;
206 ⬛ }
207 ⬛ // Writing to an unbounded channel should work, always
208 🟥 if (!_txQueue.Writer.TryWrite((bytes, length)))
209 🟥 ArrayPool<byte>.Shared.Return(bytes);
210 ⬛ }
...
239 ⬛ // Packets that were never written still hold pooled arrays
240 🟥 while (_txQueue.Reader.TryRead(out (byte[] Bytes, int Length) unsent))
241 🟥 ArrayPool<byte>.Shared.Return(unsent.Bytes);
242 ⬛ _logger?.LogH4TransportDisposed();
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/AclConnectionExtensions.cs — 33/46 changed lines covered (Lines: 71.74%, Branches: 63.89%)...
31 ⬛ ArgumentNullException.ThrowIfNull(connection);
32 🟨 activity?.SetDeconstructedTags("Response", attPdu, orderEntries: true);
33 ⬛ // Dropped once the link is down: nobody waits for it
34 ⬛ if (connection.DisconnectToken.IsCancellationRequested)
35 🟥 return;
36 🟩 (byte[] frame, int length) = Encode(connection, attPdu);
...
63 ⬛ ArgumentNullException.ThrowIfNull(connection);
64 🟨 if (connection.DisconnectToken.IsCancellationRequested)
65 🟥 return ValueTask.FromException(connection.CreateDisconnectedException($"send {attPdu.OpCode}"));
66 🟩 (byte[] frame, int length) = Encode(connection, attPdu);
...
89 🟩 BinaryPrimitives.WriteUInt16LittleEndian(frame.AsSpan(2), attChannelId);
90 🟨 if (!attPdu.TryWriteLittleEndian(frame.AsSpan(4, pduLength)))
91 ⬛ {
92 🟥 ArrayPool<byte>.Shared.Return(frame);
93 🟥 throw new HciException($"The {attPdu.OpCode} PDU could not be encoded");
94 ⬛ }
95 🟨 if (connection.Logger?.IsEnabled(LogLevel.Trace) is true)
96 ⬛ {
97 🟥 using (connection.Logger.BeginDeconstructedScope(LogLevel.Trace, "Packet", attPdu, orderEntries: true))
98 🟥 using (connection.Logger.ForContext("PacketPayload", frame.AsSpan(4, pduLength).ToArray()))
99 ⬛ {
100 🟥 connection.Logger.LogAttPacketTransmission(
101 🟥 "Host",
102 🟥 "Controller",
103 🟥 connection.ConnectionHandle,
104 🟥 attPdu.OpCode.ToString().ToUpperInvariant()
105 🟥 );
106 🟥 }
107 ⬛ }
...
112 ⬛ private static bool IsAnswer(AttOpCode opCode) =>
113 🟨 opCode
114 🟩 is AttOpCode.ATT_ERROR_RSP
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/HciPacketInFlightHandler.cs — 17/25 changed lines covered (Lines: 68%, Branches: 12.5%)...
51 ⬛ catch (OperationCanceledException)
52 🟥 when (_host.TransportFailure is { } failure && !cancellationToken.IsCancellationRequested)
53 ⬛ {
54 🟥 throw TransportFailed(failure);
55 ⬛ }
56 🟥 catch (OperationCanceledException e) when (IsTimeout(timeoutSource, cancellationToken))
57 ⬛ {
58 🟥 var timeoutException = new TimeoutException("Timeout while waiting for next command to be sent", e);
59 🟥 enqueueActivity?.AddException(timeoutException);
60 🟥 enqueueActivity?.SetStatus(ActivityStatusCode.Error, timeoutException.Message);
61 🟥 throw timeoutException;
62 ⬛ }
...
110 ⬛ private static bool IsTimeout(CancellationTokenSource timeoutSource, CancellationToken cancellationToken) =>
111 🟥 timeoutSource.IsCancellationRequested && !cancellationToken.IsCancellationRequested;
112 ⬛
...
129 ⬛ // Command status events reach this sink only, so a query for the status itself completes here
130 🟨 if (_waitingForCommandStatus)
131 ⬛ {
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/HciDevice.cs — 13/14 changed lines covered (Lines: 92.86%, Branches: 75%)...
141 ⬛ {
142 🟨 if (!_connections.TryRemove(connectionHandle, out AclConnection? removedConnection))
143 🟥 return;
144 ⬛ // The controller freed the link's buffers; frames still waiting for one fail
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/Host/HciHost.cs — 13/14 changed lines covered (Lines: 92.86%, Branches: 83.33%)...
264 ⬛ TransportFailed?.Invoke(this, new HciTransportFailedEventArgs(exception));
265 🟨 _leAclPacketQueue?.Close(exception);
266 🟩 _transportGone.Cancel();
...
416 ⬛ {
417 🟥 _transportGone.Dispose();
418 ⬛ _packetInFlightSemaphore.Dispose();
...✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/Host/GattServer.cs — 5/5 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/L2CapAssembler.cs — 3/3 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/MessageSinks.cs — 18/18 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Hci/Payload/Command/HciLeCreateConnectionCancelCommand.cs — 1/1 changed lines covered Darp.Ble.HciHost.Verify🔴 home/runner/work/Darp.Ble/Darp.Ble/test/Darp.Ble.HciHost.Verify/FakeController.cs — 145/151 changed lines covered (Lines: 96.03%, Branches: 84.62%)...
178 ⬛ /// <summary> Reports a failure of the transport to the host </summary>
179 🟨 public void Fail(Exception exception) => _onError?.Invoke(exception);
180 ⬛
...
212 🟩 _sent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
213 🟨 if (message.Type is HciPacketType.HciCommand && !_answers.TryGetValue(OpCodeOf(message), out answer))
214 ⬛ {
215 🟥 sent.TrySetResult();
216 🟥 throw new InvalidOperationException($"The fake controller has no answer to {OpCodeOf(message)}");
217 ⬛ }
...
233 🟩 sent.TrySetResult();
234 🟨 if (message.Type is not HciPacketType.HciAclData)
235 🟥 return;
236 ⬛ // Completed before the test can read the packet, so an answer the test pushes comes after it
...
264 ⬛ {
265 🟨 _onReceived?.Invoke(new HciPacket(message.Type, message.PduBytes));
266 🟩 delivered.TrySetResult();
267 🟩 }
268 🟥 catch (Exception e)
269 ⬛ {
270 🟥 delivered.TrySetException(e);
271 🟥 }
272 ⬛ }
...Darp.Ble.Mock🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Connections/MockPeripheralDriver.cs — 10/12 changed lines covered (Lines: 83.33%, Branches: 66.67%)...
65 ⬛ // Closing did not see the link yet; it goes down as closing would have taken it
66 🟨 if (closed)
67 🟥 link.DisconnectFromPeripheral();
68 ⬛ // A link that went down meanwhile already forgot itself, before it was added
69 🟨 else if (link.IsDown)
70 🟥 Forget(link);
71 ⬛ return link;
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/MockAir.cs — 5/6 changed lines covered (Lines: 83.33%, Branches: 62.5%)...
44 ⬛ {
45 🟨 if (!central.IsInRange)
46 🟥 return null;
47 ⬛ MockAdvertisingSetDriver[] advertising;
...
72 ⬛ {
73 🟨 if (!ReferenceEquals(listener.Adapter, sender) && listener.Adapter.IsInRange)
74 ⬛ listener.Receive(advertising, now, rssi);
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Connections/MockLink.cs — 26/27 changed lines covered (Lines: 96.3%, Branches: 50%)...
195 ⬛ {
196 🟨 if (!TakeDown(out Action? failOutstanding))
197 🟥 return;
198 🟩 var info = new DisconnectInfo(reason);
199 🟩 _centralSink.OnDisconnected(info);
200 🟩 _peripheralSink.OnCentralDisconnected(Peripheral, info);
201 🟨 failOutstanding?.Invoke();
202 🟩 }
...🔴 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Connections/MockCentralDriver.cs — 18/19 changed lines covered (Lines: 94.74%, Branches: 91.67%)...
51 ⬛ // A link that went down meanwhile already forgot itself, before it was added
52 🟨 if (link.IsDown)
53 🟥 Forget(link);
54 🟩 else if (!_adapter.IsInRange || !set.Adapter.IsInRange)
...✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Connections/MockPeripheral.cs — 2/2 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Connections/MockPeripheralBackend.cs — 8/8 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/Gap/MockAdvertisingSetDriver.cs — 1/1 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/MockAdapterDriver.cs — 18/18 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/MockAdapterSource.cs — 17/17 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/MockBackendOptions.cs — 1/1 changed lines covered ✓ 🟢 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble.Mock/MockRange.cs — 22/22 changed lines covered Darp.Ble🟠 home/runner/work/Darp.Ble/Darp.Ble/src/Darp.Ble/Connections/BlePeripheral.cs — 26/26 changed lines covered (Lines: 100%, Branches: 91.67%)...
203 ⬛ _centrals.Add(connection, central);
204 🟨 if (!closed)
205 ⬛ {
...Legend: 🟥 uncovered · 🟨 partial branch · 🟩 covered · ⬛ non-executable/blank |
3 tasks done
Contributor
Author
|
@codex review Comment generated by claude-opus-5-5 on behalf of Ole |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 24, 2026 17:55
4b28915 to
58dc356
Compare
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
2 times, most recently
from
October 24, 2026 19:30
86e5b36 to
c4f28ac
Compare
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 24, 2026 21:03
c4f28ac to
81b9d2d
Compare
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
2 times, most recently
from
October 25, 2026 19:26
57d50ee to
867c7f6
Compare
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
2 times, most recently
from
October 29, 2026 20:37
609f1f4 to
c81f892
Compare
OleRoss
marked this pull request as ready for review
October 29, 2026 21:59
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c81f892076
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+53
to
+54
| if (hciEvent.Role is not HciLeConnectionRole.Central || _result.Task.IsCompleted) | ||
| return; |
There was a problem hiding this comment.
When a controller reports a failed connection with a nonzero Role field, this guard discards the event before handling its error status. For failed LE Enhanced Connection Complete events, no parameters other than Status are guaranteed valid (Bluetooth HCI specification, Table 4.1). Consequently, a valid controller failure can leave ConnectAsync pending indefinitely without a caller timeout, retaining the attempt semaphore and keeping scans paused. Handle unsuccessful statuses before inspecting Role; apply the role filter only to successful connection events.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Right: of a failed event only the status is valid. The attempt now ends on a failed event whatever its role field says, and the role and peer checks apply to successful events only. Fixed in #168 (3026d52, fix(hcihost): End a connect attempt on a failed event of any role), with the existing give-up test now run for both role values.
Comment generated by claude-opus-5-5 on behalf of Ole
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 30, 2026 07:37
c81f892 to
deaaf82
Compare
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 30, 2026 07:39
deaaf82 to
5dc19c5
Compare
3 tasks done
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dc19c55f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+127
to
+130
| AttReadRsp response = await Client | ||
| .QueryAsync<AttReadReq, AttReadRsp>(new AttReadReq { AttributeHandle = handle }, request) | ||
| .ConfigureAwait(false); | ||
| return response.AttributeValue.ToArray(); |
There was a problem hiding this comment.
When a characteristic or descriptor value exceeds the negotiated ATT MTU minus one byte, this returns only the first fragment as a successful read. For example, with MTU 23, a 40-byte value becomes a 22-byte result; the existing FakePeripheral reproduces this by limiting its AttReadRsp to Mtu - 1 bytes. Both characteristic and descriptor reads use this method, so callers silently receive incomplete data. Continue with Read Blob requests when the response fills the available payload, until the complete value has been retrieved.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Right for this PR. Fixed later in the stack, in #167 (19c5695, fix(hcihost): Answer every request a peripheral sends to our central): when a read response fills the payload, the central continues with Read Blob requests until the value is complete, up to 512 bytes, for characteristics and descriptors.
Comment generated by claude-opus-5-5 on behalf of Ole
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 30, 2026 07:47
5dc19c5 to
3289bac
Compare
The v1 HciHost factories built on base classes that v2 removed, and its HCI commands timed out on the system clock even though the factories accepted a TimeProvider. Adapters now open through the backend drivers, and command timeouts, including the wait for the command slot, run on the manager's clock.
Connecting waits for the controller to accept the attempt and then for the link separately, so other commands, including the cancel, are not blocked meanwhile. A link that comes up while cancelling is taken down again. When a link goes down, its owner learns why before anything waiting on it fails.
Discovery, reads, writes, descriptors and notifications run on the core's request queue, one ATT transaction at a time, and discovered attributes keep their identity on the link. Writes without response send an ATT write command; v1 sent a write request.
Subscribing replaced the list of subscribers without synchronization, so a request subscribing while another unsubscribed could drop one of them and wait forever for its answer. Commands, ATT requests and links subscribe from their own threads while the read loop publishes.
…its caller cancelled A connection attempt cancelled between sending the command and its status looked cancelled before it started, so the controller kept trying to connect and nothing cancelled it. The caller can still cancel while a command waits for the command slot.
Writes without response complete once the controller has a buffer for them, which paces a loop of writes, and a write waiting for a buffer can be cancelled without sending anything. Links take turns at the buffers, answers to the peer go first, and the packets of one PDU are never interleaved. PDUs are encoded into pooled buffers; the transport copies each packet when it is queued. v1 always served the first link and miscounted the size of continuation fragments.
A handler cancelled because its central disconnected answered with an unlikely error, which the Mock delivered before the link went down, so a central saw a GATT error instead of the disconnect. The request now gets no answer, as on a link that is going down. ConnectedCentral.Disconnected now completes once the link is down, before the central's handlers return, so a handler that awaits it no longer waits for itself. A server app learns of connecting centrals from BlePeripheral.WatchCentralsAsync instead of polling ConnectedCentrals, and tests take a simulated peripheral out of range and back with MockRange.
Ids and names no longer change with a method's name, and the catalog lists them next to the core's. Unused log methods and span starters are gone.
A command waiting for its answer when the serial port went away waited for its timeout, so a connection attempt ended with a timeout instead of a connect error. Commands now fail as soon as the transport fails, and the central reports a lost transport or a silent controller as a connect error.
The event skipped its status byte, so the handle, interval and timeout were read one byte off.
OleRoss
force-pushed
the
feat/v2-hcihost-central
branch
from
October 31, 2026 08:53
0912236 to
edadecf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
v1's HciHost sat on base classes that v2 removed, timed out on the system clock, always served the first link with the controller's buffers, and sent write requests for writes without response.
This PR ports the HCI backend to the driver layer for the observer, broadcaster and central roles; the peripheral role follows in the next PR. The core keeps the request queue and the ATT timeout, the driver sends one ATT transaction at a time, and links share the controller's ACL buffers. It also carries:
BlePeripheral.WatchCentralsAsyncreports every central that connects,ConnectedCentral.Disconnectedcompletes once the link is down, andMockRangetakes a simulated peripheral out of range and back.Changes
Review
src/Darp.Ble.Hci/AclPacketQueue.cs,src/Darp.Ble.HciHost/Connections/HciHostCentralDriver.cswithConnectAttempt.cs,Gatt/AttClient.csandsrc/Darp.Ble.Hci/HciPacketInFlightHandler.cs.NotSupportedExceptionuntil the peripheral role arrives in the next PR.BleConnectErrorKind.Unreachable.Testing
WatchCentralsAsyncandMockRange.Impact
HciHostBleFactory,SerialHciHostBleFactoryandSingleSerialHciHostBleFactory→AddSerialHciHostandAddHciHost, with theBleManager'sTimeProvider.HciSettings(DefaultHciCommandTimeoutMs, DefaultAttTimeoutMs)→HciSettings { CommandTimeout }, with the core's 30 s ATT timeout.QueryAttPduAsync,IAclPacketQueue,IAclConnectionand the emptyGattClient→AclConnection.SendGattPacketAsync.ITransportLayer.Enqueuebefore it returns.ConnectedCentral.DisconnectedandDisconnectAsyncno longer wait for the central's handlers.InvalidOperationException.Checklist
PR generated by claude-opus-5-5 on behalf of Ole