-
Notifications
You must be signed in to change notification settings - Fork 3.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
Fix errors thrown by PTM 216Z GP commands without payload #7127
Conversation
src/converters/fromZigbee.ts
Outdated
@@ -3173,6 +3173,10 @@ const converters1 = { | |||
if (hasAlreadyProcessedMessage(msg, model, msg.data.frameCounter, `${msg.device.ieeeAddr}_${commandID}`)) return; | |||
if (commandID === 224) return; | |||
|
|||
if (!msg.data.commandFrame.raw) { |
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.
Instead of doing this, can we change:
const ID = `${commandID}_${msg.data.commandFrame?.raw.slice(0, 1).join('_')}`; // added a ? here
And add it to the lookup?
'104_undefined': 'short_press_2_of_2'
Or alternatively, we change if (commandID === 224) return;
to if (commandID === 224 || commandID == 104) return;
, which is my preference as this only seems to be send once during pairing?
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.
I don't know these GP devices enough to assume only 104 would create this error (the list of GP "non-specific" commands is long, "Short Press 1 of 2", "Short Press 1 of 1"...). That's why I went with a "global" solution; any command that doesn't have a payload would throw otherwise. Adding them all to the lookup when most devices probably only output two or three of these seems "overkill"...
Page 11-12 https://www.silabs.com/documents/public/user-guides/ug392-using-sl-green-power-with-ezp.pdf
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.
If it's just this giving the error I propose to add it, it's much more user friendly.
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.
Add them all to the lookup you mean?
Would have to match with https://github.com/Koenkk/zigbee-herdsman-converters/blob/master/src/devices/enocean.ts to be exposed to the user.
I'm just guessing, but I figure some manufacturers might have implemented the pairing procedure differently based on hardware, so, the device chris tested sends 104, but another one might send 102 - probably the whole 0x60-0x68 range is involved?).
0x60 Press 1 of 1 N/A
0x61 Release 1 of 1 N/A
0x62 Press 1 of 2 N/A
0x63 Release 1 of 2 N/A
0x64 Press 2 of 2 N/A
0x65 Release 2 of 2 N/A
0x66 Short Press 1 of 1 N/A
0x67 Short Press 1 of 2 N/A
0x68 Short Press 2 of 2 N/A
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.
This code applies to just the PTM 216Z, so not sure what actions are send.
I've updated the PR
- It will not give the error anymore, but log a
PTM 216Z: missing command
when there is no such command - Added
short_press_2_of_2
What do you think?
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.
Sounds good. This way users can report commands that log errors, and support can be added as needed.
Those 3 GP converters (215-216) cover lots of actual devices from what I've seen.
Thanks! |
…-converters * 'master' of github.com:lucasteligioridis/zigbee-herdsman-converters: fix(detect): Detect `_TZE204_aagrxlbd` as TuYa TS0601_switch_4_gang_1 Koenkk#7133 fix: Fix battery percentage doubled for ROBB ROB_200-025-0 Koenkk/zigbee2mqtt#21607 feat: Enable LED indicator functionality for 41E10PBSWMZ-VW (Koenkk#7131) fix(detect): Detect `_TZ3000_4ugnzsli` as Luminea ZX-5232 (Koenkk#7136) fix: Fixes issue of passing function to TuYa lookup value converter (Koenkk#7135) feat: Add `short_press_2_of_2` action to EnOcean PTM 216Z (Koenkk#7127)
Credits to @dduransseau & @chris-1243 😉
Koenkk/zigbee2mqtt#21462 (comment)