-
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
WideQ from ha-smartthinq-sensor component #100
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for getting this started!
The way I would love to start this off would be by changing out the core
layer to use the v2 API instead of the v1 API, and probably changing the client
layer to do the same. So maybe a good way to move forward with that would be to change your v2 implementation to overwrite the old v1 implementation, and to use names as if it were going to be the only implementation. That is, no need to put 2
s everywhere—just use the same name. Then we can go through the changes and try to find the minimum set of changes that are actually required to support the new API—and not focus on the parts of the code that haven't had to change.
wideq/core.py
Outdated
|
||
|
||
def oauth2_signature(message: str, secret: str) -> bytes: | ||
def oauth2_signature(message, secret): |
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's an example of a change we can just revert back to the current version (since it seems to be newer).
wideq/core.py
Outdated
secret_bytes = secret.encode('utf8') | ||
hashed = hmac.new(secret_bytes, message.encode('utf8'), hashlib.sha1) | ||
secret_bytes = secret.encode("utf8") | ||
hashed = hmac.new(secret_bytes, message.encode("utf8"), hashlib.sha1) |
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's an example of a change we can omit because it's just cosmetic (and it would be great to focus on the substantive changes).
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.
This was changed by black formatting. If it's really a problem I can search all this change and replace, prefer not have to do :)
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 quotes themselves are not really the problem, but the "diff noise" is because it makes it hard to review the changes. But no worries; I can do the find-and-replace.
|
||
class Tlsv1HttpAdapter(HTTPAdapter): |
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.
Is this necessary? I know some people get TLS errors when they use the wrong country server, but reverting to TLSv1 for all connections is a security concern. (Maybe we can leave this out for now and focus on other stuff first?)
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.
This is more related to some country server that need this.
Probably some check should be done during connection to choose the right TLS method, otherwise for some user/region do not work
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.
Great; that seems like a good idea. For this first iteration, let's leave stuff like this out and just focus on the minimal, core changes to support version 2 of the LG API.
wideq/core.py
Outdated
@@ -188,49 +75,84 @@ def lgedm_post(url, data=None, access_token=None, session_id=None): | |||
authenticated requests. They are not required, for example, to load | |||
the gateway server data or to start a session. | |||
""" | |||
|
|||
_LOGGER.debug("lgedm_post before: %s", url) |
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.
Debugging stuff like this can probably be left 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.
I see that you implement a different type of logging, so maybe the log have to be reviewed?
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.
Sure, sounds good—let's leave that for a future pull request.
wideq/core_v2.py
Outdated
): | ||
"""Make an HTTP request in the format used by the API2 servers.""" | ||
|
||
# this code to avoid ssl error 'dh key too small' |
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.
Is this hack necessary for all connections, or is it just in some circumstances that we need this?
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 some circumstances, but as before the problem is to really understand when should be used
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.
Alright, sounds good—but as with other tangential features discussed above, let's move carefully and just do one thing at a time, i.e., leave additional features like this to future pull requests so we can discuss them individually.
So I removed CoreV1 and renamed CoreV2 to Core. |
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.
OK, great! Thanks for getting that started. As I mentioned above, I think it would be great to focus on one thing at a time, and the first thing to focus on is the core API changes. To that end, let's try to make this PR only contain changes to the low-level core API stuff and the higher-level "client" layer. Specifically, at a very high level of abstraction, let's do this:
- Roll back the changes to
__init__.py
. - Restore
util.py
. - Move the client stuff back to
client.py
fromcore.py
anddevice.py
. - Move the exceptions back from
core_exceptions.py
tocore.py
. - Roll back the changes to all the device-specific files:
dishwasher.py
,dryer.py
,refrigerator.py
, andwasher.py
(and remove*_states.py
). We'll address these changes in future PRs.
I'd be happy to help with these (I think GitHub lets me push to your branch because the PR is open) if that would be useful.
Ok, I limited changes as you suggest. |
OK, got it, cool. Yeah, maybe we should plan on merging this into a branch and then try to fix the monitoring stuff next—then everything can go into master, because presumably we'll have full parity with the old version? |
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 a list of comments on differences to clean up in core
(I haven't worked on client
yet). Next I'll try taking care of some of the diff noise.
wideq/core.py
Outdated
@@ -137,10 +152,16 @@ def __init__(self, code, message): | |||
class NotLoggedInError(APIError): | |||
"""The session is not valid or expired.""" | |||
|
|||
def __init__(self): |
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 think we should not add these empty-argument constructors—it's actually good to keep around the error code and message!
wideq/core.py
Outdated
@@ -169,68 +196,146 @@ def __init__(self, device_id, code): | |||
self.code = code | |||
|
|||
|
|||
API_ERRORS = { | |||
API2_ERRORS = { |
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.
Change this name back.
wideq/core.py
Outdated
"0102": NotLoggedInError, | ||
"0106": NotConnectedError, | ||
"0100": FailedRequestError, | ||
9000: InvalidRequestError, # Surprisingly, an integer (not a string). |
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.
Restore these errors.
"x-thinq-app-level": V2_APP_LEVEL, | ||
"x-thinq-app-os": V2_APP_OS, | ||
"x-thinq-app-type": V2_APP_TYPE, | ||
"x-thinq-app-ver": V2_APP_VER, |
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.
Are all these headers really necessary? I think we should try pruning them down and see if the requests still work. (When I was original reverse-engineering the first version of the API, I found that the client included lots of headers that were not actually necessary to make the request work.)
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.
May be not, not sure, require some test, may be after merging the code?
): | ||
"""Make an HTTP request in the format used by the API2 servers.""" | ||
|
||
# this code to avoid ssl error 'dh key too small' |
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.
Let's remove this special SSL stuff.
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 cannot remove, without this doesn't work with my country!!!
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.
Can also confirm that POST requests in the US don't work without forcing TLSv1. There may be something else going on, though, because I POST successfully but get back error code 0009.
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.
Interestingly, the error text accompanying code 0009 translates to "Device does not exist"
@ollo69, does lgedm2_post work for you?
self.gateway.country, self.gateway.language) | ||
session_id = session_info['jsessionId'] | ||
return Session(self, session_id), get_list(session_info, 'item') | ||
return Session(self) |
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.
Hmm… in thinq2, is there no such thing as a "session"? Maybe we should add this to our list of things to clean up post-merge then… if there's no need to initiate sessions, there's no need for the Session
class.
headers["x-emp-token"] = access_token | ||
|
||
if user_number: | ||
headers["x-user-no"] = user_number |
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.
Is the user number really necessary to make the API work? A lot of things could be simplified if it could be omitted, and it seems strange that the API would need it in addition to the token.
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.
Yes it is, I'm sure about this
|
||
class Session(object): | ||
def __init__(self, auth, session_id) -> None: | ||
def __init__(self, auth, session_id=None): |
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.
Is session_id
ever non-None
in thinq2? If not, let's remove this parameter.
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 have to better check this
self.auth = auth | ||
self.session_id = session_id | ||
|
||
def post(self, path, data=None): | ||
"""Make a POST request to the API server. | ||
"""Make a POST request to the APIv1 server. |
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.
Is this really for thinq1? If so, let's delete it.
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.
As I said before, this is used, depend on the device
wideq/core.py
Outdated
|
||
return get_list(self.post('device/deviceList'), 'item') | ||
devices = self.get2("service/application/dashboard").get("item", []) | ||
return as_list(devices) |
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.
Any particular reason to use as_list
here instead of the older get_list
, which makes things easier to read?
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.
Cleanup done; here are a few scattered comments on client as a bonus.
wideq/client.py
Outdated
|
||
return client | ||
|
||
def dump(self) -> Dict[str, Any]: | ||
"""Serialize the client state.""" | ||
|
||
out: Dict[str, Any] = { |
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.
Restore this type annotation.
wideq/client.py
Outdated
} | ||
|
||
if self._gateway: | ||
out['gateway'] = self._gateway.serialize() | ||
out["gateway"] = { |
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.
Any particular reason why this no longer uses the serialize method?
wideq/client.py
Outdated
|
||
if self._auth: | ||
out['auth'] = self._auth.serialize() |
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.
Ditto.
wideq/client.py
Outdated
|
||
return out | ||
|
||
def refresh(self) -> None: | ||
self._auth = self.auth.refresh() | ||
self._session, self._devices = self.auth.start_session() | ||
self._session = self.auth.start_session() | ||
# self._device = None |
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.
Delete.
Ok, sorry but on this days I was busy. |
There are no annotations for it, see urllib3/urllib3#867 This should make the mypy check happy.
f8f8784
to
e618a87
Compare
wideq/client.py
Outdated
|
||
# The last list of devices we got from the server. This is the | ||
# raw JSON list data describing the devices. | ||
self._devices: List[Dict[str, Any]] = [] | ||
self._devices = None |
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.
This change leads to the whole script crashing in case there is no device. Since we later assume _devices to be an interable.
File "./example.py", line 33, in ls
for device in client.devices:
File "/home/frederik/dev/projects/home_automation/wideq/wideq/client.py", line 151, in devices
return (DeviceInfo(d) for d in self._devices)
TypeError: 'NoneType' object is not iterable
} | ||
|
||
@classmethod | ||
def load(cls, gateway, data): |
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.
For me this always fails, when the new wideq_state.json is written, it only contains "model_info", "gateway" and "auth" as keys.
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.
it looks like Auth.serialize() in core.py needs to be redirected to Auth.dump(). One of them should probably be deleted since they're duplicates.
|
||
def serialize(self) -> Dict[str, str]: | ||
return { | ||
'access_token': self.access_token, | ||
'refresh_token': self.refresh_token, | ||
} | ||
|
||
def dump(self): |
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.
As noted below, this function needs to be de-duped with serialize() just above.
LOGGER.debug("Injected debug device: %s", d) | ||
|
||
def _load_devices(self, force_update: bool = False): | ||
if self._session and (self._devices is None or force_update): |
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.
This doesn't work as-is for two reasons:
self._session
is itself initialized lazily, so if that hasn't happened as a side effect of something else, we bail out. I suspect you really just wantself.session
instead to force the initialization to happen.- self._devices is initialized to
[]
, which isn'tNone
. I suspect you probably want to checknot self._devices
instead?
So I'm prodding at this branch to try and get my portable AC working, and it looks like the model information has also changed? ... so I'm hacking away at ACDevice trying to get
|
id really like to see V2 implemented, Ive tried to get this working but its a bit beyond what i can do in python, seems like https://github.com/tinkerborg/thinq2-python/tree/dump-device can pull info tho and is semi working so its useful to get information, homebridge for homekit - https://github.com/sman591/homebridge-lg-thinq-ac works too for info and that has commands working, the data i was able to gather on my specific AC @ThatWileyGuy we have the same unit just diff regions mines WW i believe
|
@ThatWileyGuy can you fork and push your modifications, ill take a crack at it with you :) |
@ThatWileyGuy, @SkOODaT. I took a crack at this a couple weeks ago and was able to control my apiv2 a/c with the thinq2-python library, so however that person has it implemented works. The only part missing is they haven't implemented the control/control-sync API endpoint, so I added that in. I'm not sure of the difference between the two, they both seem to work to control stuff. I also looked at the raw API requests coming from the app and it looks like there is a step to register the individual client-id that the v1 api does not require. I checked with the thinq2-api library and verified that this step does need to happen or else the control-sync/control endpoints won't work, so at minimum, we'd need to implement:
I started looking at the wideq codebase to implement v2 and it seems like APIv2 does things just differently enought at a basic level to make it challenging to implement v2 in a way that doesn't break v1 compatibility. Currently the way devices are implemented in wideq makes a few assumptions about how the api works that would have to be changed to work with v2, e.g.
If the goal is to maintain compatibilty with the v1 api, then we might need to create an intermediate abstraction with different implementations for apiv1 and apiv2. Reiterating what @ThatWileyGuy said, it would be helpful to have the device info for v1 devices posted somewhere to make sure the new implementation will still work with older devices since I'm sure many v2 device owners (myself included) only have access to v2 devices. |
I currently have a wall-mounted LG heat pump head (ac unit) with built-in WiFi, running on the v2 api, and a ceiling cassette with the old USB adapter running the v1 api. Both are registered and available in LG's app. With the current WideQ implementation I only see the ceiling cassette. Let me know if there's something I can do help get the device info you need.
|
Are you looking for the
|
lol thank you @ljtilley i had it partially added but couldn't figure it fully out, took about 2 sec and i was able to power on my AC
True edit: ill continue to add to this as i figure functions out for my |
Just seeing this - I've got an LG window air conditioner that requires API V2 that I'd love to get connected to HA and some spare time and coding chops to throw your way. Where's the best place for me to get started? What's needed at this point? |
Great! The main thing we need is some serious hacking attention to bring together the heroic effort folks have already made to figure out the API and bring it in line with the whole library. Cloning the branch from this PR, trying it out, and fixing bugs would be a great way to start. |
@sampsyo - Can do. Is there any particular place the community has collected what they've worked out about the V2 protocol beyond the comments on this PR and the code comments on the branch? |
Rad! Alas, no—there is no real central place where this stuff is being collected. However, @tinkerborg does have another effort that I believe inspired @ollo69 to do this set of changes, which might be helpful: https://github.com/tinkerborg/thinq2-python |
Got it. I checked that out last night and got it up and running and watching events from my air conditioner - so progress! @tinkerborg - I notice you haven't done anything with thinq2-python since May - are you planning to do more or is that going by the wayside? |
Still planning to work on it, but haven't been able to lately for various reasons. I hope to finish it off soon. |
Anything I can do to help w/ adding support for LG dehumidifiers? |
quiet lol, has anyone noticed this ....
|
Hi @sampsyo - any way I can help to bring this back to life - I have a V2 LG system in Australia - with a bit but not heaps of python experience - super keen to get my AC running through HA though. |
I am about to pull the trigger on purchasing an LG ducted system for my house.. its an older model (but new) so might use APIv1? But I assume my newly created LG account might be APIv2 only? Is that the current issue? |
I can confirm using their fork that I can see the machine I couldn't see with the current stable of this repo. @pneumaticdeath would you be willing to make a PR here with your changes to get v2 working? Would be a good start, happy to help and review too. EDIT: Looks like @no2chem has this working too over in their fork: https://github.com/no2chem/wideq/tree/thinq2 Would be great to see a PR to get the ball rolling again |
I'll get a PR ready.
…On Fri, Jan 1, 2021 at 18:56 Alex Miller ***@***.***> wrote:
quiet lol, has anyone noticed this ....
https://github.com/pneumaticdeath/wideq/commits/api2_rebased
looks liek they have it working, i get a proper state file with all the
data, allthough i couldnt get any functions working yet, thier errors ~
I can confirm using their fork that I can see the machine I couldn't see
with the current stable of this repo.
@pneumaticdeath <https://github.com/pneumaticdeath> would you be willing
to make a PR here with your changes to get v2 working? Would be a good
start, happy to help and review too.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#100 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAELAYAHCSJFHIR5OVQRSN3SX2DM7ANCNFSM4NNNHXLA>
.
|
Hi, for folks that have been talking about API v2 support, I did get most of the v2 API implemented, including auth, retrieving device data, and state updates via MQTT (along with generic MQTT connectivity). The events of 2020 have left me particularly drained so I haven't found the motivation to finish it yet, but the last piece really needed is device control. The work in progress can be found here: https://github.com/tinkerborg/thinq2-python - whether you want to use it as a reference or just implement the device control part and send a PR. |
its winter here so thats why i havent had motivation to continue, the AC is away-ish lol that being said its still plugged in and connected so id be willing to test anything you guys push lol, be nice if we could get it all sorted by this summer 😜 with some of the stuff around i was able to control and do a few various things with mine a few months ago i think i posted it all above ~ also on side note as far as i can tell nothing has changed with the api, im still running a node version and nothing has been changed with it in months ~ |
I have this PR open: gladhorn#1
Is that against the wrong fork?
…-- PneumaticDeath (Mitch Patenaude)
On Sat, Jan 2, 2021 at 11:24 AM SkOODaT ***@***.***> wrote:
its winter here so thats why i havent had motivation to continue, the AC
is away-ish lol that being said its still plugged in and connected so id be
willing to test anything you guys push lol, be nice if we could get it all
sorted by this summer 😜
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#100 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAELAYEIUHDFTINPCXFSBHTSX5XGXANCNFSM4NNNHXLA>
.
|
As a new LG product owner, how can I tell if my appliance is api v2? |
Did you see this version they are working on: ollo69/ha-smartthinq-sensors#124 going to branche https://github.com/ollo69/ha-smartthinq-sensors/tree/AC-Implementation Maybe a good idea to join effort? |
i have problem on hoobs |
This seems abandoned.. have people found other solutions to operate their thinq api2 devices, or? |
I ma just using this: https://github.com/ollo69/ha-smartthinq-sensors |
Been using https://github.com/ollo69/ha-smartthinq-sensors for a year or 2 already, without any issues. Edit: sorry for double post, was just beaten to it by mere seconds 😉 |
Same use this as it works flawlessly https://github.com/ollo69/ha-smartthinq-sensorsOn 21 Dec 2023, at 14:08, Leon Vliegenthart ***@***.***> wrote:
Been using https://github.com/ollo69/ha-smartthinq-sensors for a year or 2 already, without any issues.
Good luck 👍
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
So let's start comparing the code with this PR