Skip to content

Comments

Change to properly enable LTE sensors#98

Closed
teixeluis wants to merge 2 commits intoAlexandrErohin:mainfrom
teixeluis:feature-LTE-sensors
Closed

Change to properly enable LTE sensors#98
teixeluis wants to merge 2 commits intoAlexandrErohin:mainfrom
teixeluis:feature-LTE-sensors

Conversation

@teixeluis
Copy link

In this PR I am proposing a change to this lib that consists in reorganizing the data classes related to Status. For that effect I have defined a top level Status class which parents the RouterStatus (which contains the same attributes as the former Status class) along with IPv4Status, LTEStatus, and VPNStatus.

In the mr.py module get_status() will now call populate_lte_status() in order to populate the state attributes that are specific to the LTE enabled devices.

I will also open a PR on the hassio integration project (https://github.com/AlexandrErohin/home-assistant-tplink-router) that will require these changes.

…he proper modelling of

categories of attributes that may or may not occur depending on the device
@teixeluis
Copy link
Author

PR from the hassio integration: AlexandrErohin/home-assistant-tplink-router#242

status.devices = list(devices.values())
status.clients_total = status.wired_total + status.wifi_clients_total + status.guest_clients_total

status = self.populate_lte_status(status)
Copy link
Owner

Choose a reason for hiding this comment

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

Status data class should have only information general for all TP-Link models. It should not have LTE info as only some models have LTE. Most routers do not have this feature.

Copy link
Author

Choose a reason for hiding this comment

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

True, maybe move dataclasses specific to LTE elsewhere. Regarding this particular step I would challenge because if we are here (MR router) then it is an LTE enabled router.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants