Added fritzbox_netmonitor.py #5469
Conversation
|
@PetePriority, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @rmkraus to be potential reviewers. |
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| SENSOR_TYPES = { | ||
| 'is_linked': ['Link active', '', 'mdi:web'], |
There was a problem hiding this comment.
This is a true/false sensor type and thus should be moved to a binary_sensor. There are probably a bunch more too.
|
please use also |
| @@ -0,0 +1,146 @@ | |||
| """ | |||
| Support for monitoring the local system. | |||
| Support for monitoring the local system. | ||
|
|
||
| For more details about this platform, please refer to the documentation at | ||
| https://home-assistant.io/components/sensor.systemmonitor/ |
There was a problem hiding this comment.
Should be https://home-assistant.io/components/sensor.fritzbox_netmonitor/
| CONF_DEFAULT_IP = '169.254.1.1' # This IP is valid for all FRITZ!Box routers. | ||
|
|
||
| PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
| vol.Optional(CONF_HOST, default=CONF_DEFAULT_IP): cv.string, |
There was a problem hiding this comment.
The sensors are missing. Add something like the sample below to check the configuration entry.
vol.Optional(CONF_RESOURCES, default=[]):
vol.All(cv.ensure_list, [vol.In(SENSOR_TYPES)]),
| devices = [] | ||
| for resource in config[CONF_RESOURCES]: | ||
| if 'arg' not in resource: | ||
| resource['arg'] = '' |
There was a problem hiding this comment.
I would like to suggest to simplify the configuration. You are not using arg and instead of
- type: is_linked
- type: is_connected
....just use
- is_linked
- is_connected
....| add_devices(devices) | ||
|
|
||
| entity_ids = (dev.entity_id for dev in devices) | ||
| group.Group.create_group(hass, GROUP_NAME_ALL_DEVICES, entity_ids, False) |
There was a problem hiding this comment.
I think that we should let the user do the grouping. There would be a group for one sensor if only one is selected.
From my point of view, here the right way to go would be to expose the values as attributes. Then users can use a template sensor to grab the values and convert them as bytes and bits produce very quick very large numbers.
| """Return the unit of measurement of this entity, if any.""" | ||
| return self._unit_of_measurement | ||
|
|
||
| def update(self): |
There was a problem hiding this comment.
Here you should update your data object. You are communicating with your router, you want to throttle the data retrieval, and catch exceptions caused by connection issues.
It looks like that you use the systemmonitor sensor platform as a start. This platform is not communication with a third party.
|
Hi @PetePriority, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
|
@PetePriority any updates on this PR? |
|
@andrey-git Sorry, I've been quite busy the last couple of weeks. I will implement the requested changes on the weekend. |
|
Hi @PetePriority, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
|
Top 🐬 |
Description:
Adds a system-monitor sensor that provides various network related statistics of AVM Fritz!Box routers.
Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#1839
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.