sampsyo / hass-smartthinq

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

Dishwasher support for smartthinq platform. #26

Closed dermotduffy closed 4 years ago

dermotduffy commented 4 years ago

An opening bid at dishwasher support using the recently merged wideq dishwasher functionality.

Uses a similar scheme that @wkd8176 uses in their KR version (and borrows some inspiration and init code -- thanks!) in creating a new 'smartthinq' platform that is independently configured from the existing Smartthinq 'climate' component. I didn't want to mess with climate.py in this PR, but I'd recommend (with your permission) a followup PR to convert climate to use the new toplevel smartthinq platform, and perhaps deprecate independently specifying the token (etc) in the climate component -- as this style is likely more flexible for additional devices/components down the line.

sampsyo commented 4 years ago

Looks pretty good! I see nothing to object to here. And I'd totally be up for a refactor with a top-level SmartThinQ platform configuration if you'd be interested in tackling that.

I can't test this because I don't have an LG dishwasher, but if you're confident in it, we can go ahead and merge!

dermotduffy commented 4 years ago

Excellent news!

Yes, whilst I only have 1 dishwasher to test with, it's definitely working and stable for me. For a visual look at what can be done with this component, take a look at my current UI using this (scroll down in the README).

So if you're happy, lets merge this!

Outstanding work:

Question for you:

Thanks for the continued prompt review!

sampsyo commented 4 years ago

Lovely! I'll merge it now.

Yeah, I'd recommend the soft deprecation version. It seems like it won't be too hard to support the "old way" for a while at least.