dlarrick / hass-kumo

Home Assistant module interfacing with Mitsubishi mini-split units
MIT License
97 stars 21 forks source link

Home Assistant Icon #18

Closed omriasta closed 4 years ago

omriasta commented 4 years ago

FYI, I just added the Kumo Icons to the homeassistant brand folder here: https://github.com/home-assistant/brands/pull/1619

dlarrick commented 4 years ago

Cool, thanks.

I took a quick look at what it would take to use a configuration UI flow instead or in addition to YAML config, but did not make a lot of progress. Kumo should be really straightforward since all we need is a username/password and a flag or two.

omriasta commented 4 years ago

Unfortunately I tried to setup config flow for rachio and failed miserably... Thankfully someone else came along and wrote that one. It should be fairly easy if you know your way around python, it's just a matter of getting the form setup to input user/password and any other entries but as I said I failed doing that

omriasta commented 4 years ago

I reached out to the homeassistant guys and they said: https://github.com/home-assistant/brands/pull/1619#issuecomment-629363030

omriasta commented 4 years ago

I tried to do what he asked and created a pull request...tried to tag you in it but didn't work: https://github.com/home-assistant/core/pull/35672

omriasta commented 4 years ago

They are saying that you need to create the pr yourself since it's your code...sorry, tried to help out

dlarrick commented 4 years ago

We are a long long way from Kumo being accepted into Home Assistant as a native component, which is, if I am reading right, what the PR you opened would do. It's designed in such a way as to theoretically meet their requirements but the documentation is not fully up to date, I am sure there are coding standards issues, etc. I lack the development time right now to jump through all those hoops.

No, what I was referring to earlier is adding UI configuration flow while remaining a HACS custom component for the time being. It's going to take someone who's done this before, or some research on my part, to get that to happen.

omriasta commented 4 years ago

Got it. Actually, I was able to clean up the code according to the errors that Hass development environment was giving me and was able to pass the tests in the dev environment. @bdraco said he would help with the config flow if we had a pr. He has done this before for other components so maybe easier for him... I assume you know better though...

omriasta commented 4 years ago

Finally got home to test this out using the development environment and had to make a few more edits following the errors about deprecation. It all works without error or warnings. You can see all the files with examples here https://github.com/omriasta/core/tree/kumo/homeassistant/components/kumo

dlarrick commented 4 years ago

Cool, I will take a look this weekend.

omriasta commented 4 years ago

Great, I started putting together the form for the config entries in config_flow.py

dlarrick commented 4 years ago

So if I am understanding correctly, there are two types of changes here:

  1. New config_flow.py and related changes needed to enable UI configuration
  2. Stylistic / whitespace changes required to pass some pull-request-time checks

If you could separate those out into separate pull requests against my repo (not HomeAssistant's), that would make it easier for me to review, test, and merge them, while keeping the Git history clean.

omriasta commented 4 years ago

Ok, I'm not exactly a GitHub expert but let me give this a shot

dlarrick commented 4 years ago

You can, after making the relevant branch for the separated change, copy the modified files from the HA repo into your clone of my repo. You're not going to be able to merge between them with Git commands.

omriasta commented 4 years ago

I did the first....not sure if that is how you need it...again, apologize if I misunderstood....not really sure I understand all the github terms....Let me know if I should start the second one?

bdraco commented 4 years ago

Please continue the discussion in the open PR so reviewers know what the state is.

omriasta commented 4 years ago

Stylistic changes merged, working config flow pr submitted