Sorry for the very delayed response >_>
The PDO directly having children is a bit weird, although not impossible. Is this what Windows does?
Yeah. The PDO hierarchy for Bluetooth PHYs, devices and services is odd, but probably makes sense for how the Bluetooth stack on Windows is structured. I have provided an example of what `pnputil` looks like on Windows [here](https://gitlab.winehq.org/-/snippets/23). There is an enumerator bus child for every PHY, which then is the parent of remote devices found on that radio. The `bthleenum` and `bthenum` drivers are the minidrivers that probably implement the HCI layer for the Bluetooth PHY, which is probably what necessitates an additional enumerator bus.
To be clear what's weird is that the PDO has no FDO attached. Usually you end up with a FDO in a different .sys driver which then handles IRP_MN_QUERY_DEVICE_RELATIONS. There's nothing inherently wrong with handling them from the PDO that I can think of, but it's odd.
If Windows arranges its drivers differently and it's easy for us to follow that structure, we probably should.
Why the separate CS?
Some properties, like the RSSI and transmission power, can be updated quite frequently during advertisement/inquiry (though it hasn't been implemented yet). Making the locking slightly more fine-grained lets updating them not step on the toes of LE-related IOCTLs on devices.
I would count this as premature optimization, and really, I'd be surprised if any overhead within the Bluetooth driver is going to beat the overhead of interacting with the driver over RPC.
This logic seems weirdly complex, can we just use offsetof(services, services[count]) instead?
I think it should also be possible to define the struct using a sizeless array. I don't know if you can safely use sizeof() in that case, but I'm not sure we want to use sizeof() anyway, probably better is offsetof(services, services[0]).
Well, the idea was to have the same semantics as `IOCTL_BTH_GET_DEVICE_INFO`, but I suppose that isn't really needed since this is strictly a Wine-only IOCTL.
Is there a particular reason not to just use BTH_LE_GATT_SERVICE in the driver directly, and move most of this logic there?
Hm, the only substantial "logic" in this code IMO is in `uuid_to_le`, and I made the choice to perform this conversion in userspace as:
- BlueZ only provides the full UUID for all GATT entities, and I feel the driver should do as little as possible with the data it receives from the bluetooth service.
Why, though? Frankly, given the other option is making bluetoothapis.dll do as little as possible, that would centralize all the logic in one place, and effectively avoid even needing to design a separate API.
- Using `BTH_LE_UUID` also increases the size of the IOCTL buffer (albeit by a single `BOOLEAN`).
I don't think this matters.
Any reason not to put this code in main.c? Same for the tests in the next commit.
The Win32 LE GATT API has nothing in common with the classic one, including separate headers, device interfaces and IOCTLs. For that reason, I thought it'd be more appropriate to split the code. gatt.c will likely be a larger file than main.c (for both the dll and tests) in the future, once we start achieving API parity.
Maybe it's fine, but generally a large collection of files which are each only a couple hundred lines is frowned upon. Even 3000 lines isn't that intimidating.