sampsyo / hass-smartthinq

Home Assistant component for LG SmartThinQ HVAC devices
MIT License
284 stars 98 forks source link

[WIP] use v2 names, use state file #91

Open no2chem opened 3 years ago

no2chem commented 3 years ago

This PR includes the set of changes necessary to use with with my wideq branch, opened as sampsyo/wideq#132

In particular, the following changes need to be addressed:

This is resolved by using the json state file instead of manually transferring just the token to the Home Assistant configuration.yaml file. In theory, we could support both manually inserting the entries, but referring to the JSON file seems to be more robust and user friendly.

Instead of using these values, the wideq api should provide an api independent method to get the supported operation modes and wind directions

this is marked as a wip because dishwasher support is currently disabled. Hopefully, I can get that working next.

If you would like v2 support, you can try using this branch, and installing the wideq branch here: https://github.com/no2chem/wideq using pip install . in your home-assistant environment.

addresses #87

sampsyo commented 3 years ago

Hi there! Thanks for getting this started! I'm catching up on the PRs here, and although I know this is still WIP, I wanted to make a couple of notes:

Thanks again for spearheading the effort!

jacekpaszkowski commented 3 years ago

Hello @no2chem,

i've tried to use your's PR branch and linked wideq version. No errors in logs, but there are no devices in HA. I have 3 air conditioners.

Entries from my HA logs:

2020-12-08 22:44:03 WARNING (MainThread) [homeassistant.loader] You are using a custom integration for smartthinq which has not been tested by Home Assistant. This component might cause stability problems, be sure to disable it if you experience issues with Home Assistant.

2020-12-08 22:44:13 INFO (MainThread) [homeassistant.setup] Setting up smartthinq

2020-12-08 22:44:21 INFO (MainThread) [homeassistant.setup] Setup of domain smartthinq took 8.1 seconds

2020-12-08 22:44:31 INFO (MainThread) [homeassistant.components.climate] Setting up climate.smartthinq

2020-12-08 22:44:42 INFO (MainThread) [homeassistant.components.sensor] Setting up sensor.smartthinq

What can i do to investigate this?

Using wideq in command line i'm able to generete wideq_state.json file. But output of ls command is empty.

Using wideq from https://github.com/gladhorn/wideq.git i'm able to list devices using ls command.

no2chem commented 3 years ago

@jacekpaszkowski interesting. Are your devices v2? I suspect the issue is due to a TLS issue. Are you located in the US or another region?

jacekpaszkowski commented 3 years ago

@no2chem i'm quite sure my devices are v2. I have my devices for about 4 months. I'm located in Poland (PL).

I've tested different versions of wideq. Take a look at outputs for my account

https://github.com/sampsyo/wideq image

https://github.com/no2chem/wideq image

https://github.com/gladhorn/wideq image

mjdegraaff commented 3 years ago

Any news on this pull request. Would be happy to help but I'm no programmer whatsoever...

beped commented 3 years ago

I would love to help. I have a air conditioner v2 here. If there's any test i can do to test, just say so.

XalaTheShepard commented 3 years ago

I am useless in coding, but sure want to contribute in whichever way possible. I can sure help testing the changes. It is however only possible to test the APIv2 as we do not have APIv1 devices.

Codex- commented 3 years ago

I'm able to contribute to the code but realistically we need input from the repository owner before proceeding at this point

sampsyo commented 3 years ago

We'd love any help we can get!! Carefully figuring out what remains to be done based on the thread would be a great way to start…

Codex- commented 3 years ago

I think a good first step to get this back on track would be to add tests for the existing implementation, since I imagine you'd expect both v1 and v2 support to persist?

sampsyo commented 3 years ago

That's actually the big question: nobody seems to be 100% sure whether we need both v1 and v2 support, or whether we can transition entirely to v2 without dropping support for existing devices. So I think the first thing would be to nail that down—things would certainly be way simpler if we could just support v2.

Vybo commented 3 years ago

I was searching for a solution for a few months now and found few alternate forks. I was able to make my v2 devices work with one from marciogranzotto that supports only v2, not v1 devices and it's explicitly state as usable only for v2 things. It's a fork originating from this PR, so I think v1 devices won't work if you support only v2 API. I could be wrong though, so that fork could be in theory used to test it out. (https://github.com/marciogranzotto/hass-smartthinq)

Codex- commented 3 years ago

to me, it sounds like people will be stuck with v1 still, so we should adopt a strategy that allows us to support both.

I think starting with some basic testing on the current implementation would go a long way, it sounds painful but this is the way

Once we have comprehensive or at least basic testing over v1, we can safely adopt v2 and pave the way for good support for v3 (if it ever gets done)