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 Transmission component 'scan_interval' option #20575

Merged
merged 3 commits into from Feb 4, 2019

Conversation

@jonudewux
Copy link
Contributor

jonudewux commented Jan 29, 2019

Description:

Related issue (if applicable): fixes #20574

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

Example entry for configuration.yaml (if applicable):

  host: !secret trans_host
  port: !secret trans_port
  username: !secret trans_user
  password: !secret trans_pass
  name: trans
  scan_interval: 300
  monitored_conditions:
    - 'current_status'

Checklist:

  • The code change is tested and works locally.
    Locally tested as custom component.
  • 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:
  • Documentation added/updated in home-assistant.io

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

This comment has been minimized.

Copy link
Member

dgomes commented Jan 30, 2019

documentation needs to be updated to include the new parameter

@jonudewux jonudewux force-pushed the jonudewux:fix_transmisson branch from 2d19d45 to 305d5e8 Jan 30, 2019

@jonudewux

This comment has been minimized.

Copy link
Contributor Author

jonudewux commented Jan 30, 2019

Isn't it already described in a docs?
I was thinking, that this parameter must be supported for all components.

@jonudewux jonudewux closed this Jan 30, 2019

@wafflebot wafflebot bot removed the in progress label Jan 30, 2019

@jonudewux jonudewux reopened this Jan 30, 2019

@wafflebot wafflebot bot added the in progress label Jan 30, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Jan 30, 2019

This is a component now not a platform. There is no obligation to support this option.

@MartinHjelmare MartinHjelmare changed the title Transmission component fix 'scan_interval' option Add Transmission component 'scan_interval' option Jan 30, 2019

@jonudewux jonudewux referenced this pull request Feb 2, 2019

Closed

Update transmisson.markdown #8344

1 of 2 tasks complete

@jonudewux jonudewux force-pushed the jonudewux:fix_transmisson branch 4 times, most recently from d8c1601 to 899556f Feb 4, 2019

@jonudewux jonudewux force-pushed the jonudewux:fix_transmisson branch from 899556f to b081ae7 Feb 4, 2019

password = config[DOMAIN][CONF_PASSWORD]
port = config[DOMAIN][CONF_PORT]
conf = config[DOMAIN]
host = conf.get(CONF_HOST)

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 4, 2019

Member

Use dict[key] for required config keys and keys with default config schema values.

Show resolved Hide resolved homeassistant/components/transmission/__init__.py Outdated
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Good!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 4, 2019

Can be merged when build passes.

@jonudewux

This comment has been minimized.

Copy link
Contributor Author

jonudewux commented Feb 4, 2019

10x for support

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 4, 2019

It's weird that we both schedule our own update as well as use the built in update for entities in home assistant. We should set should_poll to False in the entities and remove the update call from update in the entities.

We could fix this in another PR, since it was already a problem before this PR.

@dgomes dgomes merged commit cd04661 into home-assistant:dev Feb 4, 2019

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.009%) to 93.357%
Details

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

@jonudewux jonudewux deleted the jonudewux:fix_transmisson branch Feb 4, 2019


def setup(hass, config):
"""Set up the Transmission Component."""
host = config[DOMAIN][CONF_HOST]
username = config[DOMAIN][CONF_USERNAME]
password = config[DOMAIN][CONF_PASSWORD]
username = config[DOMAIN].get(CONF_USERNAME)

This comment has been minimized.

@balloob

balloob Feb 7, 2019

Member

This should have been fixed independently from the feature and cherry-picked for 0.87.0

@balloob balloob added this to the 0.87.1 milestone Feb 7, 2019

balloob added a commit that referenced this pull request Feb 10, 2019

Add Transmission component 'scan_interval' option (#20575)
* Transmission component fix 'scan_interval' option

* Fix dict[key] comments

* Fix latest mess

@balloob balloob referenced this pull request Feb 10, 2019

Merged

0.87.1 #20930

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment