search

hbldh / bleak

A cross platform Bluetooth Low Energy Client for Python using asyncio
MIT License
1.61k stars 279 forks source link

backends/winrt: don't throw exception for properly configured GUI apps #1581

Closed dlech closed 3 weeks ago

dlech commented 1 month ago

In commit 4a653e6 ("backends/winrt: raise exception when trying to scan with STA") we added a check to raise an exception when trying to scan when PyWinRT set the aparatment model to STA. However, properly working GUI apps will have the apartment model set to STA but Bleak will still work because there is something pumping the Windows message loop.

We don't want to raise an exception in this case to avoid breaking working apps. We can improve the test by checking if the current thread is actually pumping the message loop by scheduling a callback via a the win32 SetTimeout function. If the callback is called, then we know that the message loop is being pumped. If not, then we probably are not going to get async callbacks from the WinRT APIs and we raise an exception in this case.

dlech commented 1 month ago

Hi @MarkusPiotrowski, could you please test this with your BeeWare app?

I've tested it with your tkinter example and it seems to work.

MarkusPiotrowski commented 1 month ago

In my setup, it fails in line 138 of ...bleak\backends\winrt\util.py with AttributeError: module 'asyncio' has no attribute 'timeout'

The asyncio.timeout() context manager was added in Python 3.11 (I'm on 3.10). I don't know your dependency policy, but requiring 3.11 is possibly quite strict? Ah, I see, you are testing from Python 3.8 on (so there is no test for this PR (yet)?)

dlech commented 1 month ago

The asyncio.timeout() context manager was added in Python 3.11 (I'm on 3.10). I don't know your dependency policy, but requiring 3.11 is possibly quite strict? Ah, I see, you are testing from Python 3.8 on (so there is no test for this PR (yet)?)

This should be fixed for older Python now. Unfortunately, we can't test a lot of this stuff since CI doesn't have Bluetooth hardware, but we probably could add tests for these helper functions.

MarkusPiotrowski commented 1 month ago

Yes, this is now working for me, both with allow_sta() set or not.

dlech commented 1 month ago

Great, thanks for testing! I assume this is a good enough solution for you?

dlech commented 1 month ago

Tested with Kivy as well.