Resolve issues related to multiple global attributes instances showing in the ZAP UI #1006
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
brdandu
commented
Apr 25, 2023
brdandu
commented
- This happens when the matter/zigbee sdks are updated and the same .zap sample app file is opened using zap.
- Updating the attribute tables constraints such that attributes are unique based on cluster_ref, package_ref, code, mfg code and side
- Added CLUSTER_REF_NOT_NULL and MANUFACTURER_CODE_NOT_NULL to the table because the unique constraint does not work correctly with null values in sqlite. Therefore adding an empty string to them and using those in the unique constraint
- In query-loader#INSERT_ATTRIBUTE_QUERY using ignore such that duplicate attribute insertions are ignored.
- In query-impexp#importAttributeForEndpointType ignoring insert if an endpoint type attribute already exists based on UNIQUE(ENDPOINT_TYPE_REF, ATTRIBUTE_REF, ENDPOINT_TYPE_CLUSTER_REF ). The ZAP UI was running into this when SDK was updated and the saved sample .zap file was opened again.
- JIRA: ZAPP-1079
Codecov Report
@@ Coverage Diff @@
## master #1006 +/- ##
==========================================
+ Coverage 67.06% 67.07% +0.01%
==========================================
Files 157 157
Lines 17187 17211 +24
Branches 3753 3755 +2
==========================================
+ Hits 11527 11545 +18
- Misses 5660 5666 +6
|
brdandu
force-pushed
the
bug/removeDuplicateEntriesFromAttributeTables/ZAPP-1079
branch
from
d137b3e
to
475e6f5
Compare
…g in the ZAP UI and resolving issues related to .zap repo deletion - This happens when the matter/zigbee sdks are updated and the same .zap sample app file is opened using zap. - Updating the attribute tables constraints such that attributes are unique based on cluster_ref, package_ref, code, mfg code and side - Added CLUSTER_REF_NOT_NULL and MANUFACTURER_CODE_NOT_NULL to the table because the unique constraint does not work correctly with null values in sqlite. Therefore adding an empty string to them and using those in the unique constraint - The ZAP UI was running into this when SDK was updated and the saved sample .zap file was opened again. - In db-api, setting PRAGMA FOREIGN_KEYS=1 because each instance of the db needs to have this for on delete cascades to work properly - Removing the IGNORE commands from the insert commands for attribute and endpoint_type_attribute tables because ignore just hides the underlying issue of attempting to insert duplicate entries into the database - Adding the query-package#deletePackagesByPackageId to delete package by id and also follow up additional deletions across all the tables with the on delete cascade operation - Updating the schema with on delete cascade - In zcl-loader-silabs.js, checking for a crc mismatch(isCrcMismatch function) in the xml packages. If there is a crc mismatch in any of the xml packages then deleting it's parent package which in turn has a cascading effect on deleting all the child packages under the parent packages. After deletion, all the packages are reloaded again to make sure that the schema is validated properly. - Updating the zap schema - JIRA: ZAPP-1079
brdandu
force-pushed
the
bug/removeDuplicateEntriesFromAttributeTables/ZAPP-1079
branch
from
475e6f5
to
7e204ae
Compare
- The old .zap files have duplicate entries of attributes and commands so having the unique constraint prevents the zap file from being loaded when there are duplicate endpoint type attributes in the .zap file - Add this back once ZAPP-1192 is resolved - JIRA: ZAPP-1079
- There are 2 use cases where custom xml changes needed to be reflected in the backend when a user updates the custom xml. - The first use case is related to user editing the custom xml, deleting it from zcl extensions and then re-adding it. This flow should make sure that the configuration is updated in the backend after this change. See zcl-loader#qualifyZclFile - The second use case is when the custom xml is already saved as a reference in the .zap config file. In this case if the custom xml file is edited then the next time the .zap file is re-opened then the backend should reflect this change as well. See import-json#jsonDataLoader - JIRA: ZAPP-1079
tecimovic
requested changes
May 11, 2023
| `CRC missmatch for file ${pkg.path}, (${pkg.crc} vs ${actualCrc}) package id ${pkg.id}, parsing. | ||
| Deleting package id: ${packageId}` | ||
| ) | ||
| await queryPackage.deletePackagesByPackageId(db, packageId) |
There was a problem hiding this comment.
You can't just delete this package. There might have been another session opened which uses this package.
There was a problem hiding this comment.
I suggest you load this new package as well, and then make the session use this new package, not the old package. There is no problems if you have multiple standalone packages.
| // is re-opened then the custom xml is reloaded to reflect the correct | ||
| // configuration. | ||
| if (pkg.type == 'zcl-xml-standalone') { | ||
| let info = await queryPackage.getPackageByPackageId(db, packageId) |
There was a problem hiding this comment.
I thought we have a function for this somewhere. util.readFileContentsAndCrc() or something?
| // This check makes sure that when a custom xml is edited, deleted and | ||
| // re-added from ZCL Extensions in ZAP UI then the custom xml package is | ||
| // updated in the backend to reflect the right configuration. | ||
| await queryPackage.deletePackagesByPackageId(db, pkg.id) |
There was a problem hiding this comment.
This deleting of packages still sounds dangerous. How do you know there isn't another session using this package?
There was a problem hiding this comment.
This is done when the zcl loading happens. Is it possible to have loading happen when an existing session is still in progress?
There was a problem hiding this comment.
Sorry ignore me. This is the custom xml through zcl extension. I see your point.
|
@brdandu this bug is fixed so this PR is not needed, please reopen if I am mistaken! thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.