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 tado X support #86

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Conversation

karlbeecken
Copy link

This adds basic tado X support.

The module auto-detects if it connects to a tado X account and uses other API endpoints if applicable.

Note: the API scheme of tado X differs severely sometimes, so with this patch old applications that want to use tado X may be adapted to also reflect these changes, some endpoints do not work the same way.

@Moritz-Schmidt
Copy link

Hey @wmalgadey , @palazzem , I would love to help maintain this project. I could help with the code quality improvements and typing you've started with in other PRs, I'd also like to improve the unit tests. I'm not really a pro, but I'm quite experienced with Python, especially with typing. Let me know if any help would be appreciated!

@wmalgadey
Copy link
Owner

From a professional point of view, I would recommend solving the problem with a new abstract base class ‘BaseRequest’ and two overloads ‘Tado’ and ‘TadoX’. But I know that this is a more complex solution. Should we create an issue for this new (maybe better) solution? Or am I being too picky?

PyTado/interface.py Outdated Show resolved Hide resolved
Co-authored-by: Wolfgang Malgadey <wolfgang@malgadey.de>
@wmalgadey
Copy link
Owner

Hey @wmalgadey , @palazzem , I would love to help maintain this project. I could help with the code quality improvements and typing you've started with in other PRs, I'd also like to improve the unit tests. I'm not really a pro, but I'm quite experienced with Python, especially with typing. Let me know if any help would be appreciated!

sure, every help is welcome 👍

@Moritz-Schmidt
Copy link

From a professional point of view, I would recommend solving the problem with a new abstract base class ‘BaseRequest’ and two overloads ‘Tado’ and ‘TadoX’. But I know that this is a more complex solution. Should we create an issue for this new (maybe better) solution? Or am I being too picky?

Thanks for the fast response! Your suggestion sound good, I had a similar idea for the Zone class too. I'd be happy to implement the BaseRequest ABC and a Zone ABC in other PRs, but I would need some time with that. To get the Home Assistant Integration working with TadoX soon I'd appreciate it if you could merge this PR as is and create a new release.

@wmalgadey
Copy link
Owner

wmalgadey commented Nov 1, 2024

@Moritz-Schmidt take a look at #84, @albertomontesg already did quite a good effort in refactoring. I didn't merge it till now, because nobody responded, and I couldn't test the changes.

@Moritz-Schmidt
Copy link

@wmalgadey yes, I've already seen it and will probably continue the Work done there!

@wmalgadey
Copy link
Owner

When we merge this PR, there will be some conflicts in #84. Should we still merge?

@Moritz-Schmidt
Copy link

Yes I think we should merge, I can resolve the conflicts with #84 when I'm working on it. If we get this shipped in time, maybe we'll still get into HA 2024.11

@wmalgadey wmalgadey merged commit 0a2ed93 into wmalgadey:master Nov 1, 2024
5 checks passed
@karlbeecken
Copy link
Author

@wmalgadey Thank you very much for the fast merge!

@karlbeecken
Copy link
Author

Ah @wmalgadey, could you also create a new release and push it to PyPI?

@wmalgadey
Copy link
Owner

done!

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

Successfully merging this pull request may close these issues.

3 participants