Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential performance improvements on Windows? #56

Closed
bojanpotocnik opened this issue Mar 29, 2019 · 2 comments
Closed

Potential performance improvements on Windows? #56

bojanpotocnik opened this issue Mar 29, 2019 · 2 comments

Comments

@bojanpotocnik
Copy link
Contributor

  • bleak version: 0.3.0 (forked develop branch)
  • Python version: 3.7.1
  • Operating System: Windows 10 (10.0.17763-SP0)

Description

I am wondering about spots where the potential performance improvements could be made. This library is working great overall, however I am pushing the performance limits. My device is sending 500 notifications per second (of BLE max. 800) and I am not able to receive all of them. Using the code posted under What I Did below, the print output is:

Missed 1 samples (799 vs. 800 on remote)
Missed 11 samples (801 vs. 812 on remote)
Missed 5 samples (813 vs. 818 on remote)
Missed 1 samples (823 vs. 824 on remote)
Missed 6 samples (829 vs. 835 on remote)
Missed 1 samples (840 vs. 841 on remote)
Missed 1 samples (846 vs. 847 on remote)
Battery voltage = 4059 mV
Missed 2 samples (851 vs. 853 on remote)
Missed 3 samples (856 vs. 859 on remote)
Missed 2 samples (863 vs. 865 on remote)
Missed 1 samples (876 vs. 877 on remote)
Missed 2 samples (881 vs. 883 on remote)

iOS and Android apps are able to receive the notifications, so I set my goal to be able to receive them with Bleak as well.
I don't know where it would be the best to start. Would Make binary wheels for Windows noticeable improve performance?

What I Did

I am currently working on my own branch with following improvements:

  • Faster search (bleak.backends.dotnet.search.find(), commits 4e838f0 and 6397421) using BluetoothLEAdvertisementWatcher, which listens for BLE advertisements (without enumeration) and also supports additional filtering based on advertisement and scan response data.
  • Faster connection using BluetoothLEDevice.FromBluetoothAddressAsync to directly get the device instance from the MAC address and only perform (2 s) discovery combined with BluetoothLEDevice.FromIdAsync if the first method is not successful (6f130fa)
  • Using BleakGATTCharacteristic directly without always re-querying them in I/O methods (002e7f9). This is more usability than performance improvement, as I usually make (data)classes for my services and retrieve all characteristics in the __init__.
    More important, this also supports the cases where the same SIG characteristic (e.g. Analog) is present in multiple services as one can call service.get_characteristic() on multiple service instances. Stripped example of my usage:
@dataclass(init=False)
class PPGSensorService:
    def __init__(self, client: BleakClient) -> None:
        uuid_fmt = "0000{:04x}-99c1-8b45-835a-c30b7c408871"
        
        self.client: BleakClient = client
        self.service: BleakGATTService = client.services[uuid_fmt.format(0xe101)]
        self.char_adc_sample: BleakGATTCharacteristic = self.service.get_characteristic(uuid_fmt.format(0xe201))
        self.char_settings_basic: BleakGATTCharacteristic = self.service.get_characteristic(uuid_fmt.format(0xe202))
        self.char_settings_advanced: BleakGATTCharacteristic = self.service.get_characteristic(uuid_fmt.format(0xe203))
        self.char_analog: BleakGATTCharacteristic = self.service.get_characteristic(ble_uuid_to_128bit(0x2A58))
        # Raise error if any char is not found - device not supported.
        self.sample_counter: int = 0

    async def enable_notifications(self) -> None:
        await self.client.start_notify(self.char_adc_sample, self._on_adc_sample)
        await self.client.start_notify(self.char_settings_basic, self._on_settings)
        await self.client.start_notify(self.char_settings_advanced, self._on_settings)
        await self.client.start_notify(self.char_analog, self._on_analog)

    def _on_analog(self, uuid: str, data: bytearray):
        voltage = struct.unpack("<H", data)[0]
        print(f"Battery voltage = {voltage} mV")

    def _on_adc_sample(self, uuid: str, data: bytearray) -> None:
        unpacked = struct.unpack("<I4i4b4B4BH", data)
        sample_counter = unpacked[0]
        # ...
        self.sample_counter += 1
        if self.sample_counter != sample_counter:
            print(f"Missed {sample_counter - self.sample_counter} samples"
                  f" ({self.sample_counter} vs. {sample_counter} on remote)")
            self.sample_counter = sample_counter

    def _on_settings(self, uuid: str, data: bytearray):
        print(f"_on_settings({uuid}, {data})")
        # ...

    async def set_measuring(self, enabled: bool) -> None:
        await self.client.write_gatt_char(self.char_settings_basic, bytearray([0x01 if enabled else 0x00]))

I haven't opened the PR as my changes are Python >=3.6.

@bojanpotocnik
Copy link
Contributor Author

After opening this issue, I stopped debugging the PC side in the anticipation of your answer and switched to developing new feature in the firmware.

After working on firmware and analyzing debugger's RTT output, I discovered that when connecting to the PC, the PPCP (Peripheral Preferred Connection Parameters) were not updated - in FW I had Minimum Connection Interval set to 7.5 ms (minimum), but Maximum Connection Interval was set to 500 ms. It looks like the Android phone automatically updated the intervals to shortest allowed, which Windows does not (power saving?).

After setting the Maximum Connection Interval = Minimum Connection Interval = 7.5 ms in the firmware, there are no lost notifications anymore.

Sorry for the inconvenience.

@hbldh
Copy link
Owner

hbldh commented Mar 30, 2019

Interesting read, and good to see that bleak actually holds up even though it is an experimental wrapping of .NET code into Python! Good to see that you could resolve it; that was a setting in the peripheral's code, am I right?

Regarding the proposed changes:

  • I did try a BluetoothLEAdvertisementWatcher, but I am not sure why I did not replace the current solution with that instead. I will look at your code durint the next week.
  • I never got the FromBluetoothAddressAsync to work reliably enough to be used as I recall. Interesting to see if you did!
  • Adding a use_cached method to enable the use of the Windows cached values for characteristics. But the client.service.get_characeristic does not perform no fetching from the device; all those characteristics pre-fetched during connection establishment. Only actually calling read_gatt_char or similar will actually read from device. I am not sure what you mean here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants