-
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
Revert "Rename seconds to Ms for IM subscribe" #8310
Revert "Rename seconds to Ms for IM subscribe" #8310
Conversation
This reverts commit 9a123d5.
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.
Thank you. The other problems with this code (not matching the IM spec, using the wrong tag values) are still outstanding. :(
it is fixed here #8311 |
Size increase report for "esp32-example-build" from a55a0aa
Full report output
|
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.
Should we not rename "Ms" to "Sec" instead?
The whole point is that not using suffixes for units causes bugs. We only notice these bugs now that we actually have a unit and can say "this unit is wrong".
In spec, it does use seconds instead of Ms. https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/data_model/Interaction-Model.adoc#521-subscribe-request-action-information
Reverts #8285