Skip to content
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

jameshilliard
Copy link
Contributor

@jameshilliard 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
Copy link
Contributor

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!

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

I've refactored the client library here.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

The client approach is a lot better. 👍

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 Show resolved Hide resolved
@jameshilliard
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Just two clean ups needed. Otherwise looks good!

homeassistant/components/hlk_sw16.py Outdated Show resolved Hide resolved
homeassistant/components/hlk_sw16.py Outdated Show resolved Hide resolved
@jameshilliard
Copy link
Contributor Author

removed those now

@MartinHjelmare
Copy link
Member

Great! Can be merged when build passes.

@MartinHjelmare MartinHjelmare merged commit 832fa61 into home-assistant:dev Dec 3, 2018
@ghost ghost removed the in progress label Dec 3, 2018
@balloob balloob mentioned this pull request Dec 12, 2018
@jameshilliard jameshilliard deleted the hlk-sw16 branch March 15, 2019 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants