Add HipChat notify service. #3166
Conversation
|
I just noticed that a 0.51 PR was created about the same time I was making these PR's. Let me know if I should bump the |
| footer: true | ||
| logo: hipchat.png | ||
| ha_category: Notifications | ||
| ha_release: "0.51" |
There was a problem hiding this comment.
Bump to 0.52 as suggested 👍
| --- | ||
|
|
||
|
|
||
| The `hipchat` platform allows you to deliver notifications from Home Assistant to [HipChat](https://hipchat.com/). |
There was a problem hiding this comment.
Replace deliver with send?
| - name: NOTIFIER_NAME | ||
| platform: hipchat | ||
| token: ABCDEFGHJKLMNOPQRSTUVXYZ | ||
| room: '1234567' |
There was a problem hiding this comment.
Are the quotes around the number necessary?
There was a problem hiding this comment.
The API treats it as a string rather than a number, and I suppose there's the possibility that a room ID might be non-numeric (though I haven't seen any).
edit: I could certainly accept an int instead of str and then cast back to str when calling API, it would only be a minor code change, but as it is currently implemented a str is required/assumed
edit2: Actually, I may be an idiot. nothing else is using quotes that are strings. I guess we don't need them.
|
|
||
| - **name** (*Optional*): Setting the optional parameter `name` allows multiple notifiers to be created. The default value is `notify`. The notifier will bind to the service `notify.NOTIFIER_NAME`. | ||
| - **token** (*Required*): The HipChat API token to use for sending HipChat notifications. | ||
| - **room** (*Required*): The default room to post to if no room is explicitly specified when sending the notification message. |
There was a problem hiding this comment.
Suggest you remove the word message since it's redundant.
| - **token** (*Required*): The HipChat API token to use for sending HipChat notifications. | ||
| - **room** (*Required*): The default room to post to if no room is explicitly specified when sending the notification message. | ||
| - **color** (*Optional*): Setting color will override the default color for the notification. By default not setting this will post to HipChat using the default color yellow. Valid options are 'yellow', 'green', 'red', 'purple', 'gray', 'random'. | ||
| - **notify** (*Optional*): Setting notify will override the default notify (blink application icon, chime, or otherwise call attention) setting for the notification. By default this is false. Valid options are true and false. |
There was a problem hiding this comment.
Wrap the available values in quotes for readability.
true,false.
There was a problem hiding this comment.
It should be true / false (bool) not 'true' / 'false' (str), so I didn't put in quotes. I can change it, or is there a better third option for making it clear without implying str?
There was a problem hiding this comment.
I guess it may be irrelevant since it should be cast appropriately to bool by voluptuous? In which case I'll make that change to use quotes. The gotcha though would be that magic str to bool doesn't happen when passing data JSON to notify service, probably, so should clarify in that section that it should be bool.
|
@Landrash I've made changes based on your suggestions. |
|
Updated docs to reflect changes made to use a PyPi library. |
Description:
Documentation for HipChat notification component.
Pull request in home-assistant (if applicable): home-assistant/core#8918
I'm making an assumption in the docs that the
ha_releasewill be 0.51. I didn't find any recommendation for what you should put when it's not released yet.