project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.45k stars 1.99k forks source link

How to process incomming commands in case of Bridge device #14486

Open yonko-gospodinov opened 2 years ago

yonko-gospodinov commented 2 years ago

I can use "External" attributes in combination with emberAfExternalAttributeReadCallback, emberAfExternalAttributeWriteCallback and MatterPostAttributeChangeCallback. But this will not work for commands like IASZone#ZoneEnrollRequest?

bzbarsky-apple commented 2 years ago

Clusters need to define APIs that they will use to talk to the application.

The IAS Zone clusters basically don't work at all in Matter right now. Someone needs to actually hook them up properly.

yonko-gospodinov commented 2 years ago

The IASZone was just an example, the main topic is regarding the generic processing of incoming commands similar to MatterPostAttributeChangeCallback. Do you plan such support?

bzbarsky-apple commented 2 years ago

Nothing like that is planned for 1.0 right now if you mean "handle the command inside the stack and also send a generic notification that the command happened to the application".

If the application wants to take over processing commands, it can use CommandHandlerInterface, but then it will also need to do its own TLV decoding, creating responses, etc, etc.

What sort of "generic processing" are you looking to enable? There's a limit to how much processing one can do without understanding the schema/spec definitions of specific commands...

woody-apple commented 2 years ago

@raju-apple can you help here?

yonko-gospodinov commented 2 years ago

I’m trying to build bridge application, which should bridge non-Matter devices from different protocols to the Matter network. I checked the bridge-app example, it uses emberAfExternalAttributeReadCallback and emberAfExternalAttributeWriteCallback as entry points for incoming requests. Both commands are generic. However, I did not find similar callback for commands. In most of the cases this two callbacks can do the job, sometimes they cannot.

For example: I have a bridged ZigBee devices with LevelControl cluster and a MoveToLevel command with transition time which is sent to. In this case I assume that multiple calls to emberAfExternalAttributeWriteCallback will be received. Now I’ll need to send multiple commands to the underlying ZigBee device instead of single command. How is supposed to handle the MoveToLevel command in this case? I expected to have a callback for received command (MoveToLevel in this case).

It looks like I can use CommandHandlerInterface, but it is too low level and I’ll need to implement a lot of stuff.

bzbarsky-apple commented 2 years ago

In this case I assume that multiple calls to emberAfExternalAttributeWriteCallback will be received.

If the default level control cluster implementation in the SDK handles the command, then yes.

But presumably you would want to just transcode the level control command into the ZigBee encoding format and send it to the ZigBee device instead, right?

I expected to have a callback for received command (MoveToLevel in this case).

What data did you expect this callback to receive?

but it is too low level and I’ll need to implement a lot of stuff.

Like what? In this specific case of MoveToLevel it looks to me like you would need to:

1) Detect that this is a MoveToLevel command based on the path handed in. 2) Decode the TLV into the MoveToLevel command payload struct (this is a single DataModel::Decode call). 3) Re-encode that payload into ZigBee format and send it (this is what any bridge to ZigBee would have to do no matter what).

What API shape are you looking for here?

yonko-gospodinov commented 2 years ago

Let me share my use-case and what I did expected based on the bridge-app example. We have a framework which support devices from multiple protocols and want to make these devices available in Matter network. The idea is to have a separate application build based on Matter project, which will enable our framework to bridge the non-Matter devices connected to it.

Following callbacks was promising: emberAfExternalAttributeWriteCallback, emberAfExternalAttributeReadCallback and MatterReportingAttributeChangeCallback. They are providing a generic way to manage cluster attributes in a generic way. Essentially, they have attribute path (endpointId, clusterId and attributeId), attribute type and raw attribute value. This provides an easy way to tunnel needed information between the Matter application and our framework. I still need to implement decode/encode of supported Matter types, but supported types are a lot less compared to the supported attributes among all current and future clusters. With such callbacks I can add support for new attributes without the need to implement it in the Matter application. I’ll need to only implement it in our framework.

Regarding the command requests and responses, I expected to have a similar callback(s) with command path (endpointId, clusterId and commandId) and array of parameters (parameter type + parameter raw value). This will provide an easy way to tunnel the commands and leave the entire logic in our framework, where I have the needed information/control about the bridged devices.

Its turnouts that the approach in bridge-app example can do the job for some simple cluster but fails with slightly more complex clusters. From our communication, probably CommandHandlerInterface is my only viable solution.

I’m trying to avoid having separate implementation for every attribute and command on both places the Matter application and our framework. I hope that I manage to explain why I’m searching for more generic approach.

bzbarsky-apple commented 2 years ago

They are providing a generic way to manage cluster attributes in a generic way.

Note that those external callbacks do not work for list or struct typed attributes, because those don't have a set "raw value"...

and array of parameters (parameter type + parameter raw value)

The problem is this: what would you expect for the parameter type and "raw value" when the parameter is a struct with multiple members of different types inside, some of which may be lists of structs?

What you suggest could probably be done for commands that only have non-list-or-struct fields, yes. And I understand what you are after now, thank you.

I think for the moment CommandHandlerInterface is your best option. It gives you the command fields in TLV, which does not tell you the exact schema types (e.g. int8u vs int16u) but does tell you "unsigned integer" and the value.... And it does allow handling arbitrary types as command arguments in a generic way.

If there are better things we could do here, we can certainly think about doing them. But so far what I see is either needing to restrict to some subset of commands or figuring out a more general type descriptor format to handle struct/list types.

yonko-gospodinov commented 2 years ago

I manage to implement what I need at the moment with: CommandHandlerInterface, AttributeAccessInterface and MatterReportingAttributeChangeCallback. @bzbarsky-apple Thanks for the assistant.

stale[bot] commented 2 years ago

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

stale[bot] commented 1 year ago

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

plan44 commented 1 year ago

For example: I have a bridged ZigBee devices with LevelControl cluster and a MoveToLevel command with transition time which is sent to. In this case I assume that multiple calls to emberAfExternalAttributeWriteCallback will be received. Now I’ll need to send multiple commands to the underlying ZigBee device instead of single command. How is supposed to handle the MoveToLevel command in this case? I expected to have a callback for received command (MoveToLevel in this case).

YFYI: I (and probably everyone writing bridges) faced that particular problem with LevelControl having a too low-level API (directly ramping CurrentLevel Attribute) too. My solution so far is re-implementing the cluster (for an example see here, lines 215ff). Since #22042 was accepted, there's a clean way to do so in a bridge project without patching connectedhomeip itself.

I'd like very much to approach this (and similar, and also dynamic cluster instantiation related) bridge implementor problems in a more generic way to prevent every bridge implementor to reinvent the wheel.

stale[bot] commented 1 year ago

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