search

dlarrick / pykumo

Python library to interact with Mitsubishi KumoCloud devices via their local API
MIT License
33 stars 12 forks source link

Interactive usage improvements to KumoCloudAccount #11

Closed eteq closed 3 years ago

eteq commented 3 years ago

This PR adds a few conveniences I found helpful when trying to work with KumoCloudAccount. Specifically:

  1. If the user doesn't give a name and password it prompts for one using the input and getpass mechanisms
  2. Adds a make_pykumos method that creates PyKumo objects for everything in the kumo cloud account, skipping over some mildly awkward creation steps.
dlarrick commented 3 years ago

Thanks.

I'm a little concerned that the primary use for the pykumo lib (at least for me) is non-interactive (i.e. from Home Assistant) so having it try to prompt for username/password in a situation where they're not configured could be problematic. If you could reorganize your code so your component that's creating the KumoAccount object does the prompting instead, I think that would be better overall.

Also, adding 're' and 'getpass' imports probably means there are enough external dependencies to warrant a requirements.txt, so if you could manage that in your PR I'd appreciate it.

eteq commented 3 years ago

I'm a little concerned that the primary use for the pykumo lib (at least for me) is non-interactive (i.e. from Home Assistant) so having it try to prompt for username/password in a situation where they're not configured could be problematic. If you could reorganize your code so your component that's creating the KumoAccount object does the prompting instead, I think that would be better overall.

I see your point here, but I think it would be ideal if we could make pykumo to meet both of these needs. The point here is that I don't have a component to do this in - I am directly, at an interactive ipython prompt, interacting with these objects. (For me it's quite a bit faster to just switch to the python terminal on my desktop and call set_mode('cool') than pull out my phone and fiddle with the app, or pull it up in Home Assistant!) So I see two "compromise" solutions here:

  1. I can change it so that KumoCloudAccount.__init__ doesn't default to None for username and password. Then I have to do KumoCloudAccount(None, None), but that's not so bad (and less likely for me to accidentally reveal my password on the ipython history). And then that way there's little chance of "accidentally" triggering the non-interactive mode in a hass or similar context.
  2. Create a subclass that's something like InteractiveKumoCloud and add the changes I'm suggesting there.

Does one of those sound good to you @dlarrick ?

Also, adding 're' and 'getpass' imports probably means there are enough external dependencies to warrant a requirements.txt, so if you could manage that in your PR I'd appreciate it.

Those are both in the python standard library, so I don't think a requirements would apply there...?

eteq commented 3 years ago

Oh, but I'm happy to add a requirements.txt or add a dependency to setup.py which I think is just requests, right?

dlarrick commented 3 years ago

I think a subclass for InteractiveKumoCloud sounds fine, thanks!

Lack of 'requests' in a requirements / setup hasn't mattered till now since (naturally) HA itself needs requests. If you're willing to do something to fix this in your PR I'll be grateful, but if not I'll fix it later.

dlarrick commented 3 years ago

@eteq I have just now pushed something akin to this PR. Instead of a wrapper class I added a static method Factory that will allow you to do what you want. I'll push to PyPI shortly. I also included your make_pykumos utility.