[tuya] Add support for gateway and sub devices

[presidentio]

This PR resolves #483 and #493

First of all, @J-N-K thank you for a great binding. It works well, and UX is great. Everything is discovered automatically.
Most of my Tuya things are ZigBee based and communicate through the gateway. In this PR I added support for them

End-user changes:

• New device type gateway. It doesn't have any channel, and the configuration looks the same as in the regular device, with the same behavior of IP discovery. It can act only as a bridge
• New device type subDevice. Its configuration has few parameters and requires a bridge. It works with the channels the same way as a regular device
• Discovery service can discover new devices and they will be ready to go after adding them from the inbox

I had to refactor/extract code from existing classes to avoid code duplication. I tried to do not change the existing logic.

I've tested it with read-only devices: water leak sensor, motion sensor, door/window opening sensor

Please, let me know if you have any concerns or suggestions, and I will try to resolve them

[presidentio]

@J-N-K, will you have time to take a look?

[J-N-K]

I hope I'll find time at the weekend.

[J-N-K]

Thanks again and sorry for letting you wait.

I have some general questions:

• Why do we need the TuyaDeviceManager?
• As far as I can see the TuyaGatewayHandler does nothing, except distributing the device status messages to the child handlers and passing the disconnected messages. Is that really needed or could we integrate that in the TuyaDevice and thing handler itself?

[J-N-K]

What is the benefit over using the Storage directly?

[presidentio]

This change is not mandatory. It's an attempt to extract a clear interface for schema storage. Before that, we operated on storage directly and serialised/deserialised objects while interacting with it. Now, we see what type of objects can be stored here and we don't need to bridge Gson object along with the Storage.

[presidentio]

Thanks for the review!
As far as I can see the TuyaGatewayHandler does nothing, except distributing the device status messages to the child handlers and passing the disconnected messages. Is that really needed or could we integrate that in the TuyaDevice and thing handler itself? -> Unfortunately no. I tried what you suggested, but most of my devices including the gateway have a limit for the number of connections(1-2 at maximum). Due to that, I can't create more than one TuyaDevice for different sub-devices. So, I decided that we need to represent gateway as a bridge in the Openhab and it is responsible only for the communication. TuyaSubDeviceHandler is responsible for channels.
Why do we need the TuyaDeviceManager? -> TuyaGatewayHandler has the same flow of creating and managing TuyaDevice, so I decided to extract everything about TuyaDevice management into a separate class to do not duplicate the code

[presidentio]

@J-N-K Please, review my answers

[J-N-K]

Thanks, I'll try to have a look this weekend.

[J-N-K]

Sorry again for letting you wait so long for a good review. I have thought a lot about the design and there are several things that can't (or shouldn't) be done the way it is implemented now. One thing is that ThingHandlers should never deal with the thing registry.

Instead of the bridge/thing design I would like to propose a different approach that also solves the issue with the multiple connections.

We create a service called TuyaDeviceManager which is passed from the ThingHandlerFactory to the things handler which then requests a TuyaDevice from there (instead of calling new TuyaDevice):

    tuyaDeviceManager.getTuyaDevice(....);

The manager checks if the already have a TuyaDevice for that gateway id and then returns the already existing one, otherwise it creates a new one. When a thing handler is disposed, it unregisters itself from the factory (we can also call it the TuyaDeviceManager).

The gateway id is the same as the device id for ordinary devices, and the id of the gateway for sub devices. It can be set during discovery (as a new config parameter). If it is not set, it is considered to be the same as the device id, that makes it backward compatible.

The TuyaDiscoveryService can then just check if it is a gateway device and call the new listSubDevices method and add the sub devices to the inbox (with all required parameters).

This should all be possible with about 2050 lines of code. The only addition needed then is in TuyaDevice, which needs to be enhanced to keep a map of listeners (instead of a single one), which is managed by the TuyaDeviceManager (via a add/remove methods). The key is the device. Depending on the device id the TuyaDevice can then select the correct listener for the responses (or notify all listener if the device goes offline).

WDYT?

[presidentio]

@J-N-K Thanks for the deep review. From your answer, I see two concerns:

1. ThingHandler shouldn't deal with ThingRegistry. Perhaps you are right. For me, it was acceptable as ThingHandler in fact doesn't call ThingRegistry, instead it passes it to DiscoveryService which is responsible for managing things. Your solution of registering the bridge along with sub-devices will work, but we will lose sub-devices discovery for a created bridge. I'm fine changing this aspect
2. Introducing bridge. Your solution will work, but I envision several types of issues: concurrency issues while managing TuyaDevice and duplicated configuration across multiple devices(gateway IP will be specified in each sub-device). From my experience with Openhab and documentation, it's recommended to represent a physical device with a bridge if it's responsible for communication with other devices. So, I'd like to keep the existing implementation as it agrees with the recommendations and has a simpler design

[presidentio]

Any updates here?

[edsonaj]

Hi, has this been solved in any way?

[presidentio]

@edsonaj It's fully functional in this branch, I have used it for several months

[edsonaj]

@presidentio but your branch is for 3.2, right? not 4.0

[presidentio]

I have a branch for 4.x https://github.com/presidentio/addons/tree/4.0.x-tuya-gateway-support, but didn't create a PR as this one doesn't have progress

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[m-jarzebowski]

Any update on that feature? What is missing for the merge?

[Ilia-SB]

I would also like to ask if this has been addressed. This binding is great, but I really miss subdevices support.