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.

Sign up for GitHub

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

Jump to bottom

Type-safe System::Clock #10589

Merged
merged 2 commits into from
Oct 21, 2021
Merged

Type-safe System::Clock #10589

merged 2 commits into from
Oct 21, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

Issue #10062 Some operations on System::Clock types are not safe

Change overview

Introduce safe types based on std::chrono::duration to System::Clock.

In this change, the new types are returned from the platform clock
functions and forwarded through compatibility methods. Future changes
will use the safe types directly, starting with system timers and
proceeding upward through the stack.

Testing

Updated src/system/tests/TestSystemClock.cpp

@kpschoedel
Type-safe System::Clock
83c2dda 
#### Problem

Issue project-chip#10062 _Some operations on System::Clock types are not safe_

#### Change overview

Introduce safe types based on `std::chrono::duration` to `System::Clock`.

Future changes will use these types, starting with system timers
and proceeding upward through the stack.

#### Testing

Updated `src/system/tests/TestSystemClock.cpp`
@github-actions
Copy link

github-actions bot commented Oct 15, 2021 •
edited
Loading

PR #10589: Size comparison from efc17de to 83c2dda

8 builds
platform target config section efc17de 83c2dda change % change
k32w lock-app k32w061+debug .bss 69052 69052 0 0.0
.data 1864 1864 0 0.0
.text 515176 515256 80 0.0
shell k32w061+debug .bss 55080 55080 0 0.0
.data 672 672 0 0.0
.text 357172 357236 64 0.0
lighting-app k32w061+se05x+release .bss 78560 78560 0 0.0
.data 1900 1900 0 0.0
.text 613800 613880 80 0.0
p6 lock-app default .bss 68216 68216 0 0.0
.data 2416 2416 0 0.0
.heap 962712 962712 0 0.0
.text 1126280 1126408 128 0.0
qpg lighting-app qpg6100+debug .bss 53536 53536 0 0.0
.data 996 996 0 0.0
.text 486104 486176 72 0.0
lock-app qpg6100+debug .bss 52488 52488 0 0.0
.data 952 952 0 0.0
.text 462352 462432 80 0.0
persistent-storage-app qpg6100+debug .bss 17778 17778 0 0.0
.data 280 280 0 0.0
.text 102704 102728 24 0.0
telink lighting-app tlsr9518adk80d bss 70984 70984 0 0.0
noinit 33216 33216 0 0.0
text 458146 458184 38 0.0
14 builds
platform target config section efc17de 83c2dda change % change
efr32 lighting-app BRD4161A .bss 118020 118020 0 0.0
.data 1800 1800 0 0.0
.text 782420 782548 128 0.0
lock-app BRD4161A .bss 115892 115892 0 0.0
.data 1760 1760 0 0.0
.text 761724 761852 128 0.0
window-app BRD4161A .bss 116212 116212 0 0.0
.data 1764 1764 0 0.0
.text 762632 762744 112 0.0
lighting-app BRD4161A+rpc .bss 131348 131348 0 0.0
.data 1852 1852 0 0.0
.text 762148 762260 112 0.0
linux all-clusters-app debug .bss 52176 52176 0 0.0
.data 978 978 0 0.0
.data.rel.ro 58432 58432 0 0.0
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 512 0 0.0
.rodata 135989 135989 0 0.0
.text 1326066 1326034 -32 -0.0
chip-tool debug .bss 17552 17552 0 0.0
.data 1584 1584 0 0.0
.data.rel.ro 83312 83312 0 0.0
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 416 0 0.0
.rodata 174560 174560 0 0.0
.text 2421973 2421957 -16 -0.0
ota-provider-app debug .bss 37472 37472 0 0.0
.data 752 752 0 0.0
.data.rel.ro 23176 23176 0 0.0
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 440 440 0 0.0
.rodata 109848 109848 0 0.0
.text 1009314 1009282 -32 -0.0
ota-requestor-app debug .bss 205728 205728 0 0.0
.data 752 752 0 0.0
.data.rel.ro 24488 24488 0 0.0
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 512 512 0 0.0
.rodata 127896 127896 0 0.0
.text 1128626 1128578 -48 -0.0
shell debug .bss 16072 16072 0 0.0
.data 242 242 0 0.0
.data.rel.ro 35120 35120 0 0.0
.dynamic 592 592 0 0.0
.got 3496 3496 0 0.0
.init 27 27 0 0.0
.init_array 336 336 0 0.0
.rodata 71855 71855 0 0.0
.text 568898 568818 -80 -0.0
tv-app debug .bss 216368 216368 0 0.0
.data 2032 2032 0 0.0
.data.rel.ro 55536 55536 0 0.0
.dynamic 592 592 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 608 608 0 0.0
.rodata 151464 151464 0 0.0
.text 1462818 1462786 -32 -0.0
bridge-app debug+rpc .bss 52880 52880 0 0.0
.data 976 976 0 0.0
.data.rel.ro 25496 25496 0 0.0
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 400 400 0 0.0
.rodata 110484 110484 0 0.0
.text 1050821 1050837 16 0.0
lighting-app debug+rpc .bss 42200 42200 0 0.0
.data 1106 1106 0 0.0
.data.rel.ro 52240 52240 0 0.0
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 528 528 0 0.0
.rodata 127665 127665 0 0.0
.text 1252226 1252194 -32 -0.0
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172148 172148 0 0.0
.data 5464 5464 0 0.0
.heap 858832 858832 0 0.0
.text 1219384 1219512 128 0.0
lock-app CY8CPROTO_062_4343W+release .bss 171084 171084 0 0.0
.data 5432 5432 0 0.0
.heap 859928 859928 0 0.0
.text 1197440 1197504 64 0.0
2 builds
platform target config section efc17de 83c2dda change % change
esp32 all-clusters-app c3devkit .dram0.bss 60280 60280 0 0.0
.dram0.data 16192 16192 0 0.0
.flash.rodata 198624 198624 0 0.0
.flash.text 870142 870146 4 0.0
.iram0.text 57330 57330 0 0.0
m5stack .dram0.bss 62776 62776 0 0.0
.dram0.data 32084 32084 0 0.0
.flash.rodata 207056 207056 0 0.0
.flash.text 900507 900631 124 0.0
.iram0.text 125115 125115 0 0.0
10 builds
platform target config section efc17de 83c2dda change % change
nrfconnect lighting-app nrf52840dk_nrf52840 bss 112316 112316 0 0.0
rodata 97528 97528 0 0.0
text 577428 577508 80 0.0
lock-app nrf52840dk_nrf52840 bss 111348 111348 0 0.0
rodata 94028 94028 0 0.0
text 558928 559008 80 0.0
pigweed-app nrf52840dk_nrf52840 bss 51772 51772 0 0.0
rodata 45772 45772 0 0.0
text 339392 339392 0 0.0
pump-app nrf52840dk_nrf52840 bss 111416 111416 0 0.0
rodata 95008 95008 0 0.0
text 562084 562164 80 0.0
pump-controller-app nrf52840dk_nrf52840 bss 111356 111356 0 0.0
rodata 94088 94088 0 0.0
text 558720 558800 80 0.0
shell nrf52840dk_nrf52840 bss 107316 107316 0 0.0
rodata 71640 71640 0 0.0
text 518908 518968 60 0.0
lighting-app nrf52840dk_nrf52840+rpc bss 108556 108556 0 0.0
rodata 88320 88320 0 0.0
text 550632 550712 80 0.0
nrf5340dk_nrf5340_cpuapp bss 113688 113688 0 0.0
rodata 92772 92772 0 0.0
text 506888 506972 84 0.0
lock-app nrf5340dk_nrf5340_cpuapp bss 112720 112720 0 0.0
rodata 89284 89284 0 0.0
text 488380 488464 84 0.0
shell nrf5340dk_nrf5340_cpuapp bss 108300 108300 0 0.0
rodata 66284 66284 0 0.0
text 439504 439576 72 0.0

bzbarsky-apple
bzbarsky-apple approved these changes Oct 20, 2021
View reviewed changes
src/system/SystemClock.cpp Outdated Show resolved Hide resolved
src/system/SystemClock.h Outdated Show resolved Hide resolved
@andy31415 andy31415 merged commit 0fd7520 into project-chip:master Oct 21, 2021
@kpschoedel kpschoedel deleted the x10062-clock-type-1-types branch October 21, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@emargolis emargolis emargolis approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@shana-apple shana-apple Awaiting requested review from shana-apple

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@turon turon Awaiting requested review from turon

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
esp32 freeRTOS lib linux platform review - approved system zephyr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kpschoedel @andy31415 @emargolis @bzbarsky-apple @Damian-Nordic