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

Feature/schema command responses #7

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

mmb-davidsmith
Copy link
Contributor

Adds support for either an app message or default response specification for each command. At this point only the identify cluster is updated.

Updates the README.md file to refer to CSA instead of Zigbee. Changes most of the references to ZCL to be CL for cluster library, but notes that the XML content will still refer to the ZCL.
Updates to address concerns from @phil-sambles-schneider
Editorial updates made based on Arasch's review.
Updated the Have to Has. Waiting on feedback from @clapre about whether a CLA is even necessary given we're only accepting contributions from WG members.
Updates based on feedback from @ah-u about redundancy.
Removed requirement for CLA based on Oct 20th discussion with @clapre where it was determined that if only CSA members contribute it is not needed.
Updated language from Liz Bunker around requirements for Data Model contributions.
Adding helpdesk email until a specific one is generated.
…sponse statuses. As of yet I haven't found a way to validate the statusResponse values using JSON schema. It does validate that you're selecting a valid IdentifyQueryResponse.
@mmb-davidsmith
Copy link
Contributor Author

This is an example of how we might approach putting in possible responses. It just creates the linkage between messages and also allows a message to be specified as a "response only" message (meaning it can't be transmitted without a received message triggering it).

Review would be appreciated.

@mmb-davidsmith mmb-davidsmith marked this pull request as ready for review August 4, 2022 17:59
<command id="01" name="IdentifyQuery" required="true">
<responses>
<appResponse ref="IdentifyQueryResponse"/>
<statusResponse status="SUCCESS" />
Copy link
Contributor

@phil-jamieson phil-jamieson Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this about a response should someone request a default response or any responses from this command?

So if we are doing this for add group would we list out:
<appResponse ref="AddGroupResponse" /> <statusResponse status="SUCCESS" /> <statusResponse status="INVALID_VALUE" /> etc.
Should we perhaps tie the statusResponse to the appResponse, e.g.:

<appResponse ref="IdentifyQueryResponse> <statusResponse status="SUCCESS" /> </appResponse>
Then we could add:
<appResponse ref="DefaultResponse"> <statusResponse status="SUCCESS" /> </appResponse>
Just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. In this case the pattern is that you send an app response if you are identifying otherwise you send a default response of SUCCESS.

My logic hadn't extended this far when I laid it out, but I would think that limiting the allowable responses in the IdentifyQueryResponse is something we would do in the definition of that command rather than in the command which triggered the app response.

Having said that, I can see there being an issue if there are multiple commands which trigger an app response and different statuses on that command are permissible based on the initial message. I can't immediately think of a case where this is true (though the Image Block Request and Image Page Request messages technically both generate Image Block Response messages I don't know that we would technically model those messages are a response to the Image Page Request as the ZCL sequence number changes).

cluster.xsd Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants