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

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

Merged
merged 11 commits into from
Jun 1, 2024

Conversation

dlech
Copy link
Collaborator

@dlech dlech commented May 24, 2024

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.

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
Copy link
Collaborator Author

dlech commented May 24, 2024

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
Copy link

MarkusPiotrowski commented May 24, 2024

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)?)

bleak/backends/winrt/util.py Show resolved Hide resolved
bleak/backends/winrt/util.py Show resolved Hide resolved
bleak/backends/winrt/util.py Outdated Show resolved Hide resolved
docs/troubleshooting.rst Show resolved Hide resolved
bleak/backends/winrt/util.py Show resolved Hide resolved
bleak/backends/winrt/util.py Outdated Show resolved Hide resolved
@dlech
Copy link
Collaborator Author

dlech commented May 24, 2024

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
Copy link

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

@dlech
Copy link
Collaborator Author

dlech commented May 24, 2024

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

@dlech
Copy link
Collaborator Author

dlech commented May 24, 2024

Tested with Kivy as well.

@dlech dlech merged commit bd8f022 into develop Jun 1, 2024
14 checks passed
@dlech dlech deleted the winrt-improve-sta-check branch June 1, 2024 16:14
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

Successfully merging this pull request may close these issues.

None yet

2 participants