-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat!: Full async networking #90
Conversation
There may still be valid interfaces/devices that we should return
Removing old sync approach, and replacing with event driven. Should be no behavioural changes.
Removing old sync approach, and replacing with event driven. Should be no behavioural changes.
Removing old sync approach, and replacing with event driven. Should be no behavioural changes.
Removing old sync approach, and replacing with event driven. Should be no behavioural changes.
Broadcast is not reliably supported across OS on lo, and would fail in many cases. Unicast will still effectively exercise nearly all code, even where broadcast was expected.
Also cleaned out a duplicated test in network.py
It's going to be handled in the next update_state request.
- Bind OK - State Updates - Command Response
Hell @cmroche , I have been working on making this library fully type hinted. I was about to make a PR for your review then I saw these changes. Can you let me know if this PR is ready to be merged so I can rebase and update my work. |
# It could be either a v1 or v2 key | ||
key = GENERIC_KEY[0] if obj.get("i") == 1 else self._key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to interrupt your coding flow, but I can see that you incorporating the new KEY and encoding for new models with firmware 1.2.1. Would you mind if I through in a few suggestions. I have a new Gree AC and I would like to be able to control it using HA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to add the GCM encryption support following this change, it's not actually implemented yet though.
To be pedantic. V1.2.1 sounds like you're referring to the hardware revision, my device reports 1.2.1 and uses the older encryption technique. We won't be able to select based on that version reported in the scan result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
V1.2.1 is the firmware version showing on Gree app.
The best approach is to try the GCM if the current encryption causes a timeout.
If both make a timeout then we raise a timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... they have a weird concept of versions, but yes. Timeout approach is what I was thinking too.
That said, could you share responses from the scan and bind commands from your 1.2.1 devices please? Maybe there is something in there, I can compare with examples I have on hand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the response from the scan
{"t": "pack", "i": 1, "uid": 0, "cid": "", "tcid": "", "pack": {"t": "dev", "cid": "9424b8bb14bc", "bc": "00000000000000000000000000000000", "brand": "gree", "catalog": "gree", "mac": "9424b8bb14bc", "mid": "10001", "model": "gree", "name": "", "lock": 0, "series": "gree", "vender": "1", "ver": "V2.0.0", "ModelType": "32772", "hid": "362001065279+U-WB05RT13V1.21.bin"}}
hid is showing V1.21
When trying to bind it just timesout.
We log an exception, raising doesn't result in callers being able to do anything about it
Awesome. Been working slowly to add typehints, so will be greatly appreciated. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #90 +/- ##
==========================================
+ Coverage 92.27% 94.93% +2.65%
==========================================
Files 5 7 +2
Lines 686 651 -35
==========================================
- Hits 633 618 -15
+ Misses 53 33 -20 ☔ View full report in Codecov by Sentry. |
BREAKING CHANGE: API calls no longer block waiting for device response, use the add handler to listen for updates from the device.
# [2.0.0](v1.4.6...v2.0.0) (2024-07-02) ### Features * Full async networking ([#90](#90)) ([4587984](4587984)) ### BREAKING CHANGES * API calls no longer block waiting for device response, use the add handler to listen for updates from the device.
🎉 This PR is included in version 2.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
I am done with the big rework. Next I will look at the GCM encryption this week but changes will be much smaller if you want to still work on the type hinting, perhaps I could get you to test it for me once it's ready as I do not have one of these devices. |
Replaced the old send then receive approach to networking with asyncio.DatagramProtocol based classes
This removed a lot of duplicate network code as discovery was already using this approach, and allows handling on device responses when they are received as opposed to waiting on them explicitly.
BREAKING CHANGE:
While interfaces are unchanged, you can no longer expect a device response is already received when
push_state_update
orbind
have returned.Responses will come through their appropriate handlers once they have been received.