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 Satel_integra switchable outputs and multiple partitions #21992

Merged
merged 13 commits into from
Apr 13, 2019
Merged

Add Satel_integra switchable outputs and multiple partitions #21992

merged 13 commits into from
Apr 13, 2019

Conversation

c-soft
Copy link
Contributor

@c-soft c-soft commented Mar 12, 2019

Description:

Added multiple partitions support as well as editable outputs, so one can use Satel outputs as switches. Additionally improved reporting of arming and disarming states of the alarm.

Breaking change:
The component now supports multiple partitions and this forced change in config: instead of single parameters partition and single_home_mode there is now section partitions:. If your config so far was:

satel_integra:
  host: 192.168.1.100
  port: 7094
  partition: 1
  arm_home_mode: 1
  zones:
   (...)

... change it to:

satel_integra:
  host: 192.168.1.100
  port: 7094
  partitions: 
    01:
      name: 'House'
      arm_home_mode: 1
  zones:
    (...)

... and your config will be OK again after upgrade.

Related issue (if applicable): fixes #21589, fixes #19796, fixes #19361.

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

Example entry for configuration.yaml (if applicable):

satel_integra:
  host: 192.168.17.141
  port: 7094
  code: !secret alarm_code  
  partitions:
    01:
      name: 'Dom'
      arm_home_mode: 2
  zones:
    01:
      name: 'czujka wejście'
      type: 'motion'
    02:
      name: 'hol'
      type: 'motion'
    05:
      name: 'salon'
      type: 'motion'
  outputs:
    06:
      name: 'salon zb'
      type: 'motion'
    07:
      name: 'kuchnia dym'
      type: 'smoke'
  switchable_outputs:
    8:
      name: 'Brama garaż'
    9:
      name: 'Otwarcie bramy'

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.

If the code does not interact with devices:

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

Copy link
Contributor

@amelchio amelchio left a comment

Choose a reason for hiding this comment

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

This looks very good 👍. Just a couple of questions ...

homeassistant/components/satel_integra/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/satel_integra/switch.py Outdated Show resolved Hide resolved
@c-soft
Copy link
Contributor Author

c-soft commented Mar 27, 2019

Thanks for the review, Im on my vacation till next week, will fix it then.

@MartinHjelmare MartinHjelmare changed the title Satel_integra added switchable outputs and multiple partitions Add Satel_integra switchable outputs and multiple partitions Mar 28, 2019
@MartinHjelmare
Copy link
Member

MartinHjelmare commented Apr 5, 2019

There is a merge conflict.

@c-soft
Copy link
Contributor Author

c-soft commented Apr 12, 2019

There is a merge conflict.

Fixed, everything should be fine now.

@codecov

This comment has been minimized.

from homeassistant.core import callback
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.discovery import async_load_platform
from homeassistant.helpers.dispatcher import async_dispatcher_send

REQUIREMENTS = ['satel_integra==0.3.4']
Copy link
Member

Choose a reason for hiding this comment

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

We have removed this. Please remove it. It's now enough to define the requirements in the manifest.json file and run the scripts to validate the manifest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done - this was a merge error on my behalf.


async def async_alarm_arm_home(self, code=None):
"""Send arm home command."""
_LOGGER.debug("Arming home")

if code:
await self.hass.data[DATA_SATEL].arm(
code, self._arm_home_mode)
await self._satel\
Copy link
Member

Choose a reason for hiding this comment

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

Please break the line after the first parenthesis. That's the best way of formatting line breaks.

await self._satel.arm(
    code, [self._partition_id], self._arm_home_mode)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

"""Support for Satel Integra modifiable outputs represented as switches."""
import logging


Copy link
Member

Choose a reason for hiding this comment

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

Please remove one blank line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@callback
def _devices_updated(self, zones):
"""Update switch state, if needed."""
_LOGGER.debug("Update switch name: %s zones: %s.", self._name, zones)
Copy link
Member

Choose a reason for hiding this comment

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

We don't end logging messages with period.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #21992 into dev will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #21992      +/-   ##
==========================================
+ Coverage   93.93%   93.94%   +<.01%     
==========================================
  Files         451      451              
  Lines       36759    36758       -1     
==========================================
  Hits        34531    34531              
+ Misses       2228     2227       -1
Impacted Files Coverage Δ
homeassistant/components/rainmachine/const.py 100% <0%> (ø) ⬆️
homeassistant/components/aws/const.py 100% <0%> (ø) ⬆️
homeassistant/helpers/translation.py 98.73% <0%> (+0.03%) ⬆️
homeassistant/components/uk_transport/sensor.py 94.16% <0%> (+0.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73a473a...93d1084. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #21992 into dev will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #21992      +/-   ##
==========================================
+ Coverage   93.93%   93.94%   +<.01%     
==========================================
  Files         451      451              
  Lines       36759    36759              
==========================================
+ Hits        34531    34532       +1     
+ Misses       2228     2227       -1
Impacted Files Coverage Δ
homeassistant/components/uk_transport/sensor.py 94.16% <0%> (+0.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73a473a...93d1084. Read the comment docs.

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.

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants