New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial hlk-sw16 relay switch support #17855

Merged
merged 20 commits into from Dec 3, 2018

Conversation

Projects
None yet
6 participants
@jameshilliard
Copy link
Contributor

jameshilliard commented Oct 27, 2018

Description:

This PR adds initial support for controlling the HLK-SW16. I plan to use this for controlling a multi-zone hot water heating system in addition to possible other low voltage devices such as sprinklers.

The MCU reference code for the device is available here. The device does have an internal timer feature as well but I'm not sure if there's a reason to add support for that in home-assistant.

Currently I have implemented/tested toggling all 16 relays individually in addition to reading their current state on startup, I'm looking to see if my general approach looks correct and how I might best expose the individual switch controls and how I would best configure this for controlling hot water zone valves. I'm still working on cleaning things up.

Documentation PR: home-assistant/home-assistant.io#7250

Example entry for configuration.yaml (if applicable):

hlk_sw16:
  relay1:
    host: 10.225.225.53
    switches:
      0:
        name: relay1-0
      1:
        name: relay1-1
      2:
        name: relay1-2
      3:
        name: relay1-3
      4:
        name: relay1-4
      5:
        name: relay1-5
      6:
        name: relay1-6
      7:
        name: relay1-7
      8:
        name: relay1-8
      9:
        name: relay1-9
      a:
        name: relay1-a
      b:
        name: relay1-b
      c:
        name: relay1-c
      d:
        name: relay1-d
      e:
        name: relay1-e
      f:
        name: relay1-f

  relay2:
    host: 10.225.225.55
    switches:
      0:
        name: relay2-0
      1:
        name: relay2-1
      2:
        name: relay2-2
      3:
        name: relay2-3
      4:
        name: relay2-4
      5:
        name: relay2-5
      6:
        name: relay2-6
      7:
        name: relay2-7
      8:
        name: relay2-8
      9:
        name: relay2-9
      a:
        name: relay2-a
      b:
        name: relay2-b
      c:
        name: relay2-c
      d:
        name: relay2-d
      e:
        name: relay2-e
      f:
        name: relay2-f

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
@homeassistant

This comment has been minimized.

Copy link

homeassistant commented Oct 27, 2018

Hi @jameshilliard,

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!

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'
@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@homeassistant homeassistant added cla-signed and removed cla-needed labels Oct 27, 2018

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from a08ccda to e733a3c Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from e733a3c to 8554aaa Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from 8554aaa to fa7246b Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from fa7246b to f2ffd56 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from f2ffd56 to dfb2391 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from dfb2391 to f2c1185 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from f2c1185 to 2930b55 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from 2930b55 to 0e047f9 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from 0e047f9 to afd48f7 Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from afd48f7 to d20069b Oct 27, 2018

@houndci-bot
Copy link

houndci-bot left a comment

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/switch/hlk_sw16.py Outdated

@jameshilliard jameshilliard force-pushed the jameshilliard:hlk-sw16 branch from 036661f to 9c7e1c0 Nov 8, 2018

@jameshilliard

This comment has been minimized.

Copy link
Contributor

jameshilliard commented Nov 8, 2018

I've refactored the client library here.

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

The client approach is a lot better. 👍

Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
@jameshilliard

This comment has been minimized.

Copy link
Contributor

jameshilliard commented Nov 8, 2018

Current connection resumption behavior is now for the client to process the queue from where it last left off from so that commands executed while offline are not missed.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Nov 8, 2018

Consider limiting processing of queue to last state. Stale state changes that are applied later might be bad, eg if state is flipped back and forth multiple times.

@jameshilliard

This comment has been minimized.

Copy link
Contributor

jameshilliard commented Nov 8, 2018

Consider limiting processing of queue to last state.

That would be tricky I think with how I designed the protocol implementation. The calls themselves eg await self._client.turn_on(self._device_port) however are await'd until actual confirmed execution so it should be possible in theory for the callers to know not to call again until there is successful execution.

Stale state changes that are applied later might be bad, eg if state is flipped back and forth multiple times.

Well the device itself shouldn't really have trouble handling that, it processes the commands and returns responses quite quickly.

@jameshilliard

This comment has been minimized.

Copy link
Contributor

jameshilliard commented Nov 8, 2018

I've added timeouts here which should result in the device getting marked as unavailable within 10 seconds of connection loss. Hopefully that should prevent home-assistant from trying to queue too many state changes.

@frenck frenck removed the docs-missing label Nov 19, 2018

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Just two clean ups needed. Otherwise looks good!

Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
Show resolved Hide resolved homeassistant/components/hlk_sw16.py Outdated
@jameshilliard

This comment has been minimized.

Copy link
Contributor

jameshilliard commented Dec 2, 2018

removed those now

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 2, 2018

Great! Can be merged when build passes.

@MartinHjelmare MartinHjelmare merged commit 832fa61 into home-assistant:dev Dec 3, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 92.933%
Details

@wafflebot wafflebot bot removed the in progress label Dec 3, 2018

@balloob balloob referenced this pull request Dec 12, 2018

Merged

0.84 #19215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment