Skip to content

Conversation

@carldebilly
Copy link
Contributor

@carldebilly carldebilly commented Dec 26, 2019

The parsing of the payload for configReportRsp was incorrect and leading to raw packet type in zigbee2mqtt as defined in this comment: Koenkk/zigbee2mqtt#2058 (comment)

✔️ Also include mfgr clusters for Stelpro

@carldebilly carldebilly force-pushed the patch-1 branch 2 times, most recently from 3b3697f to 131ab79 Compare December 26, 2019 01:18
@carldebilly
Copy link
Contributor Author

carldebilly commented Dec 26, 2019

Reference payload for the new test:
image

image

@carldebilly
Copy link
Contributor Author

@Koenkk I suspect the failing test is incorrect. Also I'm wordering if the parsing should be "flat" instead of "repetitive"...

@Koenkk
Copy link
Owner

Koenkk commented Dec 26, 2019

@carldebilly thanks for the PR, unfortunately I'm on holiday now, will dive into it after.

@Koenkk
Copy link
Owner

Koenkk commented Jan 1, 2020

The reason why this was here is:

image

However I'm not sure what it exactly means, need to check with some devices.

@carldebilly carldebilly changed the title Fix parsing of payload for configReportRsp [DO-NOT-MERGE] Fix parsing of payload for configReportRsp Jan 1, 2020
@carldebilly
Copy link
Contributor Author

You're right: it's breaking somewhere else.
I think the right fix is to switch the command parsing strategy to flat.

@carldebilly carldebilly changed the title [DO-NOT-MERGE] Fix parsing of payload for configReportRsp Fix parsing of payload for configReportRsp Jan 2, 2020
@carldebilly
Copy link
Contributor Author

@Koenkk Ok I fixed the problem.

Now the payload is "optional" but when present, it won't crash the parsing as before.

@Koenkk
Copy link
Owner

Koenkk commented Jan 6, 2020

The parsing should definitely repetitive, otherwise when multiple status records are returned only this first one will be parsed. I've updated the PR to do this, test are OK now.

@carldebilly let me know if this is ok for you then this can be merged.

@carldebilly
Copy link
Contributor Author

I'm confortable with those changes.

@Koenkk
Copy link
Owner

Koenkk commented Jan 6, 2020

Awesome, thanks!

@Koenkk Koenkk merged commit 527ec9c into Koenkk:master Jan 6, 2020
@carldebilly carldebilly deleted the patch-1 branch January 6, 2020 20:27
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.

2 participants