Skip to content
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

ZAP UI should support setting Parent Endpoints and Composition Type for an Endpoint #1093

Closed
mhazley opened this issue Aug 2, 2023 · 11 comments
Assignees
Labels
high priority Tag that shows this as higher priority as other issues. matter Important to Matter SDK
Milestone

Comments

@mhazley
Copy link

mhazley commented Aug 2, 2023

As part of the Matter development for the Fall 2023 release, the concept of a "composed" device has been added using a "tree" composition.

Device composition is managed via the PartsList attribute in the Descriptor cluster for an Endpoint when an Endpoint has a device type that can be composed of other devices.

So, for example, if there's an Air Purifier on Endpoint 1, it could have a parts list of [2, 3] to indicate that it is composed of an Air Quality Sensor on Endpoint 2 and a Temperature Sensor on Endpoint 3.

This would mean that Endpoint 1 would be set as the Parent of Endpoints 2 & 3 so the SDK can build the parts list appropriately.

Also, in order to build the parts list in the correct format, the SDK also needs to be able to distinguish between Tree or Flat composition as this is described in the spec as:

  • Tree pattern of endpoints directly below the composed endpoint.
  • Flat list of all descendent endpoints below the composed endpoint.

So, it would be really useful if we could set a Parent and a Composition Type for an Endpoint in the UI, then this can be parsed by the SDK at setup.

Currently, we've added API's to set this information programmatically in this PR.

@bzbarsky-apple bzbarsky-apple added matter Important to Matter SDK high priority Tag that shows this as higher priority as other issues. labels Aug 2, 2023
@bzbarsky-apple
Copy link
Contributor

This is somewhat of a blocker for using ZAP as a "configure and just use the config" thing for the Fall 2023 devices. The APIs we are adding will let people work around, but at that point they will start asking why they are bothering with the non-programmatic ZAP config at all...

@bzbarsky-apple
Copy link
Contributor

@brdandu @tecimovic @paulr34

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Aug 2, 2023

Note that Tree vs Flat composition is associated with the device type, so ideally it would just be listed in the XML for the device types and no one would ever set it manually in the UI (but still needs backend support to expose the XML value to allow codegen based on it, perhaps). What the UI needs is a way to set the parent of an endpoint. Ideally, via showing a tree structure, not a list, for endpoints.

@paulr34 paulr34 self-assigned this Aug 2, 2023
@paulr34
Copy link
Collaborator

paulr34 commented Aug 2, 2023

ill set up a meeting with @tbrkollar to discuss the front end changes and ill work on adding the backend support to support the XML

@jvmahon
Copy link

jvmahon commented Sep 26, 2023

I'm hoping for a clarification on a related point.

The matter spec states:
image

However, when I look at the Zap files for the examples (e.g., the examples in Chef/devices), the "PartsList" is always blank.

  • Do I not understand what the matter spec requires, or are all the examples non-conforming? I.e., shouldn't all the examples have their "PartsList" set?

If it should be set, I'm confused as to how. The Zap entry box for PartsList seems to allow only a single integer digit. However, for a device with multiple endpoints, e.g., https://github.com/project-chip/connectedhomeip/blob/master/examples/chef/devices/rootnode_airpurifier_airqualitysensor_temperaturesensor_humiditysensor_thermostat_56de3d5f45.zap, as I understand it, the PartsList should identify endpoints 1, 2, 3, 4

Any help / clarification / examples would be appreciated.

@bzbarsky-apple
Copy link
Contributor

However, when I look at the Zap files for the examples (e.g., the examples in Chef/devices), the "PartsList" is always blank.

PartsList is computed at runtime from ZAP-provided (or not-ZAP-provided, for that matter) structural data by the SDK, not hardcoded in the ZAP UI.

This would be more clear if #1145 were fixed, since PartsList would then very clearly not have a way to set its value.

@jvmahon
Copy link

jvmahon commented Sep 28, 2023

This would be more clear if #1145 were fixed, since PartsList would then very clearly not have a way to set its value.

That would be fantastic. As someone new to this, it would certainly be clearer if the ZAP interface didn't suggest you should (or could) complete something where it doesn't make sense. In those cases a brief "set in software" message (or something along those lines) would help.

@paulr34
Copy link
Collaborator

paulr34 commented Feb 6, 2024

done

Parent Endpoint Feature - #1257

Matter SDK Helper - #1259

Disable defaultValue for all Attributes with External Storage -#1249

@paulr34 paulr34 closed this as completed Feb 6, 2024
@bzbarsky-apple
Copy link
Contributor

This does not work right. Specific problems with the code that was checked in:

  1. It's not letting me select the right endpoints as parents. For example, for all-clusters-app, which has endpoints 0, 1, 2, 65534, it lets me select only 1, 2, 3, 4 as parent endpoint ids.
  2. The output from the helper is a mix of integers and NULL, which is not workable. What it should be is just an array of parent endpoint ids. Anything without an explicit parent set should have 0 as parent.

@bzbarsky-apple bzbarsky-apple reopened this Feb 7, 2024
@brdandu brdandu added this to the ZAPP-1243 milestone Feb 7, 2024
@paulr34
Copy link
Collaborator

paulr34 commented Feb 7, 2024

thanks @bzbarsky-apple here are PR's to address your concerns

editing previously made endpoint - #1263

Matter SDK helper (Integer Array) - #1262

@paulr34
Copy link
Collaborator

paulr34 commented Mar 7, 2024

@mhazley the ZAP UI can now support setting a parent endpoint of an endpoint

@mhazley please lmk if you have anymore problems and concerns.

If so, feel free to reopen the ticket or make a new one, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority Tag that shows this as higher priority as other issues. matter Important to Matter SDK
Projects
Development

No branches or pull requests

6 participants