Skip to content

Add switches (on/off zones) to geniushub#28182

Merged
zxdavb merged 22 commits into
home-assistant:devfrom
castaway:feature/geniushub_switches
Nov 4, 2019
Merged

Add switches (on/off zones) to geniushub#28182
zxdavb merged 22 commits into
home-assistant:devfrom
castaway:feature/geniushub_switches

Conversation

@castaway
Copy link
Copy Markdown
Contributor

@castaway castaway commented Oct 24, 2019

Description:

The geniushub integration shows smart plug Devices as BinarySensors; it can report when they are on or off, but can not change their state (this is done in GH via the parent Zone).

This PR adds on/off Zones as SwitchDevices:

  • 'turn_on' - set Zone to GH's Override/On mode
  • 'turn_off' - set Zone to GH's Timer mode

Also bumps geniushub-client from 0.6.28 to 0.6.29, see: manzanotti/geniushub-client@v0.6.28...v0.630, which fixes:

Pull request with documentation for home-assistant.io: home-assistant/home-assistant.io#11095

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass - no tests for geniushub
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest. - nothing changed
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all. - no new ones
  • Untested files have been added to .coveragerc.

@castaway castaway requested a review from zxdavb as a code owner October 24, 2019 19:03
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @castaway,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@probot-home-assistant
Copy link
Copy Markdown

Hey there @zxdavb, mind taking a look at this pull request as its been labeled with a integration (geniushub) you are listed as a codeowner for? Thanks!

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 25, 2019

Thanks for your PR - we'll definitely get this useful PR merged.

Copy link
Copy Markdown
Member

@zxdavb zxdavb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add:

    @property
    def device_class(self):
        """Return the class of this device, from component DEVICE_CLASSES."""
        return DEVICE_CLASS_OUTLET

Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py Outdated
Comment thread homeassistant/components/geniushub/switch.py
@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 25, 2019

The geniushub integration shows outlets as binary sensors, it can report when they are on or off, but does not allow the user to change the status. This PR adds them as SwitchDevices ...

Please be aware of the distinction between a GH Device (that is a binary_sensor)), and a GH on/off Zone (a switch).

For example, an on/off Zone can have (say) three Devices (smart plugs). All 3 will turn on when the Zone is on, and all three off otherwise.

@castaway
Copy link
Copy Markdown
Contributor Author

The geniushub integration shows outlets as binary sensors, it can report when they are on or off, but does not allow the user to change the status. This PR adds them as SwitchDevices ...

Please be aware of the distinction between a GH Device (that is a binary_sensor)), and a GH on/off Zone (a switch).

For example, an on/off Zone can have (say) three Devices (smart plugs). All 3 will turn on when the Zone is on, and all three off otherwise.

I am aware of these however, (see above), I can only see ways to turn on/off whole zones, not individual devices.. At least in the v1 API.

@castaway
Copy link
Copy Markdown
Contributor Author

Please add:

    @property
    def device_class(self):
        """Return the class of this device, from component DEVICE_CLASSES."""
        return DEVICE_CLASS_OUTLET

Aha ok, I took the switch parts from the smartthings/switch.py, which doesn't contain this..

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 25, 2019

I am aware of these however, (see above), I can only see ways to turn on/off whole zones, not individual devices.. At least in the v1 API.

Correct.

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 25, 2019

The problem is, and this is why I haven't been rushing to implement this functionality, that HA switches have no concept of operating mode / profile. That is, they expect to be turned off, and turned on - they have no concept of following a schedule (Timer mode)...

So have a think about what you expect this entity to do when it is turned on, and turned off.

@castaway
Copy link
Copy Markdown
Contributor Author

Latest push: 857dce0 - should contain all of the suggestions, apart from the discussion on "!= "off" .

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 27, 2019

FYI: just identified an issue with the geniushub-client, where is can't change the override of an on/off zone.

@castaway
Copy link
Copy Markdown
Contributor Author

I felt like crap all weekend... looks like you were busy, what else do we need to poke to make this work as an initial version?

I think the set override time (without temp) on the client is needed, as otherwise the on/off only blinks it on for a second or two.

Anything else?

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 29, 2019

Anything else?

Sorry, got side tracked with another PR (now done).

The changes are here: manzanotti/geniushub-client@v0.6.28...toggle_on_off_zones

It previous had a default duration of 0 seconds for override mode - it is now 3600 seconds.

I will have another look tomorrow - in the meantime, could you mark anything resolved as resolved.

@castaway
Copy link
Copy Markdown
Contributor Author

castaway commented Oct 29, 2019 via email

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 30, 2019

Please edit manifest.json to bump the client (NB: 0.6.29):

{
  "domain": "geniushub",
  "name": "Genius Hub",
  "documentation": "https://www.home-assistant.io/integrations/geniushub",
  "requirements": [
    "geniushub-client==0.6.29"
  ],
  "dependencies": [],
  "codeowners": ["@zxdavb"]
}

... then run: python3 -m script.gen_requirements_all

@zxdavb zxdavb added the dependency Pull requests marked as a dependency upgrade label Oct 30, 2019
@castaway
Copy link
Copy Markdown
Contributor Author

Please edit manifest.json to bump the client (NB: 0.6.29):

{
  "domain": "geniushub",
  "name": "Genius Hub",
  "documentation": "https://www.home-assistant.io/integrations/geniushub",
  "requirements": [
    "geniushub-client==0.6.29"
  ],
  "dependencies": [],
  "codeowners": ["@zxdavb"]
}

... then run: python3 -m script.gen_requirements_all

I was literally about to look how to do that, well mind read..

@castaway
Copy link
Copy Markdown
Contributor Author

Please edit manifest.json to bump the client (NB: 0.6.29):

{
  "domain": "geniushub",
  "name": "Genius Hub",
  "documentation": "https://www.home-assistant.io/integrations/geniushub",
  "requirements": [
    "geniushub-client==0.6.29"
  ],
  "dependencies": [],
  "codeowners": ["@zxdavb"]
}

... then run: python3 -m script.gen_requirements_all

I was literally about to look how to do that, well mind read..

Hrm, except that updated the reqs.. if I tell it then to install from requirements_all.txt (or just run it), it doesnt update.. I did pip3 install geniushub-client==0.6.29 instead

@castaway
Copy link
Copy Markdown
Contributor Author

Hrm. Didnt work - that is, still doesnt set override to an hour.. retested with just latest client from repo to make sure.. doesnt with ghclient either.. time to debug (over on the client repo)

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Oct 30, 2019

Hrm, except that updated the reqs.. if I tell it then to install from requirements_all.txt (or just run it), it doesnt update.. I did pip3 install geniushub-client==0.6.29 instead

HA installs the updated client library from PyPI next time you start HA.

Confirm this by removing geniushub-client, and restarting HA:

pip uninstall -y geniushub-client
pip list | grep genius
hass

...

pip list | grep genius

Hrm. Didnt work - that is, still doesnt set override to an hour.. retested with just latest client from repo to make sure.. doesnt with ghclient either.. time to debug (over on the client repo)

The script in this issue works for me - have a go at it: manzanotti/geniushub-client#25

You can push your latest PR, and I'll have a look.

@castaway
Copy link
Copy Markdown
Contributor Author

Hrm, except that updated the reqs.. if I tell it then to install from requirements_all.txt (or just run it), it doesnt update.. I did pip3 install geniushub-client==0.6.29 instead

HA installs the updated client library from PyPI next time you start HA.

Confirm this by removing geniushub-client, and restarting HA:

pip uninstall -y geniushub-client
pip list | grep genius
hass

...

pip list | grep genius

Aha, the "remove existing version" wasnt obvious, I assumed it would upgrade if given a higher version.. anyhoo manually got there in the end.

Hrm. Didnt work - that is, still doesnt set override to an hour.. retested with just latest client from repo to make sure.. doesnt with ghclient either.. time to debug (over on the client repo)

The script in this issue works for me - have a go at it: zxdavb/geniushub-client#25

You can push your latest PR, and I'll have a look.

I haven't changed it..

Hm, the ghclient.py one? Aha! I'm just calling --zone=X --mode=override .. which still doesn't send a duration (and I think ought to).

@zxdavb zxdavb force-pushed the feature/geniushub_switches branch from 173705d to 71026ed Compare November 3, 2019 19:25
@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Nov 3, 2019

Woops - my fault: sorted now (and rebased).

@zxdavb
Copy link
Copy Markdown
Member

zxdavb commented Nov 3, 2019

Well done! We can merge as soon as the docs PR is sorted.

You're not interested in adding services are you?
https://developers.home-assistant.io/docs/en/dev_101_services.html

castaway added a commit to castaway/home-assistant.io that referenced this pull request Nov 4, 2019
@castaway
Copy link
Copy Markdown
Contributor Author

castaway commented Nov 4, 2019

@zxdavb zxdavb changed the title Add switches (on/off) for geniushub outlets Add switches (on/off zones) to geniushub Nov 4, 2019
Copy link
Copy Markdown
Member

@zxdavb zxdavb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to go

@zxdavb zxdavb merged commit 5b85456 into home-assistant:dev Nov 4, 2019
@lock lock Bot locked and limited conversation to collaborators Nov 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants