Skip to content

Add Kira component to sensor and remote platforms#7479

Merged
balloob merged 2 commits into
home-assistant:devfrom
stu-gott:kira
May 13, 2017
Merged

Add Kira component to sensor and remote platforms#7479
balloob merged 2 commits into
home-assistant:devfrom
stu-gott:kira

Conversation

@stu-gott
Copy link
Copy Markdown
Contributor

@stu-gott stu-gott commented May 7, 2017

Description:

Support for Kira (Keene Electronics IR-IP bridge)

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

kira:
  sensors:
    - host: 0.0.0.0
      port: 65432
  remotes:
    - host: 192.168.100.1
      port: 65432

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@homeassistant
Copy link
Copy Markdown
Contributor

Hi @stu-gott,

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!

@mention-bot
Copy link
Copy Markdown

@stu-gott, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @happyleavesaoc to be potential reviewers.

Comment thread homeassistant/components/sensor/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread homeassistant/components/sensor/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread homeassistant/components/sensor/kira.py Outdated
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 under-indented for visual indent

Comment thread homeassistant/components/remote/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (83 > 79 characters)

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (81 > 79 characters)

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (81 > 79 characters)

Comment thread homeassistant/components/kira.py Outdated
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 under-indented for visual indent

Comment thread homeassistant/components/kira.py Outdated
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 unaligned for hanging indent

Comment thread homeassistant/components/kira.py Outdated
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 unaligned for hanging indent

Comment thread homeassistant/components/kira.py Outdated
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 don't use global. Use hass.data instead, it's a dictionary that you can store data in.

Comment thread homeassistant/components/kira.py Outdated
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.

Will this start a thread per sensor ?

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 will, however the large majority of installations will require only one sensor as all Kira modules can usually send to the same UDP port. The only time a user would ever need to configure more than one is to distinguish between the same IR code sent to different Kira Modules.

Comment thread homeassistant/components/remote/kira.py Outdated
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 already the default implementation, no need to overwrite it

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.

Ah! good catch. The original iteration of this class derived from the base remote class which is a ToggleEntity.

Comment thread homeassistant/components/remote/kira.py Outdated
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.

No need to overwrite this if you already overwrite async_turn_off. (which is faster so better.) Same for turn_on

Comment thread tests/components/test_kira.py Outdated
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 over-indented for visual indent

Comment thread tests/components/test_kira.py Outdated
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 over-indented for visual indent

Comment thread tests/components/test_kira.py Outdated
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 over-indented for visual indent

Comment thread tests/components/test_kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Comment thread tests/components/sensor/test_kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

Comment thread homeassistant/components/sensor/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

module level import not at top of file
redefinition of unused 'CONF_DEVICE' from line 7

Comment thread homeassistant/components/sensor/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'homeassistant.loader.get_component' imported but unused

Comment thread homeassistant/components/remote/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'homeassistant.loader.get_component' imported but unused

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

too many blank lines (2)

Comment thread homeassistant/components/kira.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'itertools' imported but unused

Comment thread tests/components/sensor/test_kira.py Outdated
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 under-indented for visual indent

Comment thread tests/components/remote/test_kira.py Outdated
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 under-indented for visual indent

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Nice! 🐬

@balloob balloob merged commit 9c4bc2a into home-assistant:dev May 13, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 2017
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.

6 participants