-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Generate PressureMeasurement cluster XML with Alchemy #37926
base: master
Are you sure you want to change the base?
Generate PressureMeasurement cluster XML with Alchemy #37926
Conversation
PR #37926: Size comparison from d5d2f72 to 07c7ce5 Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, psoc6, qpg, stm32, telink, tizen)
|
<mandatoryConform/> | ||
</attribute> | ||
<attribute side="server" code="0x0003" name="Tolerance" define="PRESSURE_TOLERANCE" type="int16u" min="0x0000" max="0x0800" optional="true" default="0"> | ||
<attribute side="server" code="0x0000" name="MeasuredValue" define="PRESSURE_MEASURED_VALUE" type="int16s" isNullable="true" max="32767">MeasuredValue</attribute> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this 'max' value there? This is just the max for the type... Why does it need to be listed explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got some feedback from @hasty regarding this, it seems that 'max' should only be listed if it is different from the default max value for the specific type, I'll take a look at what are the expected 'default' values for int16 in the project, 32767 seems to be the max for the a int16 type but I'm not sure if that matches the default here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "default" max for int16 is 32767.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was a redundant "max" value, generated again with new Alchemy to remove those
PR #37926: Size comparison from 5adee57 to 6738adb Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Part of #37391
Code generation of Pressure measurement cluster with Alchemy.
Testing
Passed Unit Test and CI checks with Python and YAML tests.