MarkusPiotrowski / bleekWare

A limited Bleak complement for accessing Bluetooth LE on Android in Python apps made with BeeWare.
MIT License
5 stars 2 forks source link

Moved global collections into the Client class #1

Open xperroni opened 2 months ago

xperroni commented 2 months ago

Previously, the Client class relied on global collections to exchange data with the Android callback objects. This would cause problems when trying to simultaneously connect to multiple devices. Fixed by moving the global collections into the Client class.

A note about member naming conventions: in Python, "true" private members are prefixed by two underscores (__), while members prefixed with only one underscore (_) are still public, but by convention should not be accessed by external code unless strictly necessary. The committed changes follow this convention, e.g. Client._received_data is kept accessible so it can be modified by Android callbacks, while Client.__services is only accessed from inside the class itself.

References:

MarkusPiotrowski commented 2 months ago

@xperroni Thank you very much for your contribution. Clearly, you know more about programming than me (being a hobbyist programmer). Still, I want to keep understanding my code and for the moment I see more changes that you introduce than that are explained in your starting comment. So it would be nice if you can explain a little bit more what you changed for what reason.

I'm aware about Python's "private" conventions and I don't care about having 'real' private members, Therefore, I usually stick to the single underscore pattern.

Also, I do hesitate to import logging. Isn't this quite heavy-weight for this application? I confess that my status_message list was some work in progress.

xperroni commented 2 months ago

Hi Markus,

So it would be nice if you can explain a little bit more what you changed for what reason.

Of course. Apart from moving the global collections inside the Client class and inserting a logger, the main change in this commit is the removal of the Client._get_services() method, with its logic relocated inside _PythonGattCallback.onServicesDiscovered(). The aim is to simplify the service configuration logic: instead of passing a list of android.bluetooth.BluetoothGattService objects to be processed internally by Client, we already give it the BLEGattService instances it needs.

However, because connect() checks whether the __services list is empty in order to synchronize with the connection callback, we can't just iteratively add the service objects to the list either. That's why onServicesDiscovered() first collects the BLEGattService objects in a local list, then passes them to Client all at once through the Client.services property setter. Additionally, the setter uses clear() and extend() to update the __services list instead of replacing it, so that we update the exact same list being waited on by connect().

I think that's all worth mentioning on this commit, but let me know if anything else caught your attention.

Also, I do hesitate to import logging. Isn't this quite heavy-weight for this application?

I suppose "your mileage my vary", but I have been using it quite extensively while working on an Android project, and I haven't really noticed any performance issues. In any case, in a typical application the BLE client will rarely connect / disconnect more than a few times during the application's lifetime, so I don't see it becoming an important factor in this case.