Add support for additional data point values in Tuya payloads

[gmbnomis]

Although most TuYa devices send only one data point value in a data report/command, it is possible to include values for multiple data points in such a payload.

Extend the definitions of those receiving commands to decode all values present. Store them in the dpValues array. Additionally, remove the fn field: It is just the high byte of the length of the data buffer and isn't used by converters.

Change the definitions of the sending commands to use an array of DP values as well.

NOTE:

• This is a breaking change to the tuya library functions in zigbee-herdsman-converters.
• This breaks all existing Tuya from Zigbee converters using dataResponse, dataReport, or activeStatusReport. They need to be adapted to use the first entry of dpValues

Closes #478

[Koenkk]

@hacker-cb could you review this?

[hacker-cb]

@gmbnomis where you found about multiply dp in one command?

There is no such information in official tuya document.

[gmbnomis]

@gmbnomis where you found about multiply dp in one command?

Please have a look into the corresponding issue #478. I have a water leakage sensor that sends such a payload.

You are right, the Zigbee part does not mention it explicitly, but if you look at the serial protocol it is possible to send multiple DP values back-to-back. ("Sending multiple commands are supported in obj type data points").

I reckon that in my case, these back-to-back data point values are just put into the Zcl "DP data"

[hacker-cb]

@gmbnomis seems you are right, I can accept version that MCU DPs data is transfered transparently via Zigbee.

I think right way is completely replace single-dp behaviour with multi-dp because may be other devices also use multi-dp format and currently we just ignore this.

Of cause, need to refactor converters in this case. I think it will be better to use class-based dp everywhere.

After brief lookup, for incoming dp:

1. Just need to change fromZigbee.js and lidl.js.
2. ...?

For outgoing dp:

1. Add sendMultiDataPoints() method to lib/tuya.js, based on sendDataPoint().
2. Call it from sendDataPoint() for backward-compatibility.
3. Completely replace sendDataPoint[Value|sendDataPointBool|sendDataPointEnum|...]() with class-based dp.
4. ...?

So, @Koenkk how do you think?

[gmbnomis]

Of cause, need to refactor converters in this case. I think it will be better to use class-based dp everywhere.

@hacker-cb Makes sense. Essentially, we would convert these by changing msg.data.... into msg.data.dpValues[0]...?

After brief lookup, for incoming dp:

1. Just need to change fromZigbee.js and lidl.js.

There is also tuya_thermostat_weekly_schedule in legacy.js

2. ...?

I would propose:

2. Get rid of the fnfield. According to the documentation, it is just the high byte of the length and it's used nowhere but in tuya_data_point_dump.
3. Adapt tuya_data_point_dump to be able to dump multiple data points. Adapt read_tuya_dump.py (I have already done this)
4. Adapt the documentation https://www.zigbee2mqtt.io/advanced/support-new-devices/02_support_new_tuya_devices.html

[Koenkk]

Sounds good to me, I think such change has quite a risk so once ready I will merge it after the next release on 1 January.

[gmbnomis]

Great!

@hacker-cb As you have looked into the outgoing part in more detail, should we split up the work? (I can continue to work on the incoming part, which will mostly happen in other repositories I suppose)

[hacker-cb]

@gmbnomis I think at least this things of the outgoing part should be merged together with incoming to avoid double refactoring of the zigbee-herdsman project:

• Add sendMultiDataPoints() method to lib/tuya.js, based on sendDataPoint().
• Call it from sendDataPoint() for backward-compatibility.

@Koenkk this PR should be merged very quickly to avoid situations like this.

[Koenkk]

Does this pr also require any changes to zigbee-herdman-converters?

[gmbnomis]

Does this pr also require any changes to zigbee-herdman-converters?

@Koenkk Yes, this PR totally breaks all "from Zigbee" Tuya converters. The necessary adaptations are at https://github.com/Koenkk/zigbee-herdsman-converters/pull/3572.

As you said before, this change is quite risky (although I tried to be as conservative as possible), as I can't test any of those converters. So you might want to merge it after releasing.

Additional changes will be necessary for the documentation on how to add TuYa devices. (I plan to provide these as well, but haven't started working on those yet)

This is the input side. Then there is the output (sending) side of things. @hacker-cb I am not sure if I understand your statement on the output side above. In your view, should the changes to the sending methods become part of this PR, or should this PR be merged first to get the output changes on top as quickly as possible?

[hacker-cb]

@gmbnomis

I mean that all zigbee-herdsman cluster.ts modifications of the output parts should be together with input modifications in one PR. zigbee-herdsman-converters sendDataPoint() should be modified to match new cluster and should be in one PR with inputs.

[Koenkk]

@gmbnomis

So you might want to merge it after releasing.

I will set a reminder for 2 January, will ping you to update the zigbee-herdsman-converters pr then.

[gmbnomis]

I have done the input and output side now, see also https://github.com/Koenkk/zigbee-herdsman-converters/pull/3572.

For outgoing dp:

1. Add sendMultiDataPoints() method to lib/tuya.js, based on sendDataPoint().

done

2. Call it from sendDataPoint() for backward-compatibility.

done

3. Completely replace sendDataPoint[Value|sendDataPointBool|sendDataPointEnum|...]() with class-based dp.

I have not done this: Looking at the existing "to Zigbee" converters, the main use case will still be to send a single DP. I think we should not make this main use case more complicated and keep the "convenience" functions to send a single DP value.

@hacker-cb Could you have a look? Additionally, if you have a TuYa device that accepts outgoing DP commands, could you test this with a real device? (I verified that the outgoing payloads are the same, but I can't verify with a real device)

[hacker-cb]

I have not done this: Looking at the existing "to Zigbee" converters, the main use case will still be to send a single DP. I think we should not make this main use case more complicated and keep the "convenience" functions to send a single DP value.

I agree with you to keep convenience functions. Outgoing part looks nice. For the incoming part - I left comment in review.

Additionally, if you have a TuYa device that accepts outgoing DP commands, could you test this with a real device? (I verified that the outgoing payloads are the same, but I can't verify with a real device)

I can't test on real Tuya devices because I'm not in office till end of the NY holidays. May be @Koenkk can ask somebody else...

[Koenkk]

@hacker-cb no rush, will merge this after new years eve.

[hacker-cb]

Also there is one more thing left to do:

I wrote comment in review of the Processing all dp data in incoming message with for/foreach loop.

So, @gmbnomis - what about that?

[gmbnomis]

@hacker-cb Sorry, I can't find a comment at https://github.com/Koenkk/zigbee-herdsman-converters/pull/3572?

[hacker-cb]

@gmbnomis

[gmbnomis]

@hacker-cb Ah, that looks like a "Pending" comment that is only visible to you. You will need to submit the review to make it visible.

[hacker-cb]

@gmbnomis, sorry, submitted, try now :)

[Koenkk]

@gmbnomis I plan to merge everything tomorrow:

• are you ready?
• Could you link all prs I need to merge? (to make sure I don't miss any)

[gmbnomis]

• are you ready?

Yes, all PRs are final from my point of view

• Could you link all prs I need to merge? (to make sure I don't miss any)

Overall, there are three PRs:

• This one: https://github.com/Koenkk/zigbee-herdsman/pull/483/
• https://github.com/Koenkk/zigbee-herdsman-converters/pull/3572
• https://github.com/Koenkk/zigbee2mqtt.io/pull/1095/

(I will post the changes for the actual device needing this feature after they are merged)

[Koenkk]

@gmbnomis merged all, big thanks for working on this!