-
Notifications
You must be signed in to change notification settings - Fork 157
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
[WIP] Initial thinq2 support, works with LA090HSV5 AC #132
base: master
Are you sure you want to change the base?
Conversation
Hm, it seems like the formatter tool (black) I used to fix the flake8 errors makes it hard to compare the PRs. Maybe black should be perhaps added as a precommit hook? |
@no2chem just had a play, was hoping to give this a test, unfortunately returning no devices for an Aus account - any ideas? |
Is there any output at all? It doesn't crash but doesn't return any devices either? I guess it might be useful to try to add some debugging to it, maybe I'll look into adding that in a bit |
Correct; just blank - initially thought it might be the country / lang, but still nothing with it corrected - I have managed to get other PR's to list the device succesfully (but they hadn't implemented anything further). Happy to help in testing - I'm familiar with python, and can hack stuff together, but not always in the most pretty way. |
I would like to contribute at least testing it. I even don't know if here it's the right place to comment this, but I've cloned @no2chem repo and I could see both AC units (same model AC RAC_056905_WW)
2020-11-22 14:29:08 INFO [wideq.example] Session expired. |
@viniciuscordeiro Thanks. I think for now, this is the right place. Could you tell me more about your setup? Is your country code US/en-US? @davewatson91 What devices do you have in your account? |
For sure. Actually my country code is BR and language pt-BR (Brazil). I've tried in a proxmox server: `[~/wideq.no2chem]$ uname -a [~/wideq.no2chem]$ ./example.py ls [~/wideq.no2chem]$ ./example.py mon device1_id
|
@viniciuscordeiro @davewatson91 debugging through this (since you are both on non-us servers), I wonder if this has something to do with the TLSv1 issue. It seems so other forks force TLSv1. I'll try to add that as an option to see if it fixes things. |
<device_id>: Air Conditioner (AC DUCT_626301_WW) I get this returned by using @gladhorn fork (https://github.com/gladhorn/wideq/) Not sure if anything can be used from there without re-inventing the wheel? That fork seems dead, and unfortunately never went further than listing devices, so can't confirm anything else from it. |
Thanks so much for getting this going, @no2chem!! I've merged the Black changes, and I'm not sure if I've messed things up somehow, but it seems like that somehow created more conflicts than there were before? Is there any chance there's a way to disentangle the "real" changes you made here from the formatting changes so it's possible to review this diff? (Or was applying Black to the existing code a mistake that we should now back out?) |
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.
Just a few comments I was able to pinpoint before we have the whole formatting situation sorted out.
@@ -6,3 +6,5 @@ __pycache__ | |||
.vscode/ | |||
# Ignore Mac system files | |||
.DS_Store | |||
# Ignore devcontainer for now (should this go in source control?) | |||
.devcontainer |
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.
Not sure what this is—so I'm not sure if it belongs in source control. 😇
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.
I see. .devcontainer is a vscode feature which defines a container you can do development in. Some projects (including home assistant) include this in source control so that developers have a consistent development environment. I develop in a devcontainer, so I have it in the gitignore so it doesn't get added when I do a git add .
.
Is this something you'd be interested in? Otherwise I'll just figure out how to get git to skip it without polluting the repositories' gitignore.
except ValueError: | ||
print('status data: {!r}'.format(data)) | ||
else: | ||
""" | ||
else: |
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.
Seems like we'll want to bring this back at some point.
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.
Yeah. It's commented out for now because the v1/v2 api produced very different data.
DEFAULT_LANGUAGE = "en-US" | ||
|
||
# v2 | ||
API_KEY = "VGhpblEyLjAgU0VSVklDRQ==" |
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.
To confirm, is the goal here to simultaneously support v1 and v2 of the API? If not, we can just delete all the stuff that is v1-specific.
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.
Hi - I'm lurking a bit here, however simultaneous v1/v2 support would help me. I have a mix of both in my home. I can do testing :) Cheers!
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.
Well, that is one of the things I was unsure about. Are there devices that ONLY support the V1 api? If so, then it would make sense to support both APIs at the same time. Otherwise, it'd make sense to ONLY support v2.
@sampsyo Yes, I think I just need to rebase this PR, which I'll do in a moment. |
@sampsyo The latest force-push should get rid of the formatting diffs (thanks for merging the black PR). I'd like to pause work until we figure out whether we need to support the v1 API still. Supporting both APIs would take us in a different path than just supporting the v2 API, I think. |
Yes, that's a great question. It would obviously be much simpler if we only needed to support the new API. I don't currently know whether there is an issue with that and older devices or older setups. I seem to recall some commentary to the effect that supporting the older API is not necessary, but @craigsheppard might know more… is there any chance you could give this PR a try, @craigsheppard, and verify that all your devices still work via the v2 API? |
Hi, I was speaking from a place of ignorance - I just assumed v1 devices would not be available. I'll see if I can get this tested with my devices today. |
Awesome; thank you so much for looking into it! |
I just tested the @no2chem I'm unable to control the device, there's a
Aside: I'm not currently able to see my v2 device, however at the moment it's a hardware issue (I deleted my LG account and created a new one, and during the setup process, my v2 device has gone into a mode where I can't connect to it with the LG app - flashing WiFi light and no AP advertised). Looks like I might be able to reset it by turning the power off for a while, but it's hardwired so maybe tomorrow - but I don't believe you need me to test both. |
I just cloned https://github.com/tinkerborg/thinq2-python and my v1 device is not showing up in the list, so perhaps we need to support both APIs? |
Oh wow, that's quite confusing!! I wonder what's going on here… I guess it's not surprising that this current branch would crash, but I wonder why @tinkerborg's implementation would not list your device. Mysterious… |
@tinkerborg's is v2-only - perhaps my device is stuck in v1 land? Isn't this @no2chem branch currently supporting both, which explains why I can list it? |
@craigsheppard No, the branch currently doesn't support controlling v1 devices. I mainly focused on attempting to produce the minimal changeset to support v2 devices. I guess one of the reasons for the differences is probably that I get the device list using the old api (via the dashboard) whereas @tinkerborg may be using the v2 enumeration api. This is too bad... I wonder if you re-pair a v1 device if it'll end up showing as a v2 device? Sort of strange for LG to have a backend the differentiates between the devices. |
Real weird. Could we jettison most of the v1 API but just keep around enough to list the devices? |
@sampsyo It seems like the v1 API is required to control the v1 devices though. There is a platformType entry in the json from the device list which seems to list whether the device should use the v1 API or v2 API. Notably, the v2 API has different keys for data and a different mechanism (MQTT) for monitoring devices in real time.
I'll add a quick patch that will at list display that.... I suspect we can keep the v2 login mechanism, but we'll have to check the platformType to determine whether to control a device as v1 or v2. Of course, if deleting a device and readding it resurrects it as a v2 device, that would be ideal. |
@craigsheppard (or anyone else with a v1 device). If you could run ls with the update I just pushed (removing your device ids), I'd appreciate it... It should produce output like:
|
Just to contribute, Im from Brazil and have the same output like @viniciuscordeiro "mon" is working fine: on; COOL; cur 77°F; cfg 70°F; fan speed LOW "turn" and "set-temp" |
Please let me know if you need help testing. I have a mixed thinq_1 and thinq_2 devices: v1 devices non of the functions work. I can monitor thinq2 device. |
If LG made a V2 API, you may consider to split also and made a V2 wideq libreary ? I use wideq for a jeedom plugin (jeedom is a french php home assistant soft), and of course, if no other solution works, I will develop a V2 version of the plugin with your V2 wideq lib using the V2 api :) |
For me the @gladhorn fork works with the
With this fork I get no output. This fork:
|
I have the same as @ronaldvdmeer: This branch: While @gladhorn:
However, I cannot list anything with @gladhorn:
|
It's because there is an error in Session.post function of core.py. The code call thinq_request without pass the country and language, and because this, its assumes the default country (US/en). The Session.get function is correct, then the operations like "mon" gonna work in Brazil and other countries. Changing the post function to:
will fix the issue. |
@nemerdaud THANK YOU! @no2chem |
Hi all, sorry, end of the year deadlines really kept me from doing anything.
Yes, it seems like we will need to do that Thanks, will fix that Yeah, are you using it with HA? I think the fan control works. Otherwise we will have to update example.py to control those functions. |
Hi guys, @nemerdaud PERFECT!!!! Thank you so much! Will you push this to repository? @R00G3R and @no2chem did you try it with smarthinq-sensors? I saw that wideq's directory files are different and I'm afraid to break HA setup... @R00G3R are you on telegram's Brazilian group? my username is the same as here... |
@viniciuscordeiro, I don't think I have permissions to do that in this repo. But, @no2chem said he will fix it. |
@viniciuscordeiro Traceback (most recent call last): Im new to HA comunity, you can send me the link for telegram group? thank you |
@R00G3R i've tried just replacing files, I could integrate again on HA but didn't show my ACs... Use this link: https://t.me/HomeAssistantbrasil and we can chat there. |
I've made the change suggested by @nemerdaud puffnfresh@87822a5. I then had to make two manual changes in my state file to get the - https://us.m.lgaccount.com
+ https://au.m.lgaccount.com
- https://aic-service.lgthinq.com:46030/v1
+ https://kic-service.lgthinq.com:46030/v1 Seems very similar to the original problem in #55. This makes the example work, but the Home Assistant component doesn't work for me. |
Can confirm the same changes work for me too - haven't tried integration into HA, however straight from Python all appears working (albeit with the workarounds from @nemerdaud & @puffnfresh). |
Hello, is there any progress regarding the wideq for APIv2? I tested the current wideq and not able of listing the devices. |
I've opened a PR ( no2chem#1 ) with some fixes that made it work with my AC and Washer/Dryer |
@marciogranzotto (above) has another fork which has all-but resolved this. marciogranzotto/hass-smartthinq Not sure if we can get it back in here as a PR? It still needs some work (took me some modifications to get it running with my ducted AC), but it does work, and has a nice flow into HA. |
A PR would be awesome if someone can put it together! |
Add fallback to v1 API & Fix some tests
Hi all, sorry, I've merged it and currently have some time to push things through to get everything working... |
It would be awesome if this PR were merged. What is missing for it to be able to be merged? |
After it looked like #100 was in limbo for awhile, I started some work on getting v2 devices working. I only have v2 devices in my home, so it was hard for me to test anything with v1. This PR makes v2 work to make basic queries and control at least the AC unit I have (LA090HSV5), but it also breaks v1 support in the process. I don't think it would be that much work to make v1 and v2 work at the same time, but just haven't had time for it, so I've labeled it WIP. One question is whether v1 devices can run on the v2 api, thus obviating the need to continue supporting the v1 API.
I will disclaim that I'm not really a python expert, so the code is likely not idiomatic python.
The v2 API introduces some considerable changes, I've discovered:
a user number is used in place of a session (the OAuth token remains). The user number is important, because thinq now allows you to control devices that have been shared with you.
while this patch still uses the dashboard view, devices are now organized into homes, and I think you can have multiple homes.
the query APIs have mostly disappeared. the main method of getting data is through snapshots, which you get by polling the dashboard/home. I guess this will require some refactoring of how the
Monitor
works. I sort of hacked around it and just had it pull the entire dashboard, but this seems like it'll result in excessive requests. A caching and timeout mechanism are probably required.it looks like the app has mainly adopted a push notification mechanism via MQTT, so, that should be used instead. To get the MQTT endpoint, which is hosted on amazon, it looks like you have to generate a CSR to send to the API server, which will return a client certificate you're supposed to use to login to the endpoint.
the API errors are definitely different. I have a mapping for those errors and I will add them...
addresses #108