Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apack.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "Graphical configuration tool for application and libraries based on Zigbee Cluster Library.",
"path": [".", "node_modules/.bin/", "ZAP.app/Contents/MacOS"],
"requiredFeatureLevel": "apack.core:9",
"featureLevel": 109,
"featureLevel": 110,
"uc.triggerExtension": "zap",
"uc.sdkProvidedProperties": "zcl.matterZclJsonFile,zcl.matterTemplateJsonFile,zcl.zigbeeZclJsonFile,zcl.zigbeeTemplateJsonFile",
"executable": {
Expand Down
221 changes: 221 additions & 0 deletions docs/api.md

Large diffs are not rendered by default.

60 changes: 60 additions & 0 deletions src-electron/db/query-session-zcl.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,65 @@ WHERE
.then((rows) => rows.map(dbMapping.map.cluster))
}

/**
* Returns all the clusters visible for a given session that belong to a ZCL
* package with the given category.
*
* This is required for multi-protocol sessions where multiple ZCL packages
* (e.g. "matter" and "zigbee") are loaded under the same session and a template
* should only iterate over clusters belonging to its own protocol.
*
* Clusters from custom XML packages (category NULL, type 'zcl-xml-standalone')
* are always included alongside the category-matched clusters.
*
* @param {*} db
* @param {*} sessionId
* @param {*} packageCategory - ZCL package category to filter by, e.g. "matter".
* @returns the cluster objects for the session matching the given category.
*/
async function selectAllSessionClustersByCategory(
db,
sessionId,
packageCategory
) {
return dbApi
.dbAll(
db,
`
SELECT
C.CLUSTER_ID,
C.PACKAGE_REF,
C.CODE,
C.MANUFACTURER_CODE,
C.NAME,
C.DESCRIPTION,
C.DEFINE,
C.DOMAIN_NAME,
C.IS_SINGLETON,
C.REVISION
FROM
CLUSTER AS C
INNER JOIN
SESSION_PACKAGE AS SP
ON
C.PACKAGE_REF = SP.PACKAGE_REF
INNER JOIN
SESSION_PARTITION
ON
SESSION_PARTITION.SESSION_PARTITION_ID = SP.SESSION_PARTITION_REF
INNER JOIN
PACKAGE AS P
ON
SP.PACKAGE_REF = P.PACKAGE_ID
WHERE
SESSION_PARTITION.SESSION_REF = ?
AND (P.CATEGORY = ? OR (P.CATEGORY IS NULL AND P.TYPE = ?))
`,
[sessionId, packageCategory, dbEnum.packageType.zclXmlStandalone]
)
.then((rows) => rows.map(dbMapping.map.cluster))
}

/**
* Returns the attribute available to this session by the code.
*
Expand Down Expand Up @@ -244,6 +303,7 @@ WHERE
}

exports.selectAllSessionClusters = selectAllSessionClusters
exports.selectAllSessionClustersByCategory = selectAllSessionClustersByCategory
exports.selectSessionClusterByCode = selectSessionClusterByCode
exports.selectSessionAttributeByCode = selectSessionAttributeByCode
exports.selectSessionCommandByCode = selectSessionCommandByCode
3 changes: 2 additions & 1 deletion src-electron/generator/template-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ async function produceIterativeContent(
let iterationArray = await templateIterators.getIterativeObject(
singleTemplatePkg.iterator,
db,
sessionId
sessionId,
genTemplateJsonPackage?.category
)

const pool = options.iterationPool
Expand Down
52 changes: 43 additions & 9 deletions src-electron/generator/template-iterators.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
const dbEnums = require('../../src-shared/db-enum')
const querySessionZcl = require('../db/query-session-zcl')
const queryEndpointType = require('../db/query-endpoint-type')
const templateUtil = require('./template-util')

// this structure links the names of iterators with the function.
const iterators = {}
Expand All @@ -36,9 +37,17 @@ iterators[dbEnums.iteratorValues.selectedClientCluster] =
* Get all clusters available for a given session
* @param {*} db
* @param {*} sessionId
* @param {*} packageCategory - ZCL package category to filter clusters by.
* @returns promise of all clusters available for a given session
*/
async function availableClusterIterator(db, sessionId) {
async function availableClusterIterator(db, sessionId, packageCategory) {
if (packageCategory) {
return querySessionZcl.selectAllSessionClustersByCategory(
db,
sessionId,
packageCategory
)
}
return querySessionZcl.selectAllSessionClusters(db, sessionId)
}

Expand All @@ -47,10 +56,16 @@ async function availableClusterIterator(db, sessionId) {
*
* @param {*} db
* @param {*} sessionId
* @returns Promise of all clusters in the endpoint types.
* @param {*} packageCategory - ZCL package category to filter endpoints by.
* @returns Promise of all clusters in the (filtered) endpoint types.
*/
async function selectedClusterIterator(db, sessionId) {
async function selectedClusterIterator(db, sessionId, packageCategory) {
let epts = await queryEndpointType.selectEndpointTypeIds(db, sessionId)
epts = await templateUtil.filterEndpointTypeIdsByCategory(
db,
epts,
packageCategory
)
return queryEndpointType.selectAllClustersDetailsFromEndpointTypes(db, epts)
}

Expand All @@ -59,10 +74,16 @@ async function selectedClusterIterator(db, sessionId) {
*
* @param {*} db
* @param {*} sessionId
* @returns Promise of all client clusters in the endpoint types.
* @param {*} packageCategory - ZCL package category to filter endpoints by.
* @returns Promise of all client clusters in the (filtered) endpoint types.
*/
async function selectedClientClusterIterator(db, sessionId) {
async function selectedClientClusterIterator(db, sessionId, packageCategory) {
let epts = await queryEndpointType.selectEndpointTypeIds(db, sessionId)
epts = await templateUtil.filterEndpointTypeIdsByCategory(
db,
epts,
packageCategory
)
return queryEndpointType.selectAllClustersDetailsFromEndpointTypes(
db,
epts,
Expand All @@ -75,10 +96,16 @@ async function selectedClientClusterIterator(db, sessionId) {
*
* @param {*} db
* @param {*} sessionId
* @returns Promise of all server clusters in the endpoint types.
* @param {*} packageCategory - ZCL package category to filter endpoints by.
* @returns Promise of all server clusters in the (filtered) endpoint types.
*/
async function selectedServerClusterIterator(db, sessionId) {
async function selectedServerClusterIterator(db, sessionId, packageCategory) {
let epts = await queryEndpointType.selectEndpointTypeIds(db, sessionId)
epts = await templateUtil.filterEndpointTypeIdsByCategory(
db,
epts,
packageCategory
)
return queryEndpointType.selectAllClustersDetailsFromEndpointTypes(
db,
epts,
Expand All @@ -92,12 +119,19 @@ async function selectedServerClusterIterator(db, sessionId) {
* @param {*} iteratorName
* @param {*} db
* @param {*} sessionId
* @param {*} packageCategory - e.g. "matter" or "zigbee"; passed to iterators
* that filter by ZCL package category. Null for single-protocol sessions.
* @returns Iterator array
*/
async function getIterativeObject(iteratorName, db, sessionId) {
async function getIterativeObject(
iteratorName,
db,
sessionId,
packageCategory
) {
let fn = iterators[iteratorName]
if (fn != null) {
return fn(db, sessionId)
return fn(db, sessionId, packageCategory)
} else {
let validValues = Object.keys(iterators).join(', ')
throw new Error(
Expand Down
151 changes: 66 additions & 85 deletions src-electron/generator/template-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,56 @@ async function ensureTemplatePackageId(context) {
}
}

/**
* Filters a list of endpoint type id objects to only those whose device type
* belongs to a ZCL package with the given category. When packageCategory is
* null (single-protocol session) the original list is returned unchanged.
*
* Each element of eptIds must have an `endpointTypeId` property (the shape
* returned by selectEndpointTypeIds).
*
* @param {*} db
* @param {*} eptIds - array of { endpointTypeId } objects
* @param {*} packageCategory - e.g. "matter" or "zigbee", or null
* @returns Promise that resolves with the filtered array.
*/
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
// return the original list 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
)
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
}
Comment on lines +266 to +301

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?


/**
* Populate the endpoint type ids into the global context.
* @param {*} context
Expand All @@ -260,93 +310,23 @@ async function ensureEndpointTypeIds(context) {
if (context.global.genTemplatePackage != null) {
packageCategory = context.global.genTemplatePackage.category
}
let resEptIds = []

if ('endpointTypeIds' in context.global) {
let eptIds = context.global.endpointTypeIds
if (!packageCategory) {
return eptIds
} else {
for (let i = 0; i < eptIds.length; i++) {
// Get endpoint type device info
let deviceTypes =
await queryDeviceType.selectDeviceTypesByEndpointTypeId(
context.global.db,
eptIds[i].endpointTypeId
)
// Sometimes a device type cannot be found for an endpoint type(undefined)
if (deviceTypes.length == 0) {
return context.global.endpointTypeIds
}
for (let j = 0; j < deviceTypes.length; j++) {
// Get device info
let deviceType = await queryDeviceType.selectDeviceTypeById(
context.global.db,
deviceTypes[j].deviceTypeRef
)
// Get package information to see the category of the device type
let packageInfo = await queryPackage.getPackageByPackageId(
context.global.db,
deviceType.packageRef
)
// Check for package category match based on gen template category and add it to relevant endpoint types
if (
packageInfo.category == packageCategory ||
!packageCategory ||
(!packageInfo.category &&
packageInfo.type === dbEnum.packageType.zclXmlStandalone)
) {
resEptIds.push(eptIds[i])
break
}
}
}
return resEptIds
}
} else {
let eptIds = await queryEndpointType.selectEndpointTypeIds(
context.global.db,
context.global.sessionId
)
if (!packageCategory) {
context.global.endpointTypeIds = eptIds
return eptIds
} else {
for (let i = 0; i < eptIds.length; i++) {
let deviceTypes =
await queryDeviceType.selectDeviceTypesByEndpointTypeId(
context.global.db,
eptIds[i].endpointTypeId
)
// Sometimes a device type cannot be found for an endpoint type(undefined)
if (deviceTypes.length == 0) {
context.global.endpointTypeIds = eptIds
return eptIds
}
for (let j = 0; j < deviceTypes.length; j++) {
let deviceType = await queryDeviceType.selectDeviceTypeById(
context.global.db,
deviceTypes[j].deviceTypeRef
)
let packageInfo = await queryPackage.getPackageByPackageId(
context.global.db,
deviceType.packageRef
)
if (
packageInfo.category == packageCategory ||
(!packageInfo.category &&
packageInfo.type === dbEnum.packageType.zclXmlStandalone)
) {
resEptIds.push(eptIds[i])
break
}
}
}
let eptIds =
'endpointTypeIds' in context.global
? context.global.endpointTypeIds
: await queryEndpointType.selectEndpointTypeIds(
context.global.db,
context.global.sessionId
)

context.global.endpointTypeIds = eptIds
return resEptIds
}
}
// Always cache the full unfiltered list so subsequent helpers don't re-query.
context.global.endpointTypeIds = eptIds

return filterEndpointTypeIdsByCategory(
context.global.db,
eptIds,
packageCategory
)
}

/**
Expand Down Expand Up @@ -553,6 +533,7 @@ exports.ensureZclAttributeTypeSdkExtensions =
exports.ensureZclCommandSdkExtensions = ensureZclCommandSdkExtensions
exports.ensureZclEventSdkExtensions = ensureZclEventSdkExtensions
exports.ensureZclDeviceTypeSdkExtensions = ensureZclDeviceTypeSdkExtensions
exports.filterEndpointTypeIdsByCategory = filterEndpointTypeIdsByCategory
exports.ensureEndpointTypeIds = ensureEndpointTypeIds
exports.makeSynchronizablePromise = makeSynchronizablePromise
exports.templatePromise = templatePromise
Expand Down
8 changes: 8 additions & 0 deletions test/custom-matter-xml.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,14 @@ test(
expect(endpointConfig).toContain(
' /* Endpoint: 1, Cluster: Sample Custom Cluster (server) */ \\'
)

// Iterator templates must include clusters from custom XML packages
// (category NULL, type zcl-xml-standalone) alongside the main ZCL package.
const contentKeys = Object.keys(genResult.content)
expect(contentKeys).toContain('available-cluster-Sample Custom Cluster.out')
expect(contentKeys).toContain(
'selected-server-cluster-Sample Custom Cluster.out'
)
},
testUtil.timeout.medium()
)
Loading
Loading