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 Tuya smoke detector TS0205 PST-YG500A #2991

Merged
merged 10 commits into from
Aug 27, 2024
Merged

Conversation

cdalexndr
Copy link
Contributor

@cdalexndr cdalexndr commented Feb 19, 2024

Proposed change

tuya-smoke-detector-smart-zigbee

image

Additional information

Removed useless Opening sensor.
Changed motion sensor to smoke sensor.

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.30%. Comparing base (daef394) to head (a136a0e).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2991      +/-   ##
==========================================
+ Coverage   88.27%   88.30%   +0.02%     
==========================================
  Files         302      303       +1     
  Lines        9447     9471      +24     
==========================================
+ Hits         8339     8363      +24     
  Misses       1108     1108              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TheJulianJES TheJulianJES added new quirk Adds support for a new device Tuya Request/PR regarding a Tuya device missing tests PR has no tests and might need them labels Feb 26, 2024
@TheJulianJES TheJulianJES changed the title Tuya Smoke detector TS0205 PST-YG500A Add Tuya smoke detector TS0205 PST-YG500A Feb 26, 2024
Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've added some suggestions.
Ideally, we should also add a test for the actual code (that translates the Tuya attribute to the IasZone attribute).

This test can mostly be copied IMO:

@pytest.mark.parametrize("quirk", (zhaquirks.xiaomi.aqara.smoke.LumiSensorSmokeAcn03,))
async def test_aqara_smoke_sensor_attribute_update(zigpy_device_from_quirk, quirk):
"""Test update_attribute on Aqara smoke sensor."""
device = zigpy_device_from_quirk(quirk)
opple_cluster = device.endpoints[1].opple_cluster
opple_listener = ClusterListener(opple_cluster)
ias_cluster = device.endpoints[1].ias_zone
ias_listener = ClusterListener(ias_cluster)
zone_status_id = IasZone.AttributeDefs.zone_status.id
# check that updating Xiaomi smoke attribute also updates zone status on the Ias Zone cluster
# turn on smoke alarm
opple_cluster.update_attribute(0x013A, 1)
assert len(opple_listener.attribute_updates) == 1
assert len(ias_listener.attribute_updates) == 1
assert ias_listener.attribute_updates[0][0] == zone_status_id
assert ias_listener.attribute_updates[0][1] == IasZone.ZoneStatus.Alarm_1
# turn off smoke alarm
opple_cluster.update_attribute(0x013A, 0)
assert len(opple_listener.attribute_updates) == 2
assert len(ias_listener.attribute_updates) == 2
assert ias_listener.attribute_updates[1][0] == zone_status_id
assert ias_listener.attribute_updates[1][1] == 0

The opple cluster stuff just needs to be changed to the Tuya stuff (and the quirk path needs to be changed).
If you need help with adding this test, let me know and I'll add it, as soon as I have some time.

zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0205.py Outdated Show resolved Hide resolved
@TheJulianJES TheJulianJES marked this pull request as draft March 18, 2024 03:00
@mr-bembel
Copy link

Hello,

Are there any news to support the product in ZHA soon? I bought 4 of them...

@cdalexndr cdalexndr marked this pull request as ready for review July 17, 2024 18:39
@Fr33L4nc3r
Copy link

@cdalexndr Seems the pipeline failed at pre-commit step. Since you fixed it in the past (fa98480), any chance you could have a look again ?
It seems like there ain't much to have this PR approved (despite the change request still appearing in red, while it seemed addressed with dcd03eb).

Looking forward to see this device recognized correctly and automatically in zha thanks to amazing community work like yours.

@TheJulianJES TheJulianJES added smash This PR is close to be merged soon and removed missing tests PR has no tests and might need them labels Aug 26, 2024
Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

Thanks, I think this should be fine.

@TheJulianJES TheJulianJES merged commit 8a864cf into zigpy:dev Aug 27, 2024
7 checks passed
@abmantis
Copy link
Contributor

abmantis commented Sep 2, 2024

@cdalexndr why is the TuyaSmokeDetectorCluster needed? The sensor worked fine previously, it would just show motion instead of smoke as the sensor type, but that is be fixed with just the addition of the TuyaIasZone cluster on line 57. So, why the extra TuyaSmokeDetectorCluster cluster?

@Fr33L4nc3r
Copy link

Just received a Product Safety Recall mail from Aliexpress about this smoke detector, just sharing in case you didn't know, I know what I need to replace ASAP :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new quirk Adds support for a new device smash This PR is close to be merged soon Tuya Request/PR regarding a Tuya device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants