Is there a need for the local cache?
tony-gutierrez opened this issue · 9 comments
Windows has its own cache of services and chars, so do we really need the local cache that winrt currently has?
The local cache is not currently populated by DiscoverServices, so subsequent calls for a single service or char will hit windows again, windows will serve from its ble cache, and then we add to local cache.
It seems like the local cache could be eliminated.
@tony-gutierrez You're right! The way it's currently implemented it will only ask the windows ble cache.
I think we could improve this by first checking the windows ble cache with BluetoothCacheMode::Cached and if that fails let windows query the device with BluetoothCacheMode::Uncached.
One thing I'm not sure anymore but IIRC we need to keep a reference to the characteristic to get notifications for it
My understanding is that BluetoothCacheMode::Cached will automatically query the device if there is no cached value. So it shouldn't fail.
Deets here: https://docs.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.bluetoothcachemode?view=winrt-19041
Actually the way it's implement currently is correct, as all services need first to be discovered they should already be in cache once we request one.
Yeah, that is what happens. On second discovery, it misses local cache, and hits windows. On 3rd it would probably hit the local cache. I am just thinking we don't need the local cache. We might need to track chars like you said, but I don't think the local cache check before reading a service is needed.
There is now expanded information on the cache at the MS url linked above.
Was able to get a good perf improvement in my version by using Cached mode, in addition to not trashing the local cache on disconnect. Have not tested eliminating the local cache and just relying on windows, but seems like a logical next step.
As far as I can see we're already using BluetoothCacheMode::Cached everywhere. If we can rely on the windows caching then we could definitely remove our custom cache, after all it'll be less code and less of a chance to get out of sync with the device.
Ah, somehow my code started with some Uncached, from a fork I guess. I'll be in touch if I get a chance to remove local cache.
Removing cachedServices seems to cause 2 problems:
- You lose the ability to maintain cachedServices during disconnect, which for my use case allows for rapid retries of a failed procedure.
- Notify seems to break, it stops announcing characteristic value changes.
Closing this for now.