Replace and remove weakly-typed System::Clock code #11238
Merged
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
#### Problem Previous PRs added strong types to `System::Clock` and converted many uses, but a few remain. #### Change overview Convert the remaining uses of the weak types (`MonotonicMilliseconds` and `MonotonicMicroseconds`), and remove them from `System::Clock`. Fixes project-chip#10062 Some operations on System::Clock types are not safe There are some remaining uses of `Clock`-derived values as plain integers; these can be found for later cleanup by the use of `.count()`. #### Testing CI; no changes to functionality. Converted `TestPlatformTime`.
pullapprove
bot
requested review from
andy31415,
anush-apple,
austinh0,
balducci-apple,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
emargolis,
erjiaqing,
franck-apple,
harimau-qirex,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
LuDuda and
msandstedt
pullapprove
bot
requested review from
sagar-apple,
saurabhst,
selissia,
tcarmelveilleux,
tecimovic,
vivien-apple,
wbschiller,
woody-apple and
yufengwangca
|
PR #11238: Size comparison from 345723d to 2e14b9c Increases (8 builds for linux)
Decreases (25 builds for efr32, esp32, k32w, mbed, nrfconnect, p6, qpg, telink)
Full report (37 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
|
PR #11238: Size comparison from 345723d to 0711988 Increases above 0.2%:
Increases (18 builds for efr32, esp32, linux, mbed, nrfconnect)
Decreases (20 builds for efr32, k32w, mbed, nrfconnect, p6, qpg, telink)
Full report (37 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
andy31415
approved these changes
Nov 1, 2021
andy31415
approved these changes
Nov 1, 2021
andy31415
approved these changes
Nov 1, 2021
bzbarsky-apple
approved these changes
Nov 1, 2021
|
Fast track: several day old PR (had time to review if interested), generally safe change (refactor in naming and replacement), has several review checkmarks. |
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this pull request
Nov 2, 2021
#### Problem `System::Clock::Zero` is a constants that doesn't follow the usual convention of being named with a leading `k`. Requested in PR project-chip#11238 review. #### Change overview Rename `System::Clock::Zero` to `System::Clock::kZero`. #### Testing CI; no changes to functionality.
andy31415
pushed a commit
that referenced
this pull request
Nov 2, 2021
#### Problem `System::Clock::Zero` is a constants that doesn't follow the usual convention of being named with a leading `k`. Requested in PR #11238 review. #### Change overview Rename `System::Clock::Zero` to `System::Clock::kZero`. #### Testing CI; no changes to functionality.
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 2021
* WIP * Replace and remove weakly-typed System::Clock code #### Problem Previous PRs added strong types to `System::Clock` and converted many uses, but a few remain. #### Change overview Convert the remaining uses of the weak types (`MonotonicMilliseconds` and `MonotonicMicroseconds`), and remove them from `System::Clock`. Fixes project-chip#10062 Some operations on System::Clock types are not safe There are some remaining uses of `Clock`-derived values as plain integers; these can be found for later cleanup by the use of `.count()`. #### Testing CI; no changes to functionality. Converted `TestPlatformTime`. * review * initialization consistency * restyle
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 2021
#### Problem `System::Clock::Zero` is a constants that doesn't follow the usual convention of being named with a leading `k`. Requested in PR project-chip#11238 review. #### Change overview Rename `System::Clock::Zero` to `System::Clock::kZero`. #### Testing CI; no changes to functionality.
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.
Problem
Previous PRs added strong types to
System::Clockand converted manyuses, but a few remain.
Change overview
Convert the remaining uses of the weak types (
MonotonicMillisecondsand
MonotonicMicroseconds), and remove them fromSystem::Clock.Fixes #10062 Some operations on System::Clock types are not safe
There are some remaining uses of
Clock-derived values as plainintegers; these can be found for later cleanup by the use of
.count().Testing
CI; no changes to functionality.
Converted
TestPlatformTime.