Add ability to specify a sender in the clicksend notification#11046
Conversation
| PLATFORM_SCHEMA, BaseNotificationService) | ||
| from homeassistant.const import ( | ||
| CONF_API_KEY, CONF_USERNAME, CONF_RECIPIENT, CONTENT_TYPE_JSON) | ||
| CONF_API_KEY, CONF_USERNAME, CONF_RECIPIENT, CONF_SENDER, CONTENT_TYPE_JSON) |
| vol.Required(CONF_USERNAME): cv.string, | ||
| vol.Required(CONF_API_KEY): cv.string, | ||
| vol.Required(CONF_RECIPIENT): cv.string, | ||
| vol.Optional(CONF_SENDER, default=""): cv.string, |
There was a problem hiding this comment.
Set the default to CONF_RECIPIENT no need to check it later.
There was a problem hiding this comment.
This doesn't seem to work for me, if I change it to this:
vol.Optional(CONF_SENDER, default=CONF_RECIPIENT): cv.string,
and don't set the sender in the config file, then sender is set to the string "recipient" rather than the actual value of the recipient variable. The config object isn't in scope so I can't use config.get(CONF_RECIPIENT) either.
There was a problem hiding this comment.
Right, then self.sender = config.get(CONF_SENDER, CONF_RECIPIENT) in setup().
|
@heydonms, can you please fix the CLA issue that we can merge this PR? Thanks. |
|
@fabaff, I filled in the form a couple of weeks ago. I did it again yesterday but it is still tagged cla-error. Is there something else that I need to do? |
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
CLA checks out, thanks!
See below for an improvement.
| self.username = config.get(CONF_USERNAME) | ||
| self.api_key = config.get(CONF_API_KEY) | ||
| self.recipient = config.get(CONF_RECIPIENT) | ||
| self.sender = config.get(CONF_SENDER, CONF_RECIPIENT) |
There was a problem hiding this comment.
It's better to add a schema validator that sets the default value during config validation.
def validate_sender(config):
if CONF_SENDER in config:
return config
config[CONF_SENDER] = config[CONF_RECIPIENT]
return configThen use this validator like this in the schema:
PLATFORM_SCHEMA = vol.Schema(
vol.All(PLATFORM_SCHEMA.extend({...}), validate_sender))
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
Nice! Just a leftover from testing below.
|
|
||
| def get_service(hass, config, discovery_info=None): | ||
| """Get the ClickSend notification service.""" | ||
| print("#### ", config) |
ghost
added
5 participants
Description:
Add the ability to specify a sender string to the notify.clicksend component (previous version used the recipient as the sender).
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4422
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests pass