Skip to content

light.tplink: initialize supported features in setup_platform#15534

Closed
rytilahti wants to merge 1 commit intohome-assistant:devfrom
rytilahti:tplink_fix_nontemp_adjustables
Closed

light.tplink: initialize supported features in setup_platform#15534
rytilahti wants to merge 1 commit intohome-assistant:devfrom
rytilahti:tplink_fix_nontemp_adjustables

Conversation

@rytilahti
Copy link
Copy Markdown
Member

Description:

Fetch supported features already in setup_platform, avoids calling {min,max}_minreds (thanks to #15484 ) causing a zerodivisionerror on bulbs not supporting temperature changing.

Related issue (if applicable): fixes #15339

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):

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

@ghost ghost assigned rytilahti Jul 18, 2018
@ghost ghost added the in progress label Jul 18, 2018
@rytilahti rytilahti changed the title tplink.light: initialize supported features in setup_platform light.tplink: initialize supported features in setup_platform Jul 18, 2018
name = config.get(CONF_NAME)
add_devices([TPLinkSmartBulb(SmartBulb(host), name)], True)
bulb = TPLinkSmartBulb(SmartBulb(host), name)
bulb.get_features()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not keep this call inside the TPLinkSmartBulb class?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because I/O is not allowed inside constructor, and if those features are not set min_mireds may be called before there an initial update has happened. Or that's my understanding at least.

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.

In async_update_ha_state the call to update will be done before any entity properties are accessed. I propose to revert all changes except on line 45, which will make sure that update is called before the first state update, and that supported features should be up to date.

Copy link
Copy Markdown
Member Author

@rytilahti rytilahti Jul 19, 2018

Choose a reason for hiding this comment

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

Hmm, if that's the case, there's then no need for this PR at all actually and the problem itself should have been fixed by #15484, as the second parameter was True already.

@rytilahti rytilahti closed this Jul 19, 2018
@ghost ghost removed the in progress label Jul 19, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
@rytilahti rytilahti deleted the tplink_fix_nontemp_adjustables branch October 22, 2019 19:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0.73.0 - light.tplink - division by zero

4 participants