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

Improve Xiaomi attribute report parsing #883

Merged
merged 3 commits into from
Aug 26, 2021

Conversation

puddly
Copy link
Contributor

@puddly puddly commented May 11, 2021

Xiaomi attribute reports apparently can contain off-by-one length errors in the other direction too (#881):

# Attribute ID
01ff

# Data type
42

# Length
44

# Value (really 69 bytes, expected 0x44 = 68)
03282305212e0008212e12092100106410006510006e20006f2000942002953
90a078c41963999eb0c4597390030683b983980bb873c9b2100009c20010a21
00000c280000

They usually look like:

# Attribute ID
01ff

# Data type
42

# Length
1f

# Value (really 30 bytes, expected 0x1f = 31)
0121b30b0328150421a8130521660106240100000000082105140a21a2f9

Or are the correct length.

In all of these cases the attribute report needs to be postprocessed, since the data type is incorrect and this leads to binary data being written to log files (home-assistant/core#24668):

[0xe4b2:1:0x0000] ZCL request 0x000a: [[<Attribute attrid=65281 value=<TypeValue type=CharacterString, value=d�>>]]
[0xe4b2:1:0x0000] Attribute report received: 65281=d�
0x8dbb:1:0x0b04 async_update
[0x8dbb:1:0x0000] ZCL request 0x000a: [[<Attribute attrid=65281 value=<TypeValue type=CharacterString, value=�(#�!�>>]]
[0x8dbb:1:0x0000] Attribute report received: 65281=�(#�!�

I've changed the data type from "Character String" (0x42) to "Octet String" (0x41) to fix this.

@coveralls
Copy link

coveralls commented May 11, 2021

Pull Request Test Coverage Report for Build 1167290720

  • 40 of 45 (88.89%) changed or added relevant lines in 1 file are covered.
  • 89 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.3%) to 82.858%

Changes Missing Coverage Covered Lines Changed/Added Lines %
zhaquirks/xiaomi/init.py 40 45 88.89%
Files with Coverage Reduction New Missed Lines %
zhaquirks/tuya/ts130f.py 4 77.5%
zhaquirks/tuya/init.py 85 72.43%
Totals Coverage Status
Change from base Build 1024783287: 0.3%
Covered Lines: 4041
Relevant Lines: 4877

💛 - Coveralls

@puddly puddly marked this pull request as draft May 12, 2021 22:09
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #883 (3a4fe1d) into dev (833c1b9) will increase coverage by 0.35%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #883      +/-   ##
==========================================
+ Coverage   82.50%   82.85%   +0.35%     
==========================================
  Files         185      194       +9     
  Lines        4589     4877     +288     
==========================================
+ Hits         3786     4041     +255     
- Misses        803      836      +33     
Impacted Files Coverage Δ
zhaquirks/xiaomi/__init__.py 83.94% <88.88%> (+2.06%) ⬆️
zhaquirks/xiaomi/mija/sensor_switch.py 58.69% <0.00%> (-12.28%) ⬇️
zhaquirks/const.py 100.00% <0.00%> (ø)
zhaquirks/lidl/cct.py 100.00% <0.00%> (ø)
zhaquirks/tuya/valve.py 95.52% <0.00%> (ø)
zhaquirks/tuya/ts0041.py 100.00% <0.00%> (ø)
zhaquirks/tuya/ts0601.py 100.00% <0.00%> (ø)
zhaquirks/legrand/dimmer.py 100.00% <0.00%> (ø)
zhaquirks/philips/rwlfirstgen.py 100.00% <0.00%> (ø)
zhaquirks/tuya/thermostat_88teujp.py 53.94% <0.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 833c1b9...3a4fe1d. Read the comment docs.

@puddly puddly force-pushed the puddly/fix-xiaomi-attribute-parsing branch from ab81a61 to 0d641e6 Compare July 13, 2021 17:26
@puddly puddly linked an issue Jul 13, 2021 that may be closed by this pull request
@dmulcahey
Copy link
Collaborator

@puddly hows this one goin?

@puddly puddly marked this pull request as ready for review August 25, 2021 16:05
@puddly
Copy link
Contributor Author

puddly commented Aug 25, 2021

Lemme look over it again.

@dmulcahey dmulcahey merged commit fe6b9ee into zigpy:dev Aug 26, 2021
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.

[BUG] Xiaomi lumi.relay.c2acn01 Command Report Attributes 0xFF01
4 participants