Add priority and cycles to LaMetric#14414
Merged
Merged
Conversation
Priority can be "info", "warning" (default), or "critical" and cycles is the number of times the message is displayed. If cycles is set to 0 we get a persistent notification that has to be dismissed manually.
homeassistant
added
cla-signed
integration: notify
core
small-pr
PhilRW
added a commit
to PhilRW/home-assistant.github.io
that referenced
this pull request
May 12, 2018
This documents the [change in the code](home-assistant/core#14414)
PhilRW
added a commit
to PhilRW/home-assistant.github.io
that referenced
this pull request
May 12, 2018
2 tasks
PhilRW
added a commit
to PhilRW/home-assistant.github.io
that referenced
this pull request
May 12, 2018
MartinHjelmare
requested changes
May 13, 2018
Member
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
Two small changes needed. Looks good otherwise.
| priority = data['priority'] | ||
| else: | ||
| _LOGGER.warning("Priority '%s' invalid, using default " | ||
| "'%s'" % (data['priority'], priority)) |
Member
There was a problem hiding this comment.
Don't put additional quotes around the priority variables:
https://developers.home-assistant.io/docs/en/development_guidelines.html#log-messages
Pass the priority variables as parameters:
_LOGGER.warning("Priority %s blabla %s", data['priority'], priority)| vol.Optional(CONF_ICON, default="i555"): cv.string, | ||
| vol.Optional(CONF_LIFETIME, default=10): cv.positive_int, | ||
| vol.Optional(CONF_CYCLES, default=1): cv.positive_int, | ||
| vol.Optional(CONF_PRIORITY, default="warning"): cv.string, |
Member
There was a problem hiding this comment.
AVAILABLE_PRIORITIES = ["info", "warning", "critical"]
...
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
...
vol.Optional(CONF_PRIORITY, default="warning"):
vol.In(AVAILABLE_PRIORITIES),
})| else: | ||
| _LOGGER.warning("Priority '%s' invalid, using default " | ||
| "'%s'" % (data['priority'], priority)) | ||
| _LOGGER.warning("Priority %s invalid, using default %s" % |
Member
There was a problem hiding this comment.
Don't use the percent/modulo sign. Just add the variables after comma.
_LOGGER.warning("Priority %s invalid, using default %s", data['priority'], priority)https://docs.python.org/3/library/logging.html#logging.Logger.debug
Contributor
Author
There was a problem hiding this comment.
Whoops. I've moved on to .format() so I rarely use %s otherwise...
MartinHjelmare
approved these changes
May 13, 2018
PhilRW
added a commit
to PhilRW/home-assistant.github.io
that referenced
this pull request
May 14, 2018
PhilRW
added a commit
to PhilRW/home-assistant.github.io
that referenced
this pull request
May 14, 2018
MartinHjelmare
pushed a commit
to home-assistant/home-assistant.io
that referenced
this pull request
May 14, 2018
Oro
pushed a commit
to Oro/home-assistant.github.io
that referenced
this pull request
May 20, 2018
Merged
girlpunk
pushed a commit
to girlpunk/home-assistant
that referenced
this pull request
Sep 4, 2018
* Add priority and cycles to LaMetric Priority can be "info", "warning" (default), or "critical" and cycles is the number of times the message is displayed. If cycles is set to 0 we get a persistent notification that has to be dismissed manually. * Fix for schema and style * Fix for style
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Priority can be "info", "warning" (default), or "critical" and cycles is the number of times the message is displayed. If cycles is set to 0 we get a persistent notification that has to be dismissed manually.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5358
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed: