Skip to content

Add All-Linking capabilities#14065

Merged
balloob merged 35 commits intohome-assistant:devfrom
teharris1:all-linking
May 5, 2018
Merged

Add All-Linking capabilities#14065
balloob merged 35 commits intohome-assistant:devfrom
teharris1:all-linking

Conversation

@teharris1
Copy link
Copy Markdown
Contributor

@teharris1 teharris1 commented Apr 24, 2018

Description:

In order for Insteon devices to work in a network they must be linked to the PLM and to each other. New services have been added to allow devices to be linked using the Home Assistant services under Development Tools.

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

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 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.

@balloob
Copy link
Copy Markdown
Member

balloob commented May 2, 2018

I have not seen prior PRs take more than a few days to get reviewed.

Please don't be inpatient. It's open source…

Comment thread homeassistant/components/insteon_plm.py Outdated

LOAD_ALDB_SCHEMA = vol.Schema({
vol.Required(CONF_ENTITY_ID): cv.entity_id,
vol.Optional(SRV_LOAD_DB_RELOAD, default='n'): vol.In(['Y', 'N',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't you just want a boolean?

Comment thread homeassistant/components/insteon_plm.py Outdated

ADD_ALL_LINK_SCHEMA = vol.Schema({
vol.Required(SRV_ALL_LINK_GROUP): vol.Range(min=0, max=255),
vol.Required(SRV_ALL_LINK_MODE): vol.In(['C', 'R',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't use single letters, write out the word what it means instead. Also, stick to just lowercase chars

Comment thread homeassistant/components/insteon_plm.py Outdated
def __init__(self, device, state_key):
def __init__(self, hass, device, state_key):
"""Initialize the INSTEON PLM binary sensor."""
self._hass = hass
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not necessary. All entities that are added to Home Assistant will have access to the hass object via self.hass

Comment thread homeassistant/components/insteon_plm.py Outdated
rec.group, rec.address.human,
rec.data1, rec.data2, rec.data3))

else:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use a guard clause.

if aldb.status not in […]:
    _LOGGER.warning('')
    return

for mem_addr in aldb:

self._hass = hass
self._insteon_device_state = device.states[state_key]
self._insteon_device = device
self._insteon_device.aldb.add_loaded_callback(self._aldb_loaded)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not print anything during startup. Print when requested via a service is ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This method is not printing here. It is registering a callback.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

my bad. Was confused because the callback will print, but I guess it only prints when asked to load.

Comment thread homeassistant/components/services.yaml Outdated
device_id:
description: Hardware address of the device to remove.
example: 158d0000000000
insteon_plm:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move insteon_plm to a new file insteon_plm/__init__.py and create a new services.yaml file in that folder.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Happy to do that. Do you want me to make the edits it components/insteon_plm.py then move and rename it? Otherwise you will have issues with compare (unless you do a diff outside git of course:) Just want to make it easier on you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that GitHub should be able to handle it if you use git mv

@teharris1
Copy link
Copy Markdown
Contributor Author

@balloob Thanks for taking the time to review this. Please don't think I was being impatient. My concern was that most of the PRs that came in after this were being reviewed and I was afraid it slipped between the cracks. Went I looked at the PR there did not appear to be a review request so I was concerned that the PR was never going to hit anyone's queue.

import insteonplm

ipdb = IPDB()
plm = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

unexpected spaces around keyword / parameter equals

vol.Optional(SRV_LOAD_DB_RELOAD, default='false'): vol.boolean),
})

PRINT_ALDB_SCHEMA = vol.Schema({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented
unexpected spaces around keyword / parameter equals


LOAD_ALDB_SCHEMA = vol.Schema({
vol.Required(CONF_ENTITY_ID): cv.entity_id,
vol.Optional(SRV_LOAD_DB_RELOAD, default='false'): vol.boolean),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax

import insteonplm

ipdb = IPDB()
plm = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

unexpected spaces around keyword / parameter equals

vol.Optional(SRV_LOAD_DB_RELOAD, default='false'): vol.boolean),
})

PRINT_ALDB_SCHEMA = vol.Schema({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented
unexpected spaces around keyword / parameter equals


LOAD_ALDB_SCHEMA = vol.Schema({
vol.Required(CONF_ENTITY_ID): cv.entity_id,
vol.Optional(SRV_LOAD_DB_RELOAD, default='false'): vol.boolean),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax

rec.group, rec.address.human,
rec.data1, rec.data2, rec.data3))


No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blank line at end of file
blank line contains whitespace

' {:3d} {:3d} {:3d}'.format(
rec.mem_addr, in_use, mode, hwm,
rec.group, rec.address.human,
rec.data1, rec.data2, rec.data3))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

_LOGGER.info(' {:04x} {:s} {:s} {:s} {:3d} {:s}'
' {:3d} {:3d} {:3d}'.format(
rec.mem_addr, in_use, mode, hwm,
rec.group, rec.address.human,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

hwm = 'Y' if rec.control_flags.is_high_water_mark else 'N'
_LOGGER.info(' {:04x} {:s} {:s} {:s} {:3d} {:s}'
' {:3d} {:3d} {:3d}'.format(
rec.mem_addr, in_use, mode, hwm,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

@balloob balloob merged commit 64ba2c6 into home-assistant:dev May 5, 2018
@balloob balloob mentioned this pull request May 11, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
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