Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions homeassistant/components/simplepush/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
DEFAULT_NAME: Final = "simplepush"
DATA_HASS_CONFIG: Final = "simplepush_hass_config"

ATTR_ACTIONS: Final = "actions"
ATTR_ATTACHMENTS: Final = "attachments"
ATTR_ENCRYPTED: Final = "encrypted"
ATTR_EVENT: Final = "event"
ATTR_FEEDBACK_ACTION_TIMEOUT: Final = "feedback_action_timeout"

EVENT_ACTION_TRIGGERED: Final = "simplepush_action_triggered_event"

CONF_DEVICE_KEY: Final = "device_key"
CONF_SALT: Final = "salt"
111 changes: 98 additions & 13 deletions homeassistant/components/simplepush/notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import logging
from typing import Any

from simplepush import BadRequest, UnknownError, send
from simplepush import BadRequest, FeedbackActionTimeout, UnknownError, async_send

from homeassistant.components.notify import (
ATTR_DATA,
Expand All @@ -15,10 +15,20 @@
)
from homeassistant.const import CONF_EVENT, CONF_PASSWORD
from homeassistant.core import HomeAssistant
from homeassistant.helpers.aiohttp_client import async_get_clientsession
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType

from .const import ATTR_EVENT, CONF_DEVICE_KEY, CONF_SALT, DOMAIN
from .const import (
ATTR_ACTIONS,
ATTR_ATTACHMENTS,
ATTR_EVENT,
ATTR_FEEDBACK_ACTION_TIMEOUT,
CONF_DEVICE_KEY,
CONF_SALT,
DOMAIN,
EVENT_ACTION_TRIGGERED,
)

# Configuring Simplepush under the notify has been removed in 2022.9.0
PLATFORM_SCHEMA = BASE_PLATFORM_SCHEMA
Expand All @@ -44,45 +54,120 @@ async def async_get_service(
)
return None

return SimplePushNotificationService(discovery_info)
return SimplePushNotificationService(hass, discovery_info)


class SimplePushNotificationService(BaseNotificationService):
"""Implementation of the notification service for Simplepush."""

def __init__(self, config: dict[str, Any]) -> None:
def __init__(self, hass: HomeAssistant, config: dict[str, Any]) -> None:
"""Initialize the Simplepush notification service."""
self.hass = hass
self.session = async_get_clientsession(hass)
self._device_key: str = config[CONF_DEVICE_KEY]
self._event: str | None = config.get(CONF_EVENT)
self._password: str | None = config.get(CONF_PASSWORD)
self._salt: str | None = config.get(CONF_SALT)

def send_message(self, message: str, **kwargs: Any) -> None:
async def async_send_message(self, message: str, **kwargs: Any) -> None:
"""Send a message to a Simplepush user."""
title = kwargs.get(ATTR_TITLE, ATTR_TITLE_DEFAULT)

actions = None
action_ids = {}
attachments = None
# event can now be passed in the service data
event = None
feedback_action_timeout = None

if data := kwargs.get(ATTR_DATA):
event = data.get(ATTR_EVENT)

actions_data = data.get(ATTR_ACTIONS)
if isinstance(actions_data, list) and actions_data:
actions = []
for action in actions_data:
if "action" in action and "url" in action:
actions.append({"name": action["action"], "url": action["url"]})
elif "action" in action:
actions.append(action["action"])
if "id" in action:
action_ids.update({action["action"]: action["id"]})

try:
feedback_action_timeout = int(
data.get(ATTR_FEEDBACK_ACTION_TIMEOUT)
)
except (ValueError, TypeError):
feedback_action_timeout = 60

attachments_data = data.get(ATTR_ATTACHMENTS)

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.

Please limit the PR to one thing. Don't both add callbacks and attachments.

@tymm tymm Oct 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I removed attachments from this PR and create a separate PR for them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I created #81033 for attachments.

if isinstance(attachments_data, list) and attachments_data:
attachments = []
for attachment in attachments_data:
if "attachment" in attachment and "thumbnail" in attachment:
attachments.append(
{
"video": attachment["attachment"],
"thumbnail": attachment["thumbnail"],
}
)
elif "attachment" in attachment:
attachments.append(attachment["attachment"])

# use event from config until YAML config is removed
event = event or self._event

def feedback_action_callback(
action_selected, action_selected_at, action_delivered_at, feedback_id
):
payload = {
"action_selected": action_selected,
"action_selected_at": action_selected_at,
"action_delivered_at": action_delivered_at,
"feedback_id": feedback_id,
}

if action_selected in action_ids:
payload.update({"id": action_ids[action_selected]})

self.hass.bus.async_fire(EVENT_ACTION_TRIGGERED, payload)

try:
if self._password:
send(
key=self._device_key,
password=self._password,
salt=self._salt,
title=title,
message=message,
event=event,
self.hass.async_create_task(

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.

If we create a task the exceptions won't be caught here. We should await here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, this is a bit tricky.
If I don't create a task and use await instead, then a wait_for_trigger (see the example I provided in the description) will never be triggered.
And the possibility to use wait_for_trigger is crucial for this feature.

Do you have an idea on how to resolve this?

async_send(
key=self._device_key,
password=self._password,
salt=self._salt,
title=title,
message=message,
actions=actions,
attachments=attachments,
event=event,
feedback_callback=feedback_action_callback,

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.

The callback will potentially make a request every second for one minute. That doesn't seem prudent.

https://github.com/simplepush/simplepush-python/blob/52f4cf2b072f42a1dde94698f5049ce122dddfb8/simplepush/__init__.py#L258-L286

n is never increased in the loop either.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While it is certainly not prudent, I think it is a good enough first step considering that I will probably be the only user of this feature for a while anyway and an alternative solution would require a lot of work.

However, I would of course understand if Home Assistant doesn't want to subscribe to the move-fast-break-things philosophy as much as I have to.

Could there be a compromise for now where we set a fixed timeout?

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.

We're not some playground. We're real people spending real time maintaining Home Assistant and reviewing PRs. If you are making something that is only used by yourself and it's full of hacks… it doesn't belong in HA. I suggest to make it available as a custom component that people can install to HACS. There are 900 PRs open across Home Assistant, all intended for a bigger audience.

feedback_callback_timeout=feedback_action_timeout,
aiohttp_session=self.session,
Comment thread
MartinHjelmare marked this conversation as resolved.
)
)
else:
send(key=self._device_key, title=title, message=message, event=event)
self.hass.async_create_task(
async_send(
key=self._device_key,
title=title,
message=message,
actions=actions,
attachments=attachments,
event=event,
feedback_callback=feedback_action_callback,
feedback_callback_timeout=feedback_action_timeout,
aiohttp_session=self.session,
)
)

except BadRequest:
_LOGGER.error("Bad request. Title or message are too long")
except UnknownError:
_LOGGER.error("Failed to send the notification")
except FeedbackActionTimeout:
_LOGGER.error("Feedback action timed out")