Add support for RE700X - #95
Conversation
The previous CI only runs the published package on pypi. This is problematic, as it does not help detecting issues in the unpublished/development code. This patch fixes the issue by adding a workflow that runs the current implementation of the package.
87b7563 to
f0e054e
Compare
1bdfa5a to
d043a6e
Compare
|
The latest version is tested with the current version of home assistant |
|
@nielstron Hi. Thank you for your work! |
| host=host, password=password, logger=logger, verify_ssl=verify_ssl, timeout=timeout) | ||
| if client.supports(): | ||
| return client | ||
| for client in [TplinkC5400XRouter, TPLinkVRClient, TPLinkEXClient, TPLinkMRClient, TPLinkDecoClient, |
There was a problem hiding this comment.
It would be good if TplinkRe700XRouter accepts username parameter without using it inside
| def supports(self) -> bool: | ||
| return len(self.password) >= 200 | ||
| try: | ||
| self.authorize() |
There was a problem hiding this comment.
it is better to leave return len(self.password) >= 200 - as self.authorize() may return exception if the password incorrect
There was a problem hiding this comment.
yeah about that... the problem is that c5400x is chosen as a router instead of the re700x. however, as soon as trying to authorize for c5400x, the application would crash.
I think the support logic is generally too fuzzy. it should check the device model to make sure not to use wrong clients.
There was a problem hiding this comment.
Check the model leads to limiting client using. For example, I made the first client for my model and then more and more users reported that their models are supported also - so one client supports more then 40 models. So it is better not to stick to a certain model - it is better to stick to unique features\routes for your router API. So several models with same API would be supported
| try: | ||
| data = response.json() | ||
| model = data["data"]["model"] | ||
| return model == "RE700X" |
There was a problem hiding this comment.
TP-LINK uses one router API for several models. Like TplinkRouter class supports several models and also Mercusys models. It checks the route that only that API has - https://github.com/AlexandrErohin/TP-Link-Archer-C6U/blob/main/tplinkrouterc6u/client/c6u.py#L146
Is there a way to determine that this code works not only for RE700X model? Maybe it has the route that others supported models dont have?
There was a problem hiding this comment.
I don't know, I don't have any other TP-LINK Devices to test. All other APIs crashed at some point though. I can check where is the difference to c5400x, which seems closest, but I am not sure if there is anything before authorization.
There was a problem hiding this comment.
Could you check please the your device API routes that are not in clients already for exemple? Im sure your client supports several models with same API - would be great to not limiting only to one model usage :)
| def set_wifi(self, wifi: Connection, enable: bool) -> None: | ||
| raise NotImplementedError() | ||
|
|
||
| def get_ipv4_status(self) -> IPv4Status: |
There was a problem hiding this comment.
Would be great of you could add get_vpn_status and set_vpn also (https://github.com/AlexandrErohin/TP-Link-Archer-C6U/blob/main/tplinkrouterc6u/client/c6u.py#L417) - Im planning to add to HomeAssistant integration VPN stats and control
There was a problem hiding this comment.
Is this a TP-Link cloud function? The extender by default does not seem to have a VPN option.
There was a problem hiding this comment.
Ah, it is extender, I forgot. Then this VPN question is not relevant)
|
Unfortunately I'll have to abandon this PR since I returned the device (it would break during video streams). I can keep this open in case other developers want to pick it up or close it, depending on your preferences. |
2 participants
This adds support for parts of the RE700X extender, tested in Access-Point mode.
This includes #96 to run the CI on the changes, and I recommend merging it before merging the PR
Note: The RE700X allows switching the leds on/off. this might come in useful for home assistant automations as well. Not sure if other devices support this too.