-
-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
Scan QR code to log in And Migrate Tuya integration to new sharing SDK #104767
Conversation
Hey there @tuya, @zlinoliver, @frenck, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate of #103007 ?
I'm confused this is coming from you guys like this, especially since there was feedback asked, provided and without any responses something totally different comes in.
We cannot go this route, it is fully breaking for users on upgrade, which is not acceptable. We've provided a possible path (and #103007 shows that path is possible), yet this is completely ignored.
../Frenck
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
The changes here support the Tuya Smart app and have upgraded the SDK version. After email communication, we thought we would resubmit a new PR. |
As far as I am aware there has not been follow up from Tuya on the email communication. As said, this PR is not acceptable, and will not be reviewed at this point in time. Please recheck the email communication and the above linked PR (from which parts of this code was copied), for the suggested route to take. Thanks! ../Frenck |
There may have been some misunderstandings in our previous communications. Allow me to clarify the situation. The code I submitted for this pull request supports both the Tuya Smart app and the Smart Life app(instead of only Smart Life app). This means that all the existing users can scan the QR code and log in through either app after upgrading. Based on my understanding, there should be no disruption for existing users, so we don't need to deal with the "deprecation period" concerns, and we have tested it locally. Based on the reasons mentioned above, please help review if any further modifications are needed for this pull request or if the changes still need to be updated in #103007? |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @shihuimiao 👍
I've created a tuya-sharing-sdk
branch that will be used as an intermediate branch to put this code in. From there on, I'll update the code with a few changes to our frontend and standards. The result of that, I will put up as a new PR against our dev branches.
../Frenck
066a0cc
to
9fa163c
Compare
Removed the |
* Scan QR code to log in And Migrate Tuya integration to new sharing SDK (#104767) * Remove non-opt-in/out reporting * Improve setup, fix unload * Cleanup token listner, remove logging of sensitive data * Collection of fixes after extensive testing * Tests happy user config flow path * Test unhappy paths * Add reauth * Fix translation key * Prettier manifest * Ruff format * Cleanup of const * Process review comments * Adjust update token handling --------- Co-authored-by: melo <411787243@qq.com>
Breaking change
You need to re-authenticate the Home Assistant Tuya integration with your Smart Life app or Tuya Smart app after the upgrade.
Proposed change
This PR moves the Tuya integration onto the new sharing SDK, which removes the need for a developer account and can directly log users in by scanning a QR code using the Smart Life app or Tuya Smart app.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: