Skip to content

Make miflora monitored_conditions parameter optional#7598

Merged
balloob merged 4 commits into
home-assistant:devfrom
frog32:patch-miflora
May 16, 2017
Merged

Make miflora monitored_conditions parameter optional#7598
balloob merged 4 commits into
home-assistant:devfrom
frog32:patch-miflora

Conversation

@frog32
Copy link
Copy Markdown
Contributor

@frog32 frog32 commented May 14, 2017

Description:

If miflora monitored_conditions is not set default to monitoring all miflora sensors.

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

Checklist:

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

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@mention-bot
Copy link
Copy Markdown

@frog32, thanks for your PR! By analyzing the history of the files in this pull request, we identified @open-homeautomation, @fabaff and @Danielhiversen to be potential reviewers.

@hifiberry
Copy link
Copy Markdown

👍

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_MAC): cv.string,
vol.Required(CONF_MONITORED_CONDITIONS):
vol.Optional(CONF_MONITORED_CONDITIONS):
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.

Can you use the default keyword for vol.Optional instead

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_MAC): cv.string,
vol.Required(CONF_MONITORED_CONDITIONS):
vol.Optional(CONF_MONITORED_CONDITIONS, default=SENSOR_TYPES.keys()):
Copy link
Copy Markdown
Member

@pvizeli pvizeli May 15, 2017

Choose a reason for hiding this comment

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

cast default into a list

@balloob balloob dismissed pvizeli’s stale review May 16, 2017 06:13

It's all good 👍

@balloob
Copy link
Copy Markdown
Member

balloob commented May 16, 2017

Managed to simplify it even more, no need for list either as default iterable of a dictionary is the keys 👍

@balloob balloob merged commit d6081f3 into home-assistant:dev May 16, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 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.

7 participants