search

dlarrick / hass-kumo

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

New IP address missing detection and prompt #57

Closed danielgoepp closed 3 years ago

danielgoepp commented 3 years ago

Omriasta added the ability to detect missing IP addresses from the kumo cloud service, and prompt the user to enter their addresses. Also you can now edit them in the config after install.

omriasta commented 3 years ago

@dlarrick I commented in the issue but just an FYI, I changed the code for prefer_cache so it is forced to True if the cache file is missing IP addresses. If someone setup the integration with this set as false, it would work only until home assistant was restarted at which point the cache would be loaded direct from kumo and wouldn't have ips again. Not sure if that is ok but maybe we should add that note to the form that shows up if ips are missing. Also, if you have a better idea of how we can set labels dynamically in that form so that it looks better rather than populating the unit names and Mac as the default entry, I'll be glad to take another go at it.

omriasta commented 3 years ago

I guess I missed the "grandchildren" since my setup is so simple. I will try to look over these and correct but will be pretty busy this week and then "off the grid" myself from Thursday. Maybe @danielgoepp can try and correct these?

dlarrick commented 3 years ago

I guess I missed the "grandchildren" since my setup is so simple. I will try to look over these and correct but will be pretty busy this week and then "off the grid" myself from Thursday. Maybe @danielgoepp can try and correct these?

Oh no worries, this is easy to miss ... I missed it myself in the original implementation, as did the author of kumojs in their first node.js library. If Daniel doesn't get to it I might have some time myself late in the week. (though why does "a week off at home" turn into "here are 30 things to get done since you're not working this week"?)

Ideally all the parsing & editing of the raw JSON could move to pykumo so the integration can be insulated from those details. Something like "get_raw_indoor_units" that gets a dict of units (keyed by MAC or serial) from the raw JSON, and "override_indoor_unit_info" that accepts that same dict (with the user's input) and makes those changes in the raw JSON.

danielgoepp commented 3 years ago

Thanks for all the updates. I'm unlikely to get to this during the week, but might be able to check it out this weekend. Also just an FYI I have a ticket open with Mitsubishi trying to get to the root cause of why the IP are not saving in the first place. It's been a little painful so far, as you might imagine ;) However, I have not given up hope yet.

omriasta commented 3 years ago

I can't figure out how to "get in" on this pull request and update it but I have made the changes in my fork with this commit: https://github.com/omriasta/hass-kumo/commit/71b6b36c8f94464a027e02a8b3f79398b950007a

The only thing I wasn't sure about was "...and here, and in the next loop. It may be worth a pair of utility functions to get / set the IPs for all the units" . I am not sure what you meant by this?

danielgoepp commented 3 years ago

@omriasta I have invited you to my repository which I forked from @dlarrick. If you prefer to update yours, if you want to give me access I'm happy to do the formatting updates directly there too. I don't think we need to do this bouncing back and fourth though ;)

omriasta commented 3 years ago

@danielgoepp I merged your changes to my fork but to keep things in one place I'll take a look at joining your fork tomorrow.

dlarrick commented 3 years ago

If either of you wants write access to create branches directly on this repo let me know. I just ask that you not make changes directly to master (i.e. make a branch and make a PR from that branch). OTOH PRs from a fork are almost as easy, y'all just gotta settle on one fork.

danielgoepp commented 3 years ago

I think that would be easier, if you are okay with it. I know I certainly would never touch master directly, always branch. I'm a fan of the git-flow method but don't necessarily need to add that complexity here. At a minimum I think just adding a develop branch and working off feature branches from there.

omriasta commented 3 years ago

OK, I think I did it.....there is a new PR from a branch I created. Hopefully I didn't mess anything up....

danielgoepp commented 3 years ago

Looks about right to me. I'll close this pull request then to avoid confusion.