Skip to content

fix/templateIteratorsCMP#1718

Merged
dhchandw merged 2 commits into
project-chip:masterfrom
dhchandw:fix/templateIteratorsCMP
Jun 18, 2026
Merged

fix/templateIteratorsCMP#1718
dhchandw merged 2 commits into
project-chip:masterfrom
dhchandw:fix/templateIteratorsCMP

Conversation

@dhchandw

Copy link
Copy Markdown
Collaborator
  • fix: filter elements in template iterators by protocol in CMP
  • Add tests

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces ZCL package category filtering (e.g., 'matter' or 'zigbee') for clusters and endpoint types in multi-protocol sessions, ensuring templates only iterate over relevant protocols. The review feedback highlights a critical logic bug in the new filterEndpointTypeIdsByCategory helper, where an empty device type list prematurely aborts the entire filtering process instead of continuing. Additionally, the feedback suggests adding defensive null/undefined checks to prevent runtime crashes and notes potential performance optimizations for the database queries.

Comment on lines +266 to +299
async function filterEndpointTypeIdsByCategory(db, eptIds, packageCategory) {
if (!packageCategory) return eptIds
let resEptIds = []
for (let i = 0; i < eptIds.length; i++) {
let deviceTypes = await queryDeviceType.selectDeviceTypesByEndpointTypeId(
db,
eptIds[i].endpointTypeId
)
// If no device type is found we cannot determine the category; include it
// to preserve existing behaviour.
if (deviceTypes.length == 0) {
return eptIds
}
for (let j = 0; j < deviceTypes.length; j++) {
let deviceType = await queryDeviceType.selectDeviceTypeById(
db,
deviceTypes[j].deviceTypeRef
)
let packageInfo = await queryPackage.getPackageByPackageId(
db,
deviceType.packageRef
)
if (
packageInfo.category == packageCategory ||
(!packageInfo.category &&
packageInfo.type === dbEnum.packageType.zclXmlStandalone)
) {
resEptIds.push(eptIds[i])
break
}
}
}
return resEptIds
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Issues Identified:

  1. Correctness / Logic Bug: Returning eptIds on line 277 when deviceTypes.length == 0 completely aborts the filtering process for all endpoint types in the list, even if only one of them lacks a device type. Instead, we should just include that specific endpoint type (resEptIds.push(eptIds[i])) and continue the loop to filter the remaining endpoint types.
  2. Defensive Programming / Safety: selectDeviceTypeById and getPackageByPackageId can return null or undefined if the database references are invalid or missing. Accessing properties on them without checking can cause runtime crashes (TypeError). We should add null/undefined guards.
  3. Performance / Efficiency: The nested loops execute multiple database queries per endpoint type, leading to an N+1 query problem. While this might be acceptable for small datasets, it can become a performance bottleneck for larger sessions. We can mention this as an optimization opportunity.
async function filterEndpointTypeIdsByCategory(db, eptIds, packageCategory) {
  if (!packageCategory) return eptIds
  let resEptIds = []
  for (let i = 0; i < eptIds.length; i++) {
    let deviceTypes = await queryDeviceType.selectDeviceTypesByEndpointTypeId(
      db,
      eptIds[i].endpointTypeId
    )
    // If no device type is found we cannot determine the category; include it
    // to preserve existing behaviour.
    if (!deviceTypes || deviceTypes.length == 0) {
      resEptIds.push(eptIds[i])
      continue
    }
    for (let j = 0; j < deviceTypes.length; j++) {
      let deviceType = await queryDeviceType.selectDeviceTypeById(
        db,
        deviceTypes[j].deviceTypeRef
      )
      if (!deviceType) continue
      let packageInfo = await queryPackage.getPackageByPackageId(
        db,
        deviceType.packageRef
      )
      if (!packageInfo) continue
      if (
        packageInfo.category == packageCategory ||
        (!packageInfo.category &&
          packageInfo.type === dbEnum.packageType.zclXmlStandalone)
      ) {
        resEptIds.push(eptIds[i])
        break
      }
    }
  }
  return resEptIds
}
References
  1. Enforce defensive programming by ensuring appropriate null/undefined checks exist before object property accesses to prevent runtime crashes.
  2. Ensure correctness in filtering logic so that a single exceptional item does not prematurely abort or bypass the filtering of other valid items in a collection.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What about this?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes template iterator behavior in CMP for multi-protocol sessions by filtering iterator outputs to only include clusters/endpoints matching the generator template package’s protocol category (e.g., Matter vs Zigbee), and adds regression tests and iterator templates to validate the behavior.

Changes:

  • Add protocol-category filtering support to template iterators and iterative template generation.
  • Introduce a new DB query for selecting session clusters by package category, and a shared helper for filtering endpoint type IDs by category.
  • Add iterator-based generator templates + tests to ensure multi-protocol sessions don’t leak clusters across protocols.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/test-util.js Updates expected Matter template count to reflect new iterator templates.
test/multi-protocol.test.js Extends generation scope and asserts iterator outputs are Matter-only in multi-protocol sessions.
test/gen-template/matter/selected-server-clusters.zapt Adds iterator template that emits one output per selected server cluster.
test/gen-template/matter/gen-test.json Registers new iterator templates for Matter generation tests.
test/gen-template/matter/available-clusters.zapt Adds iterator template that emits one output per available cluster.
test/gen-matter-1.test.js Updates expected generated output counts to include iterator-generated files.
src-electron/generator/template-util.js Factors endpoint-type filtering into a reusable helper and updates endpoint type ID caching behavior.
src-electron/generator/template-iterators.js Passes template package category into iterators and applies endpoint/cluster filtering.
src-electron/generator/template-engine.js Provides template package category to iterator resolution during iterative generation.
src-electron/db/query-session-zcl.js Adds category-filtered session cluster query used by availableCluster iterator.
docs/api.md Documents newly added DB/JS API functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src-electron/generator/template-util.js Outdated
Comment thread src-electron/db/query-session-zcl.js

@brdandu brdandu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Update the apack.json feature level and ask Matter to do the same. Forgot to update in the previous changes recently

@dhchandw dhchandw force-pushed the fix/templateIteratorsCMP branch from caba9de to 81b8e9d Compare June 17, 2026 20:33
@dhchandw

Copy link
Copy Markdown
Collaborator Author

Update the apack.json feature level and ask Matter to do the same. Forgot to update in the previous changes recently

Done

Comment thread src-electron/generator/template-util.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread src-electron/generator/template-util.js
Comment thread src-electron/db/query-session-zcl.js
@brdandu

brdandu commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Make sure to look into the copilot suggestions as they seem appropriate. Also add custom xml tests for iterators and make sure those are generated for both zigbee and matter

- Add tests
- Increase featureLevel to 110 in apack.json
@dhchandw dhchandw force-pushed the fix/templateIteratorsCMP branch from 81b8e9d to 0018eb3 Compare June 17, 2026 20:53
@dhchandw dhchandw merged commit f438524 into project-chip:master Jun 18, 2026
19 checks passed
@dhchandw dhchandw deleted the fix/templateIteratorsCMP branch June 18, 2026 01:59
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.

4 participants