Changing the data type by name helpers to use data type name and cluster by brdandu · Pull Request #1669 · project-chip/zap

brdandu · 2025-11-10T11:45:06Z

Updating types.getSignAndSizeOfZclType to types.getSignAndSizeOfZclTypeAndClusterId for better accuracy and avoiding duplicate data types across clusters.
updating the code associated with the above as required in the code.
deprecating as_bytes and data_types_for_enum as they are no longer used.
Github: ZAP selectEnumByName and selectBitmapByName need to switch to selectEnumByNameAndClusterName and selectBitmapByNameAndClusterName #1592

gemini-code-assist · 2025-11-10T11:45:38Z

Summary of Changes

Hello @brdandu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the accuracy of ZCL data type resolution within the system by introducing cluster-awareness. The primary goal is to prevent ambiguities and errors that arise when data types with identical names exist in different ZCL clusters. This is achieved by implementing a new function that prioritizes cluster-specific type definitions, leading to more robust and precise data handling throughout the application. Additionally, older, less accurate helpers have been deprecated, and documentation has been updated to reflect these changes.

Highlights

Cluster-Aware Data Type Resolution: Introduced a new utility function, getSignAndSizeOfZclTypeAndClusterId, which accurately determines ZCL data type properties (sign and size) by first checking for cluster-specific definitions before falling back to global ones. This ensures correct handling of data types that might have the same name but different definitions across clusters.
Updated Helper Functions and Validation: Various helper functions and validation logic across the codebase have been updated to utilize the new cluster-aware data type resolution. This includes changes in helper-endpointconfig.js, helper-zcl.js, and validation.js to pass cluster context when resolving type information.
Deprecated Helpers: The as_bytes and data_type_for_enum helpers in helper-c.js have been explicitly deprecated. This is due to their inability to correctly handle cluster-specific data types, and deprecation messages have been added to guide developers.
Enhanced Documentation: Documentation for ZCL helpers (as_type_min_value, as_type_max_value, as_zcl_type_size) in docs/helpers.md has been updated to reflect the new clusterId option, improving clarity for template developers.
Exact Type Size Calculation: A noCeiling option has been added to type size calculation functions, allowing for the retrieval of exact type sizes without rounding up to the nearest power of 2.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a necessary change to handle cluster-specific data types by adding a clusterId to the type resolution logic. The new function getSignAndSizeOfZclTypeAndClusterId and its integration into various helpers are well-implemented. The documentation has also been updated accordingly.

My review includes a few suggestions to improve maintainability by reducing code duplication in helper-zcl.js and types.js. I also found a minor bug in helper-c.js where a helper is exported twice, and a typo in a log message in types.js. Overall, this is a good improvement.

src-electron/generator/helper-c.js

src-electron/generator/helper-zcl.js

src-electron/util/types.js

tecimovic

Generally looks good, but I have no way of being able to go into details of this...

- Updating types.getSignAndSizeOfZclType to types.getSignAndSizeOfZclTypeAndClusterId for better accuracy and avoiding duplicate data types across clusters. - updating the code associated with the above as required in the code. - deprecating as_bytes and data_types_for_enum as they are no longer used. - Handling clusterId for all the helpers. Now the helpers can mention clusterId=ID-NUMBER along with the helpers in .zapt templates to account for the helpers. The helpers in helpers.js files now extract this from the this object automatically - Updating the feature level of zap - Adding unit tests for the as_underlying_zcl_type helper with cluster Id reference - Github: ZAP project-chip#1592

bzbarsky-apple · 2026-01-12T18:34:58Z

docs/api.md

 - language: determines the output of the helper based on language
 for eg: (as_type_min_value language='c++') will give the output specific to
 the c++ language.
+- clusterId: The ID of the cluster the type belongs to


Is this the "id" in the sense of database ID, or the "id" in the sense of the actual spec-defined cluster identifier? It's not obvious just looking at this documentation.

bzbarsky-apple · 2026-01-12T18:35:05Z

docs/api.md

 - language: determines the output of the helper based on language
 for eg: (as_type_max_value language='c++') will give the output specific to
 the c++ language.
+- clusterId: The ID of the cluster the type belongs to


bzbarsky-apple · 2026-01-12T18:35:14Z

docs/api.md

+| Param | Type | Description |
+| --- | --- | --- |
+| type | <code>\*</code> |  |
+| options | <code>\*</code> | Available Options: - clusterId: The ID of the cluster the type belongs to |


bzbarsky-apple · 2026-01-12T18:35:59Z

docs/api.md

    * [~nullStringDefaultValue(type)](#module_JS API_ type related utilities..nullStringDefaultValue) ⇒ <code>string</code>
+    * [~processZclTypeSignAndSize(db, dataType, type, packageIds, options, clusterId, clusterName)](#module_JS API_ type related utilities..processZclTypeSignAndSize) ⇒
    * [~getSignAndSizeOfZclType(type, context, options)](#module_JS API_ type related utilities..getSignAndSizeOfZclType) ⇒
+    * [~getSignAndSizeOfZclTypeAndClusterId(db, type, clusterId, packageIds, options)](#module_JS API_ type related utilities..getSignAndSizeOfZclTypeAndClusterId) ⇒ <code>size:&#x27;bits&#x27;</code>


See above about id.

bzbarsky-apple · 2026-01-12T18:36:17Z

docs/api.md

+| type | <code>\*</code> |  | The type name (for error logging) |
+| packageIds | <code>\*</code> |  | Package IDs |
+| options | <code>\*</code> |  | Processing options |
+| clusterId | <code>\*</code> | <code></code> | Optional cluster ID for cluster-specific queries |


bzbarsky-apple · 2026-01-12T18:37:02Z

docs/api.md

+| --- | --- |
+| db | <code>\*</code> | 
+| type | <code>\*</code> | 
+| clusterId | <code>\*</code> | 


bzbarsky-apple · 2026-01-12T18:37:28Z

docs/api.md

+| --- | --- |
+| db | <code>\*</code> | 
+| enum_name | <code>\*</code> | 
+| clusterId | <code>\*</code> | 


I'll stop commenting on this, but it's a general problem in all these APIs.

bzbarsky-apple · 2026-01-12T18:43:29Z

src-electron/util/types.js

 }

+/**
+ * Given a zcl device type returns its sign, size and zcl data type info stored


This has nothing to do with "device type"....

bzbarsky-apple · 2026-01-12T18:43:51Z

src-electron/util/types.js

+}
+
+/**
+ * Given a zcl device type and cluster name returns its sign, size and zcl data type info stored


Again, not "device type".

brdandu requested review from bzbarsky-apple, dhchandw and ethanzhouyc November 10, 2025 11:45

brdandu requested review from paulr34 and tecimovic as code owners November 10, 2025 11:45

brdandu linked an issue Nov 10, 2025 that may be closed by this pull request

selectEnumByName and selectBitmapByName need to switch to selectEnumByNameAndClusterName and selectBitmapByNameAndClusterName #1592

Closed

gemini-code-assist bot reviewed Nov 10, 2025

View reviewed changes

brdandu force-pushed the feature/updateHelpersToUseDataTypeByNameAndCluster/ZAP#1592 branch from cb3f402 to 6cb445e Compare November 11, 2025 12:34

tecimovic reviewed Nov 11, 2025

View reviewed changes

tecimovic approved these changes Nov 11, 2025

View reviewed changes

brdandu force-pushed the feature/updateHelpersToUseDataTypeByNameAndCluster/ZAP#1592 branch from 193d651 to 807c101 Compare November 13, 2025 11:10

brdandu force-pushed the feature/updateHelpersToUseDataTypeByNameAndCluster/ZAP#1592 branch from 807c101 to 2708354 Compare November 13, 2025 11:11

paulr34 approved these changes Nov 13, 2025

View reviewed changes

brdandu merged commit aa68383 into project-chip:master Nov 13, 2025
14 checks passed

bzbarsky-apple reviewed Jan 12, 2026

View reviewed changes

Conversation

brdandu commented Nov 10, 2025

Uh oh!

gemini-code-assist bot commented Nov 10, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tecimovic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants