Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix integer attrib convertion with signed integer type (#1246) #1258

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

ben1hop
Copy link
Collaborator

@ben1hop ben1hop commented Feb 5, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented Feb 5, 2024

CLA assistant check
All committers have signed the CLA.

src-electron/validation/validation.js Outdated Show resolved Hide resolved
src-electron/validation/validation.js Show resolved Hide resolved
src-electron/validation/validation.js Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 90.56604% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 65.29%. Comparing base (4eb45cd) to head (b3a6b35).
Report is 8 commits behind head on master.

Files Patch % Lines
src-electron/validation/validation.js 90.56% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1258      +/-   ##
==========================================
+ Coverage   65.21%   65.29%   +0.08%     
==========================================
  Files         188      188              
  Lines       19722    19834     +112     
  Branches     4217     4245      +28     
==========================================
+ Hits        12861    12950      +89     
- Misses       6861     6884      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ben1hop ben1hop self-assigned this Mar 5, 2024
src-electron/validation/validation.js Show resolved Hide resolved
src-electron/validation/validation.js Outdated Show resolved Hide resolved
src-electron/validation/validation.js Outdated Show resolved Hide resolved
let fakeEndpointAttribute = {
defaultValue: '30',
}

let fakeAttribute = {
type: 'UINT16',
type: 'int16u',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this changed?

Copy link
Collaborator Author

@ben1hop ben1hop Mar 7, 2024

Choose a reason for hiding this comment

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

while testing the queryZcl.selectNumberByNameAndClusterId() returned nothing with the upper case one

my locally built zap tool also uses the lower case format whenever i edit an attribute, why was the other one used in the first place?

Copy link
Collaborator

Choose a reason for hiding this comment

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

How was this passing before?
The xml can vary how it represents unsigned 16 bit integers. I believe both representations exist based on matter/zigbee as of today.

Copy link
Collaborator Author

@ben1hop ben1hop Mar 7, 2024

Choose a reason for hiding this comment

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

the validation did not check for specific integer types before my implementation, only decided if the input string can be an integer and its unsigned value within bounds

@ben1hop ben1hop marked this pull request as ready for review March 5, 2024 14:24
@ben1hop ben1hop requested a review from brdandu March 5, 2024 14:24
@ben1hop ben1hop linked an issue Mar 8, 2024 that may be closed by this pull request
@ben1hop ben1hop merged commit 41c0089 into project-chip:master Mar 8, 2024
13 checks passed
@ben1hop ben1hop deleted the feature/ZAPP-1323 branch March 8, 2024 12:50
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.

ZAP editor should support signed data types
4 participants