-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Move OccupancySensing to match the spec #29956
Move OccupancySensing to match the spec #29956
Conversation
PR #29956: Size comparison from cdea4cf to 742b068 Increases (16 builds for bl702, bl702l, cc32xx, linux, psoc6)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
@@ -89,13 +89,13 @@ limitations under the License. | |||
<access op="write" role="manage"/> | |||
</attribute> | |||
|
|||
<attribute side="server" code="0x0030" define="PHYSICAL_CONTACT_OCCUPIED_TO_UNOCCUPIED_DELAY" type="int16u" writable="true" default="0x0000" optional="true"> | |||
<attribute side="server" code="0x0030" define="PHYSICAL_CONTACT_OCCUPIED_TO_UNOCCUPIED_DELAY" type="int16u" writable="true" default="0x0000" optional="true" isNullable="true"> |
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.
This is a backwards incompatible API and wire change. We need to change the spec instead.
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.
Looking at the spec, this is also a bit strange. The PhysicalContactOccupiedToUnoccupiedDelay and PhysicalContactUnoccupiedToOccupiedDelay are the only delay attributes which is nullable.
I can make a PR to remove the X quality and remove "The null value indicates that the sensor does not report occupied to unoccupied transition." statement from the attributes? @andreilitvin?
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 think we should change the spec here: make things consistent and also not break backwards wire compatibility (which we certainly cannot do)
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.
Closing, keeping spec change instead only. |
Changes: