Skip to content
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.

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

Already on GitHub? Sign in to your account

Add Synology SRM sensor to get information from router #31523

Closed
wants to merge 7 commits into from
Closed
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
2 changes: 1 addition & 1 deletion homeassistant/components/synology_srm/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"domain": "synology_srm",
"name": "Synology SRM",
"documentation": "https://www.home-assistant.io/integrations/synology_srm",
"requirements": ["synology-srm==0.0.7"],
"requirements": ["synology-srm==0.1.4"],
"dependencies": [],
"codeowners": ["@aerialls"]
}
141 changes: 141 additions & 0 deletions homeassistant/components/synology_srm/sensor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
"""Sensor for Synology SRM routers.

For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/device_tracker.synology_srm/
"""
import synology_srm
import voluptuous as vol

from homeassistant.const import (
CONF_HOST,
CONF_MONITORED_CONDITIONS,
CONF_NAME,
CONF_PASSWORD,
CONF_PORT,
CONF_SSL,
CONF_USERNAME,
CONF_VERIFY_SSL,
)
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity

CONF_TRAFFIC_INTERVAL = "traffic_interval"

DEFAULT_NAME = "synology_srm"
DEFAULT_USERNAME = "admin"
DEFAULT_PORT = 8001
DEFAULT_SSL = True
DEFAULT_VERIFY_SSL = False

EXTERNALIP_CONDITION = "core.ddns_extip"
TRAFFIC_CONDITION = "core.ngfw_traffic"

POSSIBLE_MONITORED_CONDITIONS = {
"base.encryption",
"base.info",
EXTERNALIP_CONDITION,
"core.ddns_record",
"core.system_utilization",
"core.network_nsm_device",
TRAFFIC_CONDITION,
"mesh.network_wanstatus",
"mesh.network_wifidevice",
"mesh.system_info",
}

DEFAULT_MONITORED_CONDITIONS = [
EXTERNALIP_CONDITION,
]

POSSIBLE_TRAFFIC_INTERVAL = {"live", "day", "week", "month"}

DEFAULT_TRAFFIC_INTERVAL = [
"live",
]

PLATFORM_SCHEMA = cv.PLATFORM_SCHEMA.extend(
Copy link
Member

Choose a reason for hiding this comment

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

We no longer accept additions or changes to the yaml config of integrations. See: https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md

This integration needs to be refactored to use a config flow and config entries.

https://developers.home-assistant.io/docs/config_entries_config_flow_handler

Copy link
Member

Choose a reason for hiding this comment

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

I agree, should move this platform based copmponent to an integration based.

Will avoid double config for one device.

{
vol.Required(CONF_NAME, default=DEFAULT_NAME): cv.string,
vol.Required(CONF_HOST): cv.string,
vol.Required(CONF_USERNAME, default=DEFAULT_USERNAME): cv.string,
vol.Required(CONF_PASSWORD): cv.string,
vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port,
vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,
vol.Optional(CONF_TRAFFIC_INTERVAL, default=DEFAULT_TRAFFIC_INTERVAL): vol.All(
cv.ensure_list, [vol.In(POSSIBLE_TRAFFIC_INTERVAL)]
),
vol.Optional(
CONF_MONITORED_CONDITIONS, default=DEFAULT_MONITORED_CONDITIONS
): vol.All(cv.ensure_list, [vol.In(POSSIBLE_MONITORED_CONDITIONS)]),
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid monitored_contitions from https://github.com/home-assistant/architecture/blob/master/adr/0003-monitor-condition-and-data-selectors.md

Is your different conditions are making a specific fetch ?

Or maybe you can use entity_registry_enabled_default https://developers.home-assistant.io/docs/core/entity#advanced-properties

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes ... they make separate requests ... so I think this should remain.

}
)


def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up the Synology SRM Sensor."""
add_devices([SynologySrm(config)])
Copy link
Member

Choose a reason for hiding this comment

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

This does not allow to automatically add a new SynologySrm sensor entity automatically when a new device connects to the router.

See #30334 freebox/device_tracker or #35412 synology_srm/device_tracker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a device tracker; this is to obtain information from the router; the existing SRM integration can be used for device tracking.

Copy link
Member

Choose a reason for hiding this comment

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

I know this is not a device_tracker, but the behavior is the same.
You can actually see it in icloud/sensor.

But since when you have a new device connected to your router, you have to create a sensor, this should work like this.



class SynologySrm(Entity):
"""This class gets information from a SRM router."""

def __init__(self, config):
"""Initialize."""
self.config = config

self.client = synology_srm.Client(
host=config[CONF_HOST],
port=config[CONF_PORT],
username=config[CONF_USERNAME],
password=config[CONF_PASSWORD],
https=config[CONF_SSL],
)

if not config[CONF_VERIFY_SSL]:
self.client.http.disable_https_verify()

self._state = None

self._attribs = None

@property
def name(self):
"""Sensors name."""
return self.config.get(CONF_NAME)

@property
def icon(self):
"""Return the icon."""
return "mdi:router-wireless"

@property
def state(self):
"""Return the state."""
return self._state

@property
def device_state_attributes(self):
"""Attributes."""
return self._attribs

def update(self):
"""Check the router for various information."""
monitored_conditions = self.config.get(CONF_MONITORED_CONDITIONS)
attribs = {}
for condition in monitored_conditions:
parts = condition.split(".")
conditionname = condition.replace(".", "_")
if condition == TRAFFIC_CONDITION:
for interval in self.config.get(CONF_TRAFFIC_INTERVAL):
attrib = self.client.core.ngfw_traffic(interval=interval)
attribs[f"{conditionname}_{interval}"] = attrib
else:
attrib = getattr(self.client, parts[0])
attrib = getattr(attrib, parts[1])()
attribs[conditionname] = attrib
if condition == EXTERNALIP_CONDITION:
first_wan = next(iter(attrib), None)
self._state = first_wan and first_wan["ip"]

self._attribs = attribs
2 changes: 1 addition & 1 deletion requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1933,7 +1933,7 @@ surepy==0.1.10
swisshydrodata==0.0.3

# homeassistant.components.synology_srm
synology-srm==0.0.7
synology-srm==0.1.4

# homeassistant.components.tahoma
tahoma-api==0.0.16
Expand Down