8000 Fix Shelly button device triggers by thecode · Pull Request #48974 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix Shelly button device triggers #48974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Fix Shelly button device triggers #48974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 15 additions & 1 deletion homeassistant/components/shelly/device_trigger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
DOMAIN,
EVENT_SHELLY_CLICK,
INPUTS_EVENTS_SUBTYPES,
SHBTN_1_INPUTS_EVENTS_TYPES,
SUPPORTED_INPUTS_EVENTS_TYPES,
)
from .utils import get_device_wrapper, get_input_triggers
Expand All @@ -45,7 +46,7 @@ async def async_validate_trigger_config(hass, config):

# if device is available verify parameters against device capabilities
wrapper = get_device_wrapper(hass, config[CONF_DEVICE_ID])
if not wrapper:
if not wrapper or not wrapper.device.initialized:
return config

trigger = (config[CONF_TYPE], config[CONF_SUBTYPE])
Expand All @@ -68,6 +69,19 @@ async def async_get_triggers(hass: HomeAssistant, device_id: str) -> list[dict]:
if not wrapper:
raise InvalidDeviceAutomationConfig(f"Device not found: {device_id}")

if wrapper.model in ("SHBTN-1", "SHBTN-2"):
for trigger in SHBTN_1_INPUTS_EVENTS_TYPES:
triggers.append(
{
CONF_PLATFORM: "device",
CONF_DEVICE_ID: device_id,
CONF_DOMAIN: DOMAIN,
CONF_TYPE: trigger,
CONF_SUBTYPE: "button",
}
)
return triggers

for block in wrapper.device.blocks:
input_triggers = get_input_triggers(wrapper.device, block)

Expand Down
70 changes: 70 additions & 0 deletions tests/components/shelly/test_device_trigger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
"""The tests for Shelly device triggers."""
from unittest.mock import AsyncMock, Mock

import pytest

from homeassistant import setup
from homeassistant.components import automation
from homeassistant.components.device_automation.exceptions import (
InvalidDeviceAutomationConfig,
)
from homeassistant.components.shelly import ShellyDeviceWrapper
from homeassistant.components.shelly.const import (
ATTR_CHANNEL,
ATTR_CLICK_TYPE,
COAP,
CONF_SUBTYPE,
DATA_CONFIG_ENTRY,
DOMAIN,
EVENT_SHELLY_CLICK,
)
Expand Down Expand Up @@ -52,6 +57,71 @@ async def test_get_triggers(hass, coap_wrapper):
assert_lists_same(triggers, expected_triggers)


async def test_get_triggers_button(hass):
"""Test we get the expected triggers from a shelly button."""
await async_setup_component(hass, "shelly", {})

config_entry = MockConfigEntry(
domain=DOMAIN,
data={"sleep_period": 43200, "model": "SHBTN-1"},
unique_id="12345678",
)
config_entry.add_to_hass(hass)

device = Mock(
blocks=None,
settings=None,
shelly=None,
update=AsyncMock(),
initialized=False,
)

hass.data[DOMAIN] = {DATA_CONFIG_ENTRY: {}}
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to interact with integration details in the tests. We should just set up the integration and patch in the periphery, eg the library, as needed.

https://developers.home-assistant.io/docs/development_testing#writing-tests-for-integrations

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not specific to this test, I am planning to rewrite all the tests, already tried some options but I was not happy with the result, If it is OK by you I would like to use your help for mocking the library in the right way.

Copy link
Member

Choose a reason for hiding this comment

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

Sure!

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the library, one thing that makes the library harder to patch is that it is mixing the business logic of the Device class with the transport handling (aiohttp, coap and socket). If we'd break out the transport handling into a client class that the device can use, it would be easier to patch the client and know that all I/O is patched.

hass.data[DOMAIN][DATA_CONFIG_ENTRY][config_entry.entry_id] = {}
coap_wrapper = hass.data[DOMAIN][DATA_CONFIG_ENTRY][config_entry.entry_id][
COAP
] = ShellyDeviceWrapper(hass, config_entry, device)

await coap_wrapper.async_setup()

expected_triggers = [
{
CONF_PLATFORM: "device",
CONF_DEVICE_ID: coap_wrapper.device_id,
CONF_DOMAIN: DOMAIN,
CONF_TYPE: "single",
CONF_SUBTYPE: "button",
},
{
CONF_PLATFORM: "device",
CONF_DEVICE_ID: coap_wrapper.device_id,
CONF_DOMAIN: DOMAIN,
CONF_TYPE: "double",
CONF_SUBTYPE: "button",
},
{
CONF_PLATFORM: "device",
CONF_DEVICE_ID: coap_wrapper.device_id,
CONF_DOMAIN: DOMAIN,
CONF_TYPE: "triple",
CONF_SUBTYPE: "button",
},
{
CONF_PLATFORM: "device",
CONF_DEVICE_ID: coap_wrapper.device_id,
CONF_DOMAIN: DOMAIN,
CONF_TYPE: "long",
CONF_SUBTYPE: "button",
},
]

triggers = await async_get_device_automations(
hass, "trigger", coap_wrapper.device_id
)

assert_lists_same(triggers, expected_triggers)


async def test_get_triggers_for_invalid_device_id(hass, device_reg, coap_wrapper):
"""Test error raised for invalid shelly device_id."""
assert coap_wrapper
Expand Down
0