[wip] Adding dryer and washer support

[gvdhoven]

(see descriptions of respective commits for more info)

[sampsyo]

Looks good to me! Instead of duplicating the setup code across the two modules, however, what do you think about factoring it out into a common, shared module that both import?

[gvdhoven]

@sampsyo i totally agree, but this is as far as my Python code goes right now. I was even thinking of adding some sort of ToString on the device class which would give back something like "_ID" as value.

Can you maybe merge this already if it doesn't break anything?

[gvdhoven]

I cleaned up some code:

• removed PLATFORM_SCHEMA since doesnt seem to be used.
• this causes us to be able to remove imports
• made imports in climate.py and dishwasher.py 'generic' and further down 'specific'.

[gvdhoven]

@sampsyo can you review it until now?

[sampsyo]

Hi—the import wideq statements are intentionally embedded in functions instead of being at the module level. This makes it possible to load the component even when the library is not installed, which last I checked was a recommendation when writing HA components?

[sampsyo]

Also, it looks like most of the setup_platform code is still duplicated across the two modules. It would be really great to move this into a utility module so it only has to appear once.

[gvdhoven]

Yeah i see that. If you can give me an example of how you think it should be implemented that would help (given the fact that i don't have the right background for that). I think HA calls the setup_platform methods, right? maybe add some sort of utility method which can be called then in the setup_platform method of each appliance type?

[sampsyo]

Sure! I was thinking we could create a new module (i.e., a .py file) alongside these where the duplicated code would live. There would be a function or two containing the setup code that both modules need. Then, both of them would import that module and invoke those functions.

[gvdhoven]

Working on that right now @sampsyo can you review my latest update which re-adds the imports to the functions?

[sampsyo]

You're on the right track, but please put those after the docstrings. I recommend you take a look at the "diff view" here on GitHub: https://github.com/sampsyo/hass-smartthinq/pull/32/files

Where you can see the changes relative to the master branch. You can modify your code so that unnecessary changes don't appear there—meaning that the code is identical to the current master branch except where there's a meaningful change you actually want to make.

[gvdhoven]

Is my assumption correct that the entity YAML example in the readme; where you say: entity: sensor.lg_dishwasher_[ID]

would mean that it is the 'sensor.py' platform, with named entity as above?

Because i am thinking in the lines of creating a sensor.py with 1 'setup_platform' call which can load ALL supported entities from SmartThinq and will generate something like:

• sensor.lgdishwasher[ID]
• sensor.lgclimate[ID]
• sensor.lgdryer[ID]

Would that be about right? If so; this can be removed from init.py:

SMARTTHINQ_COMPONENTS = [
    'climate',
    'dishwasher',
]

And this can be replaced:

    for component in SMARTTHINQ_COMPONENTS:
        discovery.load_platform(hass, component, DOMAIN, {}, config)

With:

    discovery.load_platform(hass, 'sensor', DOMAIN, {}, config)

Correct?

TIA!

[gvdhoven]

would i even need to call the discovery.load_platform at all? versus just calling add_devices immediately from within the __init__.py script?

[sampsyo]

Hmm; correct me if I'm wrong, but creating everything using the sensor component would be undesirable, right? For example, a climate device (thermostat) shouldn't be misclassified as a sensor.

It is also nice to have separate Python modules for the different kinds of devices—putting everything together into one big sensor.py could make the code harder to read/navigate.

To be honest, I am not an expert on the HomeAssistant API. (And I didn't even write the dishwasher part of this module…) This page about multiple platforms in a single integration may be relevant: https://developers.home-assistant.io/docs/en/creating_component_generic_discovery.html

And the people on the HA Discourse may have better answers than I can to general questions about structuring the code.

[gvdhoven]

@sampsyo finally i have HA running and now are testing the code; during testing i found out that the language should be lowercase (at least for NL) otherwise you would get a tokenError(). Is this something you can recall?

[marthoc]

@sampsyo @Webunity I followed a post on Discord here with an interest in helping; I’ve got a LG fridge and dishwasher myself. I’m currently the code owner of the ecobee integration so I have a fair bit of familiarity with the component/platform architecture of HASS.

To answer the question posed in Discord, you need a setup_platform method for each platform you wish to set up (i.e. sensor, climate). All of the code for those platforms should be contained in the sensor.py or climate.py files, but common code (e.g. the LGDevice base class) can be defined in init.py.

@sampsyo I haven’t looked at wideq too closely, but is there an API endpoint that can be polled to return the status/info of all devices? Would be more efficient to cache all that data every x-seconds and then have each HA entity update by reading from that cache than separately polling by itself.

[sampsyo]

is there an API endpoint that can be polled to return the status/info of all devices?

Sadly, no. In fact, you need to "register" for monitoring updates for each device at a time, and then use the resulting token to poll for data about each individual device. AFAICT, LG does not have an API to poll multiple devices in a single request.

[marthoc]

@sampsyo are you open to me refactoring some of the code? We could also implement a basic config flow to make it easier for users to obtain a refresh token (ie no needing to clone wideq). Sorry to hijack the discussion in this PR.

[sampsyo]

Yes! I have always wanted to build in a token-grabbing workflow to the HA component—if you want to add that, that would be awesome.

And indeed, refactoring would be great too. Feel free to propose whatever.

[gvdhoven]

So, the code is working now without errors again; i will now start restructuring the code so we have only 1 'setup' call.

[gvdhoven]

Ok, looks like the way home assistant is setup (with discovery and load_platform) it doesn't allow me to generate just one setup call. Alternatively, i've cleaned up the code as much as i could and split off the device types inside sensor.py and climate.py to a "DeviceTypes" subfolder.

[gvdhoven]

Getting there...

[gvdhoven]

I am trying to add stuff based on the current wideq version (1.2.0); some stuff will be taken from https://github.com/GuGu927/wideq/blob/gugu_patch/wideq/dryer.py but there he has a wideq version with more features.

[gvdhoven]

Status for today; tomorrow is the test run since my wife has to do the washing :-)

[gvdhoven]

And just now updated also the remaining_time and initial_time fields to show N/A when the dryer is off.

[stboch]

I updated to your repo and tested, It adds the dryer device but I am getting Error doing job: Task exception was never retrieved Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 275, in async_update_ha_state self._async_write_ha_state() File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 309, in _async_write_ha_state state = self.state File "/config/custom_components/smartthinq/DeviceTypes/LGDryerDevice.py", line 222, in state key = self._status.State.name AttributeError: 'DryerStatus' object has no attribute 'State'

[gvdhoven]

Let me check that @stboch

[gvdhoven]

Ok, seems i was basing my changes on some forked wideq libraries which had more functionality. I am back-porting it to my dryer class. I will first test it at home and come back when i'm done.

[gvdhoven]

@stboch i think i've got it now; can you recheck?

[gvdhoven]

btw, @sampsyo reason for my enormous amount of commits was that i had to go via my repo before i could go into my HA install; sorry for that, which basically meant i had to commit a lot of failing code in between... :-(

[stboch]

@Webunity yeah ouch my email was buzzing along all day. But so glad you had time to work on it... I said I was gonna work on it months ago then just sat on my hands soooo...

No more errors on load so that is a good sign. its populating N/A for all the states so far will need to start my dryer when I get some to get some data. But looks like you got it.

[stboch]

Looks good running,

Need to map temperature control labels will try to get you them all...

temperature control @WM_DRY27_TEMP_MID_HIGH_W

[gvdhoven]

Can you try and post your JSON url for your model? (you can get it maybe from the example.py script) Then i can add the missing variables.

On 2019-11-13 05:07, stboch wrote:

Looks good running,

Need to map temperature control labels will try to get you them all...

temperature control @WM_DRY27_TEMP_MID_HIGH_W

-- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].

Links:

[1] https://github.com/sampsyo/hass-smartthinq/pull/32?email_source=notifications&email_token=AABNN3KZEAZQMWZSHAAFNTLQTN4RLA5CNFSM4JDP2RMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED4Z4BI#issuecomment-553229829 [2] https://github.com/notifications/unsubscribe-auth/AABNN3P3RICD4LJH3YQQHOLQTN4RLANCNFSM4JDP2RMA

[stboch]

@Webunity I have the US Steam Dryer https://aic.lgthinq.com:46030/api/webContents/modelJSON?modelName=RV13B6ES_D_US_WIFI&countryCode=WW&contentsId=JS1126002523643728&authKey=thinq

[knackrack615]

Hello @Webunity.

I have an LG Washer (Model: F4J8TS2W) with the following states: https://pastebin.com/WhGaPwfy

Hopefully you can include it on your fork :)

Thanks!

[gvdhoven]

@sampsyo do you have plans on merging this? because after last pull request i now have merge conflicts which i would like to spend some time on resolving and extending with additional devices but i am not going to maintain my fork indefinitely (as in; it currently works for my use-case already)

[sampsyo]

Hi, @Webunity—I am certainly quite interested in merging support for washers & dryers in the UI. But I am concerned that this branch has now changed quite a bit of the rest of the component and has reorganized the top-level structure of the code, which makes it pretty hard to review independently.

Is there any chance you could summarize the changes that you see as necessary in the overall structure of the component in order to get to W&D support? Then perhaps I can get started on the work to integrate those changes, at which point we can get to a "clean" PR that just adds the new functionality on top of that.

[gvdhoven]

To be honest that is quite difficult to do; i did not change anything in the visual representation of the AC for example; Maybe here is an idea; can you create a branch from master, call it WasherAndDryer or something then i'll fix the merge conflicts and create a pull request on that branch. Then people can review those changes individually without the need to take in master with some risks?

On 2019-11-28 14:46, Adrian Sampson wrote:

Hi, @Webunity [1]—I am certainly quite interested in merging support for washers & dryers in the UI. But I am concerned that this branch has now changed quite a bit of the rest of the component and has reorganized the top-level structure of the code, which makes it pretty hard to review independently.

Is there any chance you could summarize the changes that you see as necessary in the overall structure of the component in order to get to W&D support? Then perhaps I can get started on the work to integrate those changes, at which point we can get to a "clean" PR that just adds the new functionality on top of that.

-- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub [2], or unsubscribe [3].

Links:

[1] https://github.com/Webunity [2] https://github.com/sampsyo/hass-smartthinq/pull/32?email_source=notifications&email_token=AABNN3K3UMVHIPVT4U4GQ63QV7DUHA5CNFSM4JDP2RMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFMU2TQ#issuecomment-559500622 [3] https://github.com/notifications/unsubscribe-auth/AABNN3KEARULYOKBJIVNLGTQV7DUHANCNFSM4JDP2RMA

[sampsyo]

Sure. While I understand that this might be more difficult than it sounds, here's how I would do it in my wildest dreams:

1. You open a new PR with just the reorganization of the existing code that you want to do, without adding the new functionality. We iterate on that to get it into a state where I can understand everything that's changed. For example, I don't fully understand what's going on with the new DeviceTypes Python package, and I would like to grok that. And the new LGDevice base class functionality seems really helpful, so I'd like to make sure I understand the separation of concerns there.
2. You open a second new PR with just the W&D functionality built on that above reorganization. Hopefully this is a small, simple PR that just adds one new file into the new organization.

If separating that out seems too hard, I will instead try to get to work understanding exactly what needs to be done in the reorganization so I can facilitate Step 2 alone.

In the mean time, I don't think it matters too much whether the code lives in a branch here or in your fork.

[ItsRhen]

...I would greatly appreciate this support... This is the last set of devices to add to HA for me to have everything linked up! Very much appreciate the effort here.

[towerhand]

Any updates? Would be great to have support for the washers and dryers.

[JeedHome44]

Hello, if you are looking for testers to add the washing machine to home assistant, I have one. tell me what to do to test. Sorry I don't know how to program otherwise I will have helped you. thank you for your work

[gvdhoven]

Hey guys, sorry for not updating you; but it currently works for me in my own environment. Since then people have been adding stuff so i either have to start all over bit by bit as @sampsyo suggested or abandon this pull request. Either way, i might be able to take a crack at it but it won't be soon (to busy atm)

[JeedHome44]

Hi, You say it’s work. But I don’t see my washer whe I add the GitHub in my Hassio. How can I search new device in Hassio with this GitHub? I’m a beginner in Hassio. Thank for your help.

[stboch]

Hi, You say it’s work. But I don’t see my washer whe I add the GitHub in my Hassio. How can I search new device in Hassio with this GitHub? I’m a beginner in Hassio. Thank for your help.

You need to pull down from webunity's repo and put that into custom_components and also follow the directions in the readme to get your token.

[JeedHome44]

Hi, I find my token. That is ok. But after, what I have to do for add my washer in Hassio?

[bakazm]

Hi, I find my token. That is ok. But after, what I have to do for add my washer in Hassio?

I browsed through the code and i only see items pertaining to dishwashers, HVAC, and dryers, not clothes washers.

[paoloantinori]

Hi, I'm also interested in getting this functionality having a Dryer, a Washing Machine and a Fridge.

I've checked out the PR, fixed a little bug with a wrong variable that was preventing dryers to get picked up and I can report that it works ok!

I'm netiher a python nor a HA expert but I'd be interested in helping with this and I'm planning to provide a PR soon.

[towerhand]

Have you guys looked into this new repo? Already have authentication via UI, set up in the HA integrations and only support for washers so far but looks promising. https://github.com/ollo69/ha-smartthinq-washer

[sampsyo]

It's sorta troubling that it copies the wideq code into the repository instead of depending on the PyPI release? And it's also disappointing that it does so without attribution or incorporating the license of the original code. 😞

I guess we can hope that they'd contribute the authentication flow back to this repository.

[cwttdb70]

Any update on this pull request? Just ordered LG ThinQ (what a dumb name) washer & dryer and would be great to integrate into my HA system.