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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for TP Link XDR3010 #55

Merged
merged 8 commits into from
Nov 18, 2024
Merged

Conversation

picone
Copy link
Contributor

@picone picone commented Nov 14, 2024

I've added support for the XDR3010 router. I tested in my network and everything work perfectly.

Hardware version: TL-XDR3010易展版 2.0
Software version: 1.0.18 Build 220711 Rel.56168n

@AlexandrErohin
Copy link
Owner

Thank you!
Could you implement all methods?

self._session = Session()

def supports(self) -> bool:
response = self._session.get(self.host)
Copy link
Owner

Choose a reason for hiding this comment

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

please dont forget to pass verify_ssl, timeout


def supports(self) -> bool:
response = self._session.get(self.host)
return response.text.index('TL-XDR3010') >= 0
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a way to check supports not by checking the model? Adding routers model to the method supports - greatly reduces the possibilities. As I was very surprised how many different types routers have same API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or... try to login in? If it works, that means it's supported.

Copy link
Owner

Choose a reason for hiding this comment

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

Could you write a test for supports method? Maybe check if model starts with string - TL-XDR ?

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 might be a bit tricky; I used Session().get() directly for the request. Now, none of the other routes have been tested with this method.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok. Could you just post here the output from print(response.text)? and change check to if model starts with string - TL-XDR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The response is like to this:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
	<title>TL-XDR3010易展版</title>
	<meta name="MobileOptimized" content="240" />
	<meta name="viewport" content="width=device-width, height=device-height, initial-scale=1.0, minimum-scale=0.5, maximum-scale=2.0, user-scalable=yes" />
	<link rel="shortcut Icon" href="../web-static/images/icon.ico" type="image/x-icon" />
	<link rel="stylesheet" href="../web-static/dynaform/class.css">
	<script type="text/javascript" src="../web-static/dynaform/class.js"></script>
	<script type="text/javascript" src="../web-static/dynaform/jtopo.js"></script>
</head>
<body>
	<div id="Error"></div>
	<div id="Confirm"></div>
	<div id="Con"></div>
	<div id="Help"></div>
	<div id="Cover"></div>
	<div id="Login"></div>
	<script type="text/javascript">
		var gBeInCNA = "NO";
		var proName="TL-XDR3010易展版";
		pageOnload();
    </script>
</body>
</html>

I'm not sure whether the routing for other XDR models is the same across the board. If it is, I should rename the class to TPLinkXDRClient, but I don't have other router models to test this on.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes - It would be great to rename TPLinkXDRClient. After your good suggestion I have changed the files and now the file structure changed. Could you update your MR with changes from main branch and create tplinkrouterc6u/client/xdr.py and put the client to this file?
Also would be great if you could add methods

return response.text.index('TL-XDR3010') >= 0

def authorize(self) -> None:
response = self._session.post(self.host, json={
Copy link
Owner

Choose a reason for hiding this comment

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

please dont forget to pass verify_ssl, timeout

def _encode_password(pwd: str) -> str:
return TPLinkXDR3010Client._security_encode(
pwd,
'RDpbLfCPsJZ7fiv',
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure that these two strings are constant? Maybe there is a route to get them? Like this one https://github.com/AlexandrErohin/TP-Link-Archer-C6U/blob/main/tplinkrouterc6u/client.py#L184

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, because I don't think I have two XDR3010 routes.
This key is hardcoded in the JS file instead of being in the API, and I think TP Link's engineers wouldn't dynamically generate the JS file.

Copy link
Owner

Choose a reason for hiding this comment

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

If it in the JS file - agree

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 [TPLinkXDR3010Client, TplinkC5400XRouter, TPLinkMRClient, TplinkC6V4Router, TPLinkDecoClient,
Copy link
Owner

Choose a reason for hiding this comment

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

Could you change it to

for client in [TplinkC5400XRouter, TPLinkMRClient, TplinkC6V4Router, TPLinkDecoClient, TplinkRouter, TPLinkXDR3010Client]:

and test for your router TplinkRouterProvider.get_client(...) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get it, but I'm not sure how to write unit tests for the supports() methods of other routes just yet.

Copy link
Owner

Choose a reason for hiding this comment

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

So does get_client works for you when XDR3010 in the end for provider priority?

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, it works

@AlexandrErohin AlexandrErohin merged commit 7312b23 into AlexandrErohin:main Nov 18, 2024
@AlexandrErohin
Copy link
Owner

Thank you!

@picone
Copy link
Contributor Author

picone commented Nov 19, 2024

@picone Hi Could you add

I checked my router settings and couldn't find any information about VPN functions.

@AlexandrErohin
Copy link
Owner

Ok. Thats mean your router doesnt support VPN. Thank you

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