Add async open support for various network-based devices #720
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #720 +/- ##
==========================================
+ Coverage 86.87% 86.88% +0.01%
==========================================
Files 69 69
Lines 3559 3562 +3
==========================================
+ Hits 3092 3095 +3
Misses 467 467 ☔ View full report in Codecov by Sentry. |
Technically this is a race condition as it stands.
All the callers of SensorsBase.__init__() now call it with start_polling=False, so we can just remove the option.
alexdewar
force-pushed
the
net-devices-async
branch
from
57e29fd to
ca44260
Compare
alexdewar
requested review from
dc2917,
jamesturner246 and
dalonsoa
and removed request for
dc2917
| @@ -122,6 +123,10 @@ def _on_reply_received(self, reply: QNetworkReply) -> None: | |||
|
|
|||
| # If the status has changed, notify listeners | |||
| if new_status != self._status: | |||
| # On first update, we need to signal that the device is now open | |||
| if self._status == SpectrometerStatus.UNDEFINED: | |||
There was a problem hiding this comment.
Isn't it a bit odd that when it's undefined, it is set as open?
There was a problem hiding this comment.
It is kinda ugly. Plus, if the server actually returned a status of UNDEFINED (this would be dumb -- but sometimes hardware does dumb things) then that would actually break our code.
I've changed it to be None instead.
There was a problem hiding this comment.
Sorry, to clarify -- I set it to UNDEFINED in __init__ before we actually have a status, so it's a sentinel value. But it's probably not a good idea, for the reason I mentioned
Previously we were using `SpectrometerStatus.UNDEFINED` as the initial value, but this is a bad idea because: 1. It's a bit ugly 2. The server could conceivably return `UNDEFINED` as a status, which would break our code -- probably best not to rely on this not happening
Description
This PR changes the various network-based devices to use the
async_openfeature. While I haven't been able to test this on the real hardware, unfortunately, I have checked that HTTP requests are sent after device opening (of course, they time out). I'm pretty confident it will work with the real hardware as the changes are pretty minimal, though you can never be sure. Jon has volunteered to test it while he's away, so I think we should just merge it; if it breaks things we can always revert and we don't want to be hanging around waiting to merge this.Closes #415. Closes #666. Closes #667.
Type of change
Key checklist
pre-commit run -a)pytest)mkdocs build -s)pyinstaller-built executable works (if relevant)Further checks