Fix issues with Qt 5.15 where QSocketNotifier.activated became overloaded #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Explicitly use int socket argument, otherwise slot would receive a QSocketDescriptor resulting in silently ignored fd_to_key map misses
|
Related merge request in asyncqt: gmarull/asyncqt#28 |
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
==========================================
- Coverage 64.84% 64.75% -0.09%
==========================================
Files 4 4
Lines 657 661 +4
==========================================
+ Hits 426 428 +2
- Misses 231 233 +2
Continue to review full report at Codecov.
|
|
Nice work! I'll try and get a new release published to PyPI tomorrow. |
|
This is now live in version |
nocarryr
added a commit
to nocarryr/jvconnected
that referenced
this pull request
Dec 9, 2020
Eventloop interaction issues fixed as of CabbageDevelopment/qasync#12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After upgrading to Qt 5.15/PySide2 5.15.1 I noticed that the asyncio-Qt mainloop integration was completely broken - websockets stayed hanging in a connecting state, async HTTP requests never returned a response i.e. no callbacks were ever called.
Troubleshooting revealed that the
__on_read_activatedslots in_unix.pyreceived aPySide2.QtCore.QSocketDescriptor object. Those were obviously not found in the_fd_to_keydirectory mapping, as the key expected there is the (int) file descriptor.In the Qt documentation I found that the QSocketNotifier.activated behavior changed with Qt 5.15, whereas previously only the
QSocketNotifier::activated(int socket)signal existed, now this signal became overloaded withQSocketNotifier::activated(QSocketDescriptor socket, QSocketNotifier::Type type)as arguments.Long story short, this pull requests fixes the issue by explicitly connecting the signal with "int" soket fd as argument.