Skip to content

Commit 74eadd0

Browse files
brdandubrdandu
authored
Fix the Legal Clusters view where users will only see clusters associated with a device type and not just enabled clusters which is wrong (#1573)
- Use the device types on the selected endpoint to figure out the device type clusters and then only show those clusters in the UI. - The Legal clusters UI view will also not allow users to enable/disable non compliant clusters and also not show non compliant clusters. - Adding cypress tests for filters of legal and enabled clusters - Add more tests where the clusters are check for enablement when clicking on the client and server checkboxes - Github: ZAP#316
1 parent e40b09d commit 74eadd0

File tree

13 files changed

+370
-52
lines changed

13 files changed

+370
-52
lines changed

‎cypress/e2e/clusters/cluster-filter.cy.js

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,94 @@ describe('Testing cluster filters', () => {
1313
})
1414
cy.setZclProperties()
1515
cy.fixture('data').then((data) => {
16-
cy.addEndpoint(data.endpoint1, data.cluster1)
16+
// Selecting SE Range Extender for Zigbee
17+
// Selecting Matter Dimmable Light (0x0101) for Matter
18+
cy.addEndpoint(data.endpoint9)
1719
})
1820
})
1921
it(
2022
'filter enabled clusters and check clusters',
2123
{ retries: { runMode: 2, openMode: 2 } },
2224
() => {
2325
cy.get('[data-test="filter-input"]').click()
26+
// Selecting Enabled clusters
2427
cy.get('.q-virtual-scroll__content > :nth-child(3)').click({
2528
force: true
2629
})
2730
cy.fixture('data').then((data) => {
28-
cy.get('tbody').children().contains(data.cluster2).should('not.exist')
31+
// Power Configurator for Zigbee is disabled and should not show up
32+
// Occupancy Sensing for Matter is disabled and should not exist
33+
cy.get('tbody').children().contains(data.cluster10).should('not.exist')
34+
// Basic for Zigbee is enabled and should show up
35+
// Identify for Matter is enabled and should show up
36+
cy.get('tbody').children().contains(data.cluster11).should('exist')
37+
})
38+
}
39+
)
40+
it(
41+
'filter legal clusters and check clusters',
42+
{ retries: { runMode: 2, openMode: 2 } },
43+
() => {
44+
cy.get('[data-test="filter-input"]').click()
45+
// Selecting Legal Clusters
46+
cy.get('.q-virtual-scroll__content > :nth-child(4)').click({
47+
force: true
48+
})
49+
cy.fixture('data').then((data) => {
50+
// Power Configurator for Zigbee is legal and should show up
51+
// Occupancy Sensing for Matter is legal and should show up
52+
if (data.cluster10 == 'Power Configuration') {
53+
cy.get('tbody')
54+
.children()
55+
.contains(data.cluster10)
56+
.should('exist')
57+
.parents('tr')
58+
.within(() => {
59+
cy.get('div[role="checkbox"][aria-label="Client"]')
60+
.should('have.attr', 'aria-checked', 'false')
61+
.click({ force: true })
62+
.should('have.attr', 'aria-checked', 'false') // Even when clicking on the client checkbox it should not be enabled because of legal cluster filter setting does not allow non Device type clusters to be enabled.
63+
})
64+
cy.get('tbody')
65+
.children()
66+
.contains(data.cluster10)
67+
.should('exist')
68+
.parents('tr')
69+
.within(() => {
70+
cy.get('div[role="checkbox"][aria-label="Server"]')
71+
.should('have.attr', 'aria-checked', 'false')
72+
.click({ force: true })
73+
.should('have.attr', 'aria-checked', 'true') // when clicking on the server checkbox it should be enabled because of legal cluster filter setting allows it to be selected
74+
})
75+
} else {
76+
// Occupancy Sensing
77+
cy.get('tbody')
78+
.children()
79+
.contains(data.cluster10)
80+
.should('exist')
81+
.parents('tr')
82+
.within(() => {
83+
cy.get('div[role="checkbox"][aria-label="Client"]')
84+
.should('have.attr', 'aria-checked', 'false')
85+
.click({ force: true })
86+
.should('have.attr', 'aria-checked', 'true') // when clicking on the server checkbox it should be enabled because of legal cluster filter setting allows it to be selected
87+
})
88+
cy.get('tbody')
89+
.children()
90+
.contains(data.cluster10)
91+
.should('exist')
92+
.parents('tr')
93+
.within(() => {
94+
cy.get('div[role="checkbox"][aria-label="Server"]')
95+
.should('have.attr', 'aria-checked', 'false')
96+
.click({ force: true })
97+
.should('have.attr', 'aria-checked', 'false') // Even when clicking on the client checkbox it should not be enabled because of legal cluster filter setting does not allow non Device type clusters to be enabled.
98+
})
99+
}
100+
101+
// Basic for Zigbee is legal and should show up
102+
// Identify for Matter is legal and should show up
103+
cy.get('tbody').children().contains(data.cluster11).should('exist')
29104
})
30105
}
31106
)

‎cypress/fixtures/data.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"endpoint6": "LO Dimmable Light (0x0101)",
88
"endpoint7": "Billing Unit (0x0203)",
99
"endpoint8": "Billing Unit (0x0203)",
10+
"endpoint9": "SE Range Extender (0x0008)",
1011
"cluster1": "General",
1112
"cluster2": "Power Configuration",
1213
"cluster3": "Basic",
@@ -16,6 +17,8 @@
1617
"cluster7": "Identify",
1718
"cluster8": "Data Sharing",
1819
"cluster9": "General",
20+
"cluster10": "Power Configuration",
21+
"cluster11": "Basic",
1922
"attribute1": "ZCL version",
2023
"attribute2": "application version",
2124
"attribute3": "OTA Upgrade Server ID",

‎cypress/matterFixtures/data.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"endpoint6": "Matter Dimmable Light (0x0101)",
88
"endpoint7": "Matter Control Bridge",
99
"endpoint8": "Matter Color Temperature Light (0x010C)",
10+
"endpoint9": "Matter Dimmable Light (0x0101)",
1011
"cluster1": "General",
1112
"cluster2": "On/off Switch Configuration",
1213
"cluster3": "Basic",
@@ -16,6 +17,8 @@
1617
"cluster7": "Identify",
1718
"cluster8": "Pulse Width Modulation",
1819
"cluster9": "General",
20+
"cluster10": "Occupancy Sensing",
21+
"cluster11": "Identify",
1922
"attribute1": "GeneratedCommandList",
2023
"attribute2": "IdentifyTime",
2124
"attribute3": "ClusterRevision",

‎src/App.vue

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ export default defineComponent({
248248
}
249249
this.$store.dispatch('zap/updateSelectedEndpoint', endpoint.id)
250250
this.$store.commit('zap/toggleEndpointModal', false)
251+
this.$store.dispatch(
252+
'zap/updateDeviceTypeClustersForSelectedEndpoint',
253+
this.endpointDeviceTypeRef[this.endpointType[endpoint.id]]
254+
)
251255
}
252256
},
253257
getAppData() {

‎src/components/ZclClusterManager.vue

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,23 +199,26 @@ export default {
199199
},
200200
relevantClusters: {
201201
get() {
202+
let relevantClusters = []
202203
if (this.clusters.clusterData) {
203-
return this.clusters.clusterData.filter((cluster) =>
204+
relevantClusters = this.clusters.clusterData.filter((cluster) =>
204205
this.filterString == ''
205206
? true
206207
: cluster.label
207208
.toLowerCase()
208209
.includes(this.filterString.toLowerCase())
209210
)
210211
} else {
211-
return this.clusters.filter((cluster) =>
212+
relevantClusters = this.clusters.filter((cluster) =>
212213
this.filterString == ''
213214
? true
214215
: cluster.label
215216
.toLowerCase()
216217
.includes(this.filterString.toLowerCase())
217218
)
218219
}
220+
this.$store.commit('zap/setRelevantClusters', relevantClusters)
221+
return relevantClusters
219222
}
220223
},
221224
enabledClusters: {
@@ -227,6 +230,12 @@ export default {
227230
return clusters
228231
}
229232
},
233+
deviceTypeClustersForSelectedEndpoint: {
234+
get() {
235+
return this.$store.state.zap.endpointTypeView
236+
.deviceTypeClustersForSelectedEndpoint
237+
}
238+
},
230239
filterOptions: {
231240
get() {
232241
return this.$store.state.zap.clusterManager.filterOptions
@@ -302,7 +311,12 @@ export default {
302311
.filter((a) => {
303312
return typeof this.filter.clusterFilterFn === 'function'
304313
? this.filter.clusterFilterFn(a, {
305-
enabledClusters: this.enabledClusters
314+
enabledClusters: this.enabledClusters,
315+
relevantClusters: this.relevantClusters,
316+
deviceTypeRefsForSelectedEndpoint:
317+
this.endpointDeviceTypeRef[this.selectedEndpointId],
318+
deviceTypeClustersForSelectedEndpoint:
319+
this.deviceTypeClustersForSelectedEndpoint
306320
})
307321
: true
308322
})
@@ -339,7 +353,12 @@ export default {
339353
changeDomainFilter(filter) {
340354
this.$store.dispatch('zap/setDomainFilter', {
341355
filter: filter,
342-
enabledClusters: this.enabledClusters
356+
enabledClusters: this.enabledClusters,
357+
relevantClusters: this.relevantClusters,
358+
deviceTypeRefsForSelectedEndpoint:
359+
this.endpointDeviceTypeRef[this.selectedEndpointId],
360+
deviceTypeClustersForSelectedEndpoint:
361+
this.deviceTypeClustersForSelectedEndpoint
343362
})
344363
},
345364
doActionFilter(filter) {

‎src/components/ZclCreateModifyEndpoint.vue

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,10 @@ export default {
780780
deviceTypeRefs: deviceTypeRef,
781781
endpointTypeRef: res.id
782782
})
783+
this.$store.dispatch(
784+
'zap/updateDeviceTypeClustersForSelectedEndpoint',
785+
this.endpointDeviceTypeRef[this.endpointType[res.id]]
786+
)
783787
})
784788
})
785789
.catch((err) => console.log('Error in newEpt: ' + err.message))
@@ -861,6 +865,10 @@ export default {
861865
deviceTypeRefs: deviceTypeRef,
862866
endpointTypeRef: this.endpointReference
863867
})
868+
this.$store.dispatch(
869+
'zap/updateDeviceTypeClustersForSelectedEndpoint',
870+
deviceTypeRef
871+
)
864872
},
865873
getDeviceOptionLabel(item) {
866874
if (item == null || item.deviceTypeRef == null) return ''

‎src/components/ZclDomainClusterView.vue

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,14 @@ limitations under the License.
166166
:options="optionsForCheckboxes"
167167
color="primary"
168168
type="checkbox"
169+
:disable="{
170+
client:
171+
getClusterDisableStatus(props.row.id).client ||
172+
!getClusterEnableStatus(props.row.id).client,
173+
server:
174+
getClusterDisableStatus(props.row.id).server ||
175+
!getClusterEnableStatus(props.row.id).server
176+
}"
169177
@update:model-value="handleClusterSelection(props.row.id, $event)"
170178
inline
171179
/>
@@ -239,6 +247,11 @@ export default {
239247
props: ['domainName', 'clusters'],
240248
mixins: [CommonMixin, uiOptions, EditableAttributesMixin],
241249
computed: {
250+
isLegalClusterFilterActive() {
251+
return (
252+
this.$store.state.zap.clusterManager.filter.label === 'Legal Clusters'
253+
)
254+
},
242255
isInStandalone: {
243256
get() {
244257
return this.$store.state.zap.standalone
@@ -254,6 +267,18 @@ export default {
254267
return this.$store.state.zap.clustersView.recommendedServers
255268
}
256269
},
270+
// Includes client clusters which are optional for a device type
271+
optionalClients: {
272+
get() {
273+
return this.$store.state.zap.clustersView.optionalClients
274+
}
275+
},
276+
// Includes server clusters which are optional for a device type
277+
optionalServers: {
278+
get() {
279+
return this.$store.state.zap.clustersView.optionalServers
280+
}
281+
},
257282
visibleColumns: function () {
258283
let names = this.columns.map((x) => x.name)
259284
let statusColumn = 'status'
@@ -281,6 +306,38 @@ export default {
281306
}
282307
},
283308
methods: {
309+
getClusterEnableStatus(id) {
310+
// Only enforce constraints if the active filter is "Legal Clusters"
311+
if (!this.isLegalClusterFilterActive) {
312+
return { client: true, server: true } // Allow all actions
313+
}
314+
let isClientRecommended = this.recommendedClients.includes(id)
315+
let isServerRecommended = this.recommendedServers.includes(id)
316+
let isClientOptional = this.optionalClients.includes(id)
317+
let isServerOptional = this.optionalServers.includes(id)
318+
let isClientEnabled = this.selectionClients.includes(id)
319+
let isServerEnabled = this.selectionServers.includes(id)
320+
321+
return {
322+
client: isClientRecommended || isClientOptional || isClientEnabled, // Allow enabling only if recommended, optional, or already enabled
323+
server: isServerRecommended || isServerOptional || isServerEnabled // Allow enabling only if recommended, optional, or already enabled
324+
}
325+
},
326+
getClusterDisableStatus(id) {
327+
// Only enforce constraints if the active filter is "Legal Clusters"
328+
if (!this.isLegalClusterFilterActive) {
329+
return { client: false, server: false } // No constraints
330+
}
331+
332+
let isClientRecommended = this.recommendedClients.includes(id)
333+
let isServerRecommended = this.recommendedServers.includes(id)
334+
let isClientOptional = this.optionalClients.includes(id)
335+
let isServerOptional = this.optionalServers.includes(id)
336+
return {
337+
client: isClientRecommended && !isClientOptional, // Disable if recommended and not optional
338+
server: isServerRecommended && !isServerOptional // Disable if recommended and not optional
339+
}
340+
},
284341
enableAllClusters() {
285342
this.clusters.forEach(async (singleCluster) => {
286343
await this.updateZclRolesByClusterSelection(singleCluster.id, [
@@ -499,6 +556,57 @@ export default {
499556
return tmpEvent
500557
},
501558
async handleClusterSelection(id, event) {
559+
const activeFilter = this.$store.state.zap.clusterManager.filter.label
560+
561+
// Only enforce constraints if the active filter is "Legal Clusters"
562+
if (activeFilter === 'Legal Clusters') {
563+
const disableStatus = this.getClusterDisableStatus(id)
564+
const enableStatus = this.getClusterEnableStatus(id)
565+
566+
// Prevent disabling recommended client
567+
if (disableStatus.client && event.includes('client') === false) {
568+
this.$q.notify({
569+
type: 'warning',
570+
position: 'top',
571+
message:
572+
'You cannot disable a required client cluster for the device types on this endpoint when using a Legal Clusters Filter.'
573+
})
574+
return
575+
}
576+
577+
// Prevent disabling recommended server
578+
if (disableStatus.server && event.includes('server') === false) {
579+
this.$q.notify({
580+
type: 'warning',
581+
position: 'top',
582+
message:
583+
'You cannot disable a required server cluster for the device types on this endpoint when using a Legal Clusters Filter.'
584+
})
585+
return
586+
}
587+
588+
// Prevent enabling non-recommended client
589+
if (!enableStatus.client && event.includes('client') === true) {
590+
this.$q.notify({
591+
type: 'warning',
592+
position: 'top',
593+
message:
594+
'You cannot enabled a client cluster not required by the device types on this endpoint when using a Legal Clusters Filter.'
595+
})
596+
return
597+
}
598+
599+
// Prevent enabling non-recommended server
600+
if (!enableStatus.server && event.includes('server') === true) {
601+
this.$q.notify({
602+
type: 'warning',
603+
position: 'top',
604+
message:
605+
'You cannot enabled a server cluster not required by the device types on this endpoint when using a Legal Clusters Filter.'
606+
})
607+
return
608+
}
609+
}
502610
let modifiedEvent = this.parseCheckboxEventToSelectionEvent(event)
503611
let selectionEvents = this.processZclSelectionEvent(id, modifiedEvent)
504612

0 commit comments

Comments
 (0)