Adds support for EX511 router #54
Conversation
|
Hi @krlosrenan Tests are needed for supporting the router, as there is always would need to fix something in code and without tests it would be very difficult to do |
tplinkrouterc6u/client.py
Outdated
| class TplinkRouterProvider: | ||
| @staticmethod | ||
| def get_client(host: str, password: str, username: str = 'admin', logger: Logger = None, | ||
| verify_ssl: bool = True, timeout: int = 30) -> AbstractRouter: | ||
| for client in [TplinkC5400XRouter, TPLinkMRClient, TplinkC6V4Router, TPLinkDecoClient, TplinkRouter]: | ||
| for client in [TPLinkEXClient, TplinkC5400XRouter, TPLinkMRClient, TplinkC6V4Router, TPLinkDecoClient, TplinkRouter]: |
There was a problem hiding this comment.
Could you change it to
for client in [TplinkC5400XRouter, TPLinkMRClient, TPLinkEXClient, TplinkC6V4Router, TPLinkDecoClient, TplinkRouter]:
and test for your router TplinkRouterProvider.get_client(...) as there is a possibility that requests for TPLinkEXClient may take the main role before TPLinkMRClient. As we dont have now MR router - so it is better to test it in reverse order
There was a problem hiding this comment.
About creating test_client_ex.py, it will take me a few days, but I can create it too.
About the order, if we change the order, it will always assume MR, since the MR test consists of a query to /getParm, and EX511 also has this query, so MR will return true.
Another possibility would be to make the EX test more robust.
What do you think of this solution:
def supports(self) -> bool:
try:
supported_routers = ['EX511', 'EX220']
url = self._get_url('js/oid_str.js')
(code, response) = self._request(url)
assert code == 200
pattern = r'\s*var\s+INCLUDE_BOOT_AGINETCONFIG_MODEL\s*=\s*"([^"]*)"'
match = search(pattern, response)
assert match and match.group(1) in supported_routers
self._req_rsa_key()
return True
except AssertionError:
return False
There was a problem hiding this comment.
Would be great if you could add tests test_client_ex.py. Thank you!
In your client method supports calls self._get_url('cgi/getGDPRParm'). In MR client method supports calls self._get_url('cgi/getParm'), So your router can response on self._get_url('cgi/getGDPRParm') and self._get_url('cgi/getParm') with same response?
Adding routers model to the method supports - greatly reduces the possibilities. As I was very surprised how many different types routers have same API
There was a problem hiding this comment.
1 - I added the tests (test_client_ex.py)
2 - Yes, the router I tested on responds to "cgi/getGDPRParm" and "cgi/getParm" in the same way.
3 - I believe this new model will support several other routers
Looking at the oid_str.js file, I noticed that there are possibly several other routers that may be supported.
Note the lines that start with INCLUDE_SPEC_
I will test it on a friend's EX220 soon.
I am attaching the file.
oid_str.txt
There was a problem hiding this comment.
1 - Thank you for the tests
2 - Thats bad. Because this may break the MR support. I will try to find MR series router - to check response to "cgi/getGDPRParm". Hope it doesnt have
There was a problem hiding this comment.
I believe this won't be a problem because I adjusted the supports method so that it only responds positively for the EX511 and EX220 models.
Before, the verification was only by requesting getGDPRParm, but now, in addition to that, the router needs to be on the list of supported models.
However, it would be great to test on a MR, because if there is no conflict, we could simplify the code so that it could support models that we haven't tested, but that use the same API.
If you need to change anything else, let me know.
|
Could you please post the hardware version for EX220 - when you test it? |
|
I have tested it with MR router and it works - I have changed support method. Thank you for help and for your work for adding support for your router |
Everything is working. I just did some tests and no errors occurred. |
|
@krlosrenan Hi |
Hello, in a few minutes I will make the pull request. |
|
This implementation works for NX510v as well.
|
|
@lebdim Thank you for the info! |
This pull request adds support for the EX511 router, I tested it with the Home Assistant integration and it seems to be working.
Here I'm using the following version:
Firmware Version:0.7.0 3.0.0 v607e.0 Build 240930 Rel.11206n
Hardware Version:EX511 v2.0 00000000
If you want me to change anything, please let me know.