Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
106 changes: 97 additions & 9 deletions homeassistant/components/synology_dsm/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@

from synology_dsm import SynologyDSM
from synology_dsm.api.core.security import SynoCoreSecurity
from synology_dsm.api.core.system import SynoCoreSystem
from synology_dsm.api.core.upgrade import SynoCoreUpgrade
from synology_dsm.api.core.utilization import SynoCoreUtilization
from synology_dsm.api.dsm.information import SynoDSMInformation
from synology_dsm.api.dsm.network import SynoDSMNetwork
from synology_dsm.api.storage.storage import SynoStorage
from synology_dsm.api.surveillance_station import SynoSurveillanceStation
from synology_dsm.exceptions import SynologyDSMRequestException
from synology_dsm.exceptions import (
SynologyDSMLoginFailedException,
SynologyDSMRequestException,
)
import voluptuous as vol

from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry
Expand All @@ -29,7 +33,7 @@
CONF_USERNAME,
CONF_VERIFY_SSL,
)
from homeassistant.core import callback
from homeassistant.core import ServiceCall, callback
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers import entity_registry
import homeassistant.helpers.config_validation as cv
Expand All @@ -42,6 +46,7 @@
from homeassistant.helpers.typing import HomeAssistantType

from .const import (
CONF_SERIAL,
CONF_VOLUMES,
DEFAULT_SCAN_INTERVAL,
DEFAULT_USE_SSL,
Expand All @@ -53,6 +58,9 @@
ENTITY_NAME,
ENTITY_UNIT,
PLATFORMS,
SERVICE_REBOOT,
SERVICE_SHUTDOWN,
SERVICES,
STORAGE_DISK_BINARY_SENSORS,
STORAGE_DISK_SENSORS,
STORAGE_VOL_SENSORS,
Expand Down Expand Up @@ -170,7 +178,8 @@ def _async_migrator(entity_entry: entity_registry.RegistryEntry):
api = SynoApi(hass, entry)
try:
await api.async_setup()
except SynologyDSMRequestException as err:
except (SynologyDSMLoginFailedException, SynologyDSMRequestException) as err:
Comment thread
Quentame marked this conversation as resolved.
_LOGGER.debug("async_setup_entry - Unable to connect to DSM: %s", str(err))
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 start logging messages with capital letter.

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 don't need to copy exceptions to string when logging them. They have a __str__ magic method already.

raise ConfigEntryNotReady from err

undo_listener = entry.add_update_listener(_async_update_listener)
Expand All @@ -181,6 +190,9 @@ def _async_migrator(entity_entry: entity_registry.RegistryEntry):
UNDO_UPDATE_LISTENER: undo_listener,
}

# Services
await _async_setup_services(hass)

# For SSDP compat
if not entry.data.get(CONF_MAC):
network = await hass.async_add_executor_job(getattr, api.dsm, "network")
Expand Down Expand Up @@ -221,6 +233,49 @@ async def _async_update_listener(hass: HomeAssistantType, entry: ConfigEntry):
await hass.config_entries.async_reload(entry.entry_id)


async def _async_setup_services(hass: HomeAssistantType):
"""Service handler setup."""

async def service_handler(call: ServiceCall):
"""Handle service call."""
_LOGGER.debug(
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.

This is already logged by the core.

"service_handler - called as '%s' with data: %s", call.service, call.data
)
serial = call.data.get(CONF_SERIAL)
dsm_devices = hass.data[DOMAIN]
Comment thread
Quentame marked this conversation as resolved.

if serial:
dsm_device = dsm_devices.get(serial)
elif len(dsm_devices) == 1:
dsm_device = next(iter(dsm_devices.values()))
serial = next(iter(dsm_devices))
else:
_LOGGER.error(
"service_handler - more than one DSM configured, must specify one of serials %s",
sorted(dsm_devices),
)
return

if not dsm_device:
_LOGGER.error(
"service_handler - DSM with specified serial %s not found", serial
)
return

_LOGGER.info("%s DSM with serial %s", call.service, serial)
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.

Max debug level.

dsm_api = dsm_device[SYNO_API]
if call.service == SERVICE_REBOOT:
await dsm_api.async_reboot()
elif call.service == SERVICE_SHUTDOWN:
await dsm_api.system.shutdown()

for service in SERVICES:
_LOGGER.debug(
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.

Logging the services we register seems not needed. There's no variation.

"_async_setup_services - register service %s on domain %s", service, DOMAIN
)
hass.services.async_register(DOMAIN, service, service_handler)
Comment thread
Quentame marked this conversation as resolved.


class SynoApi:
"""Class to interface with Synology DSM API."""

Expand All @@ -234,19 +289,21 @@ def __init__(self, hass: HomeAssistantType, entry: ConfigEntry):
self.information: SynoDSMInformation = None
self.network: SynoDSMNetwork = None
self.security: SynoCoreSecurity = None
self.upgrade: SynoCoreUpgrade = None
self.storage: SynoStorage = None
self.utilisation: SynoCoreUtilization = None
self.surveillance_station: SynoSurveillanceStation = None
self.system: SynoCoreSystem = None
self.upgrade: SynoCoreUpgrade = None
self.utilisation: SynoCoreUtilization = None

# Should we fetch them
self._fetching_entities = {}
self._with_information = True
self._with_security = True
self._with_storage = True
self._with_surveillance_station = True
self._with_system = True
self._with_upgrade = True
self._with_utilisation = True
self._with_information = True
self._with_surveillance_station = True

self._unsub_dispatcher = None

Expand Down Expand Up @@ -315,6 +372,7 @@ def _async_setup_api_requests(self):
self._fetching_entities.get(SynoCoreSecurity.API_KEY)
)
self._with_storage = bool(self._fetching_entities.get(SynoStorage.API_KEY))
self._with_system = bool(self._fetching_entities.get(SynoCoreSystem.API_KEY))
self._with_upgrade = bool(self._fetching_entities.get(SynoCoreUpgrade.API_KEY))
self._with_utilisation = bool(
self._fetching_entities.get(SynoCoreUtilization.API_KEY)
Expand All @@ -337,6 +395,10 @@ def _async_setup_api_requests(self):
self.dsm.reset(self.storage)
self.storage = None

if not self._with_system:
self.dsm.reset(self.system)
self.system = None

if not self._with_upgrade:
self.dsm.reset(self.upgrade)
self.upgrade = None
Expand Down Expand Up @@ -364,21 +426,47 @@ def _fetch_device_configuration(self):
if self._with_upgrade:
self.upgrade = self.dsm.upgrade

if self._with_system:
self.system = self.dsm.system

if self._with_utilisation:
self.utilisation = self.dsm.utilisation

if self._with_surveillance_station:
self.surveillance_station = self.dsm.surveillance_station

async def async_reboot(self):
"""Reboot NAS."""
if not self.system:
_LOGGER.debug("async_reboot - System API not ready: %s", self)
return
self._hass.async_add_executor_job(self.system.reboot)
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.

Why do we not await this executor job?


async def async_shutdown(self):
"""Shutdown NAS."""
if not self.system:
_LOGGER.debug("async_shutdown - System API not ready: %s", self)
return
self._hass.async_add_executor_job(self.system.shutdown)

async def async_unload(self):
"""Stop interacting with the NAS and prepare for removal from hass."""
self._unsub_dispatcher()

async def async_update(self, now=None):
"""Update function for updating API information."""
self._async_setup_api_requests()
await self._hass.async_add_executor_job(self.dsm.update, self._with_information)
async_dispatcher_send(self._hass, self.signal_sensor_update)
try:
await self._hass.async_add_executor_job(
self.dsm.update, self._with_information
)
async_dispatcher_send(self._hass, self.signal_sensor_update)
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 only wrap the line that can raise in the try... except block.

except (SynologyDSMLoginFailedException, SynologyDSMRequestException) as err:
Comment thread
Quentame marked this conversation as resolved.
_LOGGER.warning(
"async_update - connection error during update, fallback by reloading the entry"
)
_LOGGER.debug("async_update - exception: %s", str(err))
await self._hass.config_entries.async_reload(self._entry.entry_id)


class SynologyDSMEntity(Entity):
Expand Down
4 changes: 3 additions & 1 deletion homeassistant/components/synology_dsm/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,10 @@ async def async_step_user(self, user_input=None):
if errors:
return await self._show_setup_form(user_input, errors)

# Check if already configured
# unique_id should be serial for services purpose
await self.async_set_unique_id(serial, raise_on_progress=False)

# Check if already configured
self._abort_if_unique_id_configured()

config_data = {
Expand Down
9 changes: 9 additions & 0 deletions homeassistant/components/synology_dsm/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
UNDO_UPDATE_LISTENER = "undo_update_listener"

# Configuration
CONF_SERIAL = "serial"
CONF_VOLUMES = "volumes"

DEFAULT_USE_SSL = True
Expand All @@ -42,6 +43,14 @@
ENTITY_CLASS = "device_class"
ENTITY_ENABLE = "enable"

# Services
SERVICE_REBOOT = "reboot"
SERVICE_SHUTDOWN = "shutdown"
SERVICES = [
SERVICE_REBOOT,
SERVICE_SHUTDOWN,
]

# Entity keys should start with the API_KEY to fetch

# Binary sensors
Expand Down
15 changes: 15 additions & 0 deletions homeassistant/components/synology_dsm/services.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# synology-dsm service entries description.

reboot:
description: Reboot the NAS.
fields:
serial:
description: serial of the NAS to reboot; required when multiple NAS are configured.
example: 1NDVC86409

shutdown:
description: Shutdown the NAS.
fields:
serial:
description: serial of the NAS to shutdown; required when multiple NAS are configured.
example: 1NDVC86409
22 changes: 22 additions & 0 deletions tests/components/synology_dsm/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from homeassistant import data_entry_flow, setup
from homeassistant.components import ssdp
from homeassistant.components.synology_dsm import _async_setup_services
from homeassistant.components.synology_dsm.config_flow import CONF_OTP_CODE
from homeassistant.components.synology_dsm.const import (
CONF_VOLUMES,
Expand All @@ -20,6 +21,7 @@
DEFAULT_USE_SSL,
DEFAULT_VERIFY_SSL,
DOMAIN,
SERVICES,
)
from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER
from homeassistant.const import (
Expand Down Expand Up @@ -496,3 +498,23 @@ async def test_options_flow(hass: HomeAssistantType, service: MagicMock):
assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
assert config_entry.options[CONF_SCAN_INTERVAL] == 2
assert config_entry.options[CONF_TIMEOUT] == 30


async def test_services_registered(hass: HomeAssistantType):
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.

This test should not be part of the config flow tests. It should move to a test_init.py module.

"""Test if all services are registered."""
with patch(
"homeassistant.core.ServiceRegistry.async_register", return_value=Mock(True)
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.

Don't patch the core. Set up the integration and check that the services are registered in the service registry.

) as async_register:
await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_USER},
data={
CONF_HOST: HOST,
CONF_PORT: PORT,
CONF_SSL: USE_SSL,
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
},
)
await _async_setup_services(hass)
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.

assert async_register.call_count == len(SERVICES)