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

Add LCN cover platform #20288

Merged
merged 4 commits into from Feb 23, 2019

Conversation

Projects
None yet
4 participants
@alengwenus
Copy link
Contributor

alengwenus commented Jan 21, 2019

Description:

This adds the LCN cover platform. The implementation follows the already merged LCN light and switch platforms.

Related issue (if applicable):

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8234

Example entry for configuration.yaml (if applicable):

lcn:
  connections:
    - name: myhome
      host: 192.168.2.41
      port: 4114
      username: !secret lcn_username
      password: !secret lcn_password

  covers:
    - name: Living room cover
      address: myhome.0.7
      motor: motor1

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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.
Show resolved Hide resolved homeassistant/components/cover/lcn.py Outdated
Show resolved Hide resolved homeassistant/components/cover/lcn.py Outdated
Show resolved Hide resolved homeassistant/components/cover/lcn.py Outdated
Show resolved Hide resolved homeassistant/components/cover/lcn.py Outdated
Show resolved Hide resolved homeassistant/components/lcn.py Outdated
@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 21, 2019

There are merge conflicts.

@alengwenus alengwenus force-pushed the alengwenus:dev_lcn_cover branch from 4b68f29 to d08bf89 Feb 22, 2019

@alengwenus

This comment has been minimized.

Copy link
Contributor Author

alengwenus commented Feb 22, 2019

Unfortunately I had to force push because of the changed component filesystem structure. But I incorporated all your requested changes @MartinHjelmare.

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Is it ok that stop could make the state incorrect, at least for a while?

Show resolved Hide resolved homeassistant/components/lcn/cover.py
Show resolved Hide resolved homeassistant/components/lcn/cover.py

alengwenus added some commits Jan 21, 2019

@alengwenus alengwenus force-pushed the alengwenus:dev_lcn_cover branch from d08bf89 to 3c1206a Feb 23, 2019

@alengwenus

This comment has been minimized.

Copy link
Contributor Author

alengwenus commented Feb 23, 2019

Is it ok that stop could make the state incorrect, at least for a while?

Set closed state to None when stoping, so that open, close can be activated after stop.


DIM_MODES = ['STEPS50', 'STEPS200']
OUTPUT_PORTS = ['OUTPUT1', 'OUTPUT2', 'OUTPUT3', 'OUTPUT4']
RELAY_PORTS = ['RELAY1', 'RELAY2', 'RELAY3', 'RELAY4',
'RELAY5', 'RELAY6', 'RELAY7', 'RELAY8']
'RELAY5', 'RELAY6', 'RELAY7', 'RELAY8',
'MOTORONOFF1', 'MOTORUPDOWN1', 'MOTORONOFF2', 'MOTORUPDOWN2',

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

Why do we extend the relay ports? Will the cover be exposed as a light or switch too?

This comment has been minimized.

@alengwenus

alengwenus Feb 23, 2019

Author Contributor

The relais ports of the LCN hardware modules are also used for motor controls (with a certain wiring described in the docs). I added the constants just for completeness. They are just synonyms for the relais ports. The motor (aka relais) ports could be used as switches in combination with the template cover platform (for example if someone wants to add end-switch sensors).

@MartinHjelmare MartinHjelmare merged commit 8f70c16 into home-assistant:dev Feb 23, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on dev_lcn_cover at 92.712%
Details

@wafflebot wafflebot bot removed the in progress label Feb 23, 2019

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Please make a new PR with the adjustment below. Sorry about missing this.

('switch', CONF_SWITCHES)):
hass.async_create_task(
async_load_platform(hass, component, DOMAIN,
config[DOMAIN][conf_key], config))

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 23, 2019

Member

I just realized that the platform config keys are optional, so we have to check for that before using it and loading the platform.

This comment has been minimized.

@alengwenus

alengwenus Feb 23, 2019

Author Contributor

Thanks! I should have seen this by myself.
Here we go: #21354

@alengwenus alengwenus deleted the alengwenus:dev_lcn_cover branch Feb 25, 2019

@balloob balloob referenced this pull request Mar 6, 2019

Merged

0.89.0 #21712

@tango38317

This comment has been minimized.

Copy link

tango38317 commented Mar 8, 2019

Hi @alengwenus ,
i tried to configure the lcn cover but have issues with direcotio of covers. if i press up the covers go down, if i press down the covers goes up - maybe you have an idea how to solve this issues? or do you need further information or log files?
many thx
Chris

@alengwenus

This comment has been minimized.

Copy link
Contributor Author

alengwenus commented Mar 8, 2019

Hi @tango38317,
please check your cover wiring. It has to be exactly as given in the manufacturer documentation for the LCN-R8H or LCN-R4M2H. I rechecked everything on my side. Seems to be ok. The R4M2H has some builtin leds in the housing. They also show the right state when switching. If you have the possibility, send up/down commands via LCN-Pro software to check if your wiring is inverted.
If everything fails and you are not able to change the wiring, you could use the template cover platform in combination with the LCN switch platform (relays) to setup a custom cover behaviour.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Mar 8, 2019

@tango38317

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Mar 8, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.