Add support for Tuya fault sensors
Proposed change
Added support for Tuya fault sensors, indicating any active fault sensors on a Tuya product.
I noticed that my MeacoDry Arete Two 12L exposed a field called fault, with a type of Bitmap, and the following spec:
{
"label": [
"tankfull",
"defrost",
"E1",
"E2",
"L2",
"L3",
"L4",
"wet"
]
}
I then found out through Tuya documentation that this is a general field, so I decided to try and implement detection and parsing of this field.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Ruff (
ruff format homeassistant tests) - [ ] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [ ] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Hey there @tuya, @zlinoliver, @frenck, mind taking a look at this pull request as it has been labeled with an integration (tuya) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of tuya can trigger bot actions by commenting:
-
@home-assistant closeCloses the pull request. -
@home-assistant rename Awesome new titleRenames the pull request. -
@home-assistant reopenReopen the pull request. -
@home-assistant unassign tuyaRemoves the current integration label and assignees on the pull request, add the integration domain after the command. -
@home-assistant add-label needs-more-informationAdd a label (needs-more-information, problem in dependency, problem in custom component) to the pull request. -
@home-assistant remove-label needs-more-informationRemove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.
I'm aware that some code changes might be needed, for example extracting the new logic in is_on(self) to separate methods. I just wanted to get some feedback on my code before I did the final prettifying.
I've done some cleanup of the code and extracted the new logic to separate methods. I hope maybe @frenck will get some time to look at it at some point - I'm very open to feedback.
@frenck any chance you can take a look at my PR?
@frenck I fixed the ruff and mypy errors encountered after you merged the latest dev branch into my PR, so it should be fine to review now. I had some problems getting the devcontainer to work when I developed this, which is why I missed those checks - got it working now though.
@frenck, any chance you have time to look at my PR, or can help me find someone else who can? I know you are busy, but it's quite a small PR 🤞
@frenck pinging you again. It's been almost 5 months, so I hope you will get some time to look at this.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
@frenck it looks like several PR's have been opened since I opened mine back in September, trying to solve the same issue: https://github.com/home-assistant/core/pull/129437 https://github.com/home-assistant/core/pull/137771
We should probably figure out which one should be merged, and close the others as they solve the same problem.
@epenet and @jbouwh I can see that you have been reviewing one of the other PR's but it looks like the author hasn't gotten back yet - maybe you can help me get this reviewed and merged, so we can get this feature added?
I think this PR looks like the most promising of the three alternatives.
@MrAdam Could you please add some tests though? Note that we really want tests to set up the integration and check the resulting state in the state machine unlike what the tests in the alternative PRs do.
Please feel free to reach out to me on Discord if you need some help with implementing tests.
I think this PR looks like the most promising of the three alternatives.
@MrAdam Could you please add some tests though? Note that we really want tests to set up the integration and check the resulting state in the state machine unlike what the tests in the alternative PRs do.
Please feel free to reach out to me on Discord if you need some help with implementing tests.
I'll give it a try. Do you have a reference to an integration that does it in the best possible way, just so I can see how it should be structured?
I leave some review comments. Also, I think we need to expand this code to whole bitmap types not for faults only
I resolve some own coments on this PR https://github.com/MrAdam/core/pull/1
@MrAdam, take a look, for pr to this branch, maybe it would be simple to merge it https://github.com/MrAdam/core/pull/1
About bitmaps - padding is needed only when converted to binary string representation, bitwise shift with & works well with "all" numbers. Tuya API returning dec number, so loss is not possible. Also, if shift used - array length no need any more.
It will probably take some time before I can look at this again (I have started looking into tests), as I just started a new job this week, and am a bit swamped.
@emontnemery I've added some tests, and tried to work with the state machine - I hope you will take a look and see if it's done correctly 👍
I can see that coverage is failing, due to me not having done any negative tests yet - I'll get that fixed soon.
Replaced by #148485 If you want to add E1 / E2 / L2 / L3 / L4 this can be done in a follow-up PR