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

Fix signdness for device type: DataModelTypes.h says DeviceTypeId is … #23015

Closed
wants to merge 1 commit into from
Closed

Fix signdness for device type: DataModelTypes.h says DeviceTypeId is … #23015

wants to merge 1 commit into from

Conversation

andy31415
Copy link
Contributor

DataModelTypes defines DeviceTypeId as unsigned. Without this fix, dynamic-bridge fails to compile on clang:

'chip::DeviceTypeId' (aka 'unsigned int') to 'int32_t' (aka 'int') [-Werror,-Wsign-conversion]
Step #2 - "CompileAll": 2022-10-04 05:16:04 INFO            type = t.type;
Step #2 - "CompileAll": 2022-10-04 05:16:04 INFO                 ~ ~~^~~~
Step #2 - "CompileAll": 2022-10-04 05:16:04 INFO    gen/bridge/Descriptor.h:25:16: error: implicit conversion changes signedness: 'const int32_t' (aka 'const int') to 'chip::DeviceTypeId' (aka 'unsigned int') [-Werror,-Wsign-conversion]
Step #2 - "CompileAll": 2022-10-04 05:16:04 INFO          t.type = type;
Step #2 - "CompileAll": 2022-10-04 05:16:04 INFO                 ~ ^~~~

After the fix, compilation succeeds.

@andy31415 andy31415 added the hotfix urgent fix needed, can bypass review label Oct 4, 2022
@pullapprove pullapprove bot added review - pending and removed hotfix urgent fix needed, can bypass review scripts labels Oct 4, 2022
@andy31415
Copy link
Contributor Author

Hotfix because this fixes compile error in bridge and CI does not test dynamic-bridge compilation.

Damian-Nordic
Damian-Nordic reviewed Oct 4, 2022
View reviewed changes
scripts/idl/generators/types.py
@@ -195,7 +195,7 @@ def is_struct(self) -> bool:
"command_id": BasicInteger(idl_name="command_id", byte_count=4, is_signed=True),
"data_ver": BasicInteger(idl_name="data_ver", byte_count=4, is_signed=True),
"date": BasicInteger(idl_name="date", byte_count=4, is_signed=True),
"devtype_id": BasicInteger(idl_name="devtype_id", byte_count=4, is_signed=True),
"devtype_id": BasicInteger(idl_name="devtype_id", byte_count=4, is_signed=False),
"endpoint_no": BasicInteger(idl_name="endpoint_no", byte_count=2, is_signed=True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that there are more unsigned types here, e.g. CommandId, EndpointNo etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update in #23017 .. this was intended as hotfix to fix a compile error, but for now we may as well fix the rest.

@github-actions
Copy link

github-actions bot commented Oct 4, 2022 •
edited
Loading

PR #23015: Size comparison from 26e72ab to 04b077e

Increases (3 builds for bl702, cc13x2_26x2, psoc6)
platform target config section 26e72ab 04b077e change % change
bl702 lighting-app bl702+rpc .debug_info 41809186 41809187 1 0.0
.text 1030324 1030326 2 0.0
cc13x2_26x2 pump-controller-app LP_CC2652R7 (read only) 671895 671903 8 0.0
.text 585328 585336 8 0.0
psoc6 light cy8ckit_062s2_43012 .debug_info 22027216 22027217 1 0.0
Decreases (7 builds for cc13x2_26x2, esp32, nrfconnect, psoc6, qpg, telink)
platform target config section 26e72ab 04b077e change % change
cc13x2_26x2 pump-controller-app LP_CC2652R7 (read/write) 177624 177616 -8 -0.0
esp32 all-clusters-app c3devkit (read/write) 1788126 1788118 -8 -0.0
.flash.rodata 257696 257688 -8 -0.0
nrfconnect all-clusters-minimal-app nrf52840dk_nrf52840 text 803272 803268 -4 -0.0
psoc6 all-clusters cy8ckit_062s2_43012 .debug_info 26826793 26826792 -1 -0.0
lock cy8ckit_062s2_43012 .debug_info 22265887 22265885 -2 -0.0
qpg lighting-app qpg6105+debug (read/write) 1148140 1148132 -8 -0.0
.text 595240 595232 -8 -0.0
telink light-switch-app tlsr9518adk80d text 552776 552774 -2 -0.0
Full report (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
platform target config section 26e72ab 04b077e change % change
bl602 lighting-app bl602 (read/write) 1388826 1388826 0 0.0
.bss 90729 90729 0 0.0
.data 9928 9928 0 0.0
.text 1068428 1068428 0 0.0
bl602+rpc (read/write) 1434038 1434038 0 0.0
.bss 98161 98161 0 0.0
.data 10312 10312 0 0.0
.text 1099776 1099776 0 0.0
bl702 lighting-app bl702 (read only) 3262 3262 0 0.0
(read/write) 1188027 1188027 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 67094 67094 0 0.0
.bss_psram 29696 29696 0 0.0
.comment 48 48 0 0.0
.data 4272 4272 0 0.0
.debug_abbrev 1506897 1506897 0 0.0
.debug_aranges 133088 133088 0 0.0
.debug_frame 486444 486444 0 0.0
.debug_info 37902577 37902577 0 0.0
.debug_line 5255713 5255713 0 0.0
.debug_loc 3366207 3366207 0 0.0
.debug_ranges 359136 359136 0 0.0
.debug_str 3456572 3456572 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 144 144 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 116536 116536 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 564835 564835 0 0.0
.symtab 171584 171584 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 0 0 0 0.0
956624 956624 0 0.0
bl702+rpc (read only) 3262 3262 0 0.0
(read/write) 1283963 1283963 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 75142 75142 0 0.0
.bss_psram 29936 29936 0 0.0
.comment 48 48 0 0.0
.data 4800 4800 0 0.0
.debug_abbrev 1644442 1644442 0 0.0
.debug_aranges 140592 140592 0 0.0
.debug_frame 511820 511820 0 0.0
.debug_info 41809186 41809187 1 0.0
.debug_line 5630248 5630248 0 0.0
.debug_loc 3558839 3558839 0 0.0
.debug_ranges 381592 381592 0 0.0
.debug_str 3852472 3852472 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 160 160 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 129928 129928 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 624008 624008 0 0.0
.symtab 189392 189392 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 0 0 0 0.0
1030324 1030326 2 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 676555 676555 0 0.0
(read/write) 174980 174980 0 0.0
.bss 81228 81228 0 0.0
.data 3380 3380 0 0.0
.rodata 89547 89547 0 0.0
.text 586696 586696 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640803 640803 0 0.0
(read/write) 157996 157996 0 0.0
.bss 80500 80500 0 0.0
.data 3380 3380 0 0.0
.rodata 78683 78683 0 0.0
.text 561800 561800 0 0.0
lock-ftd LP_CC2652R7 (read only) 675239 675239 0 0.0
(read/write) 173448 173448 0 0.0
.bss 78468 78468 0 0.0
.data 3304 3304 0 0.0
.rodata 77071 77071 0 0.0
.text 597688 597688 0 0.0
lock-mtd LP_CC2652R7 (read only) 659091 659091 0 0.0
(read/write) 185284 185284 0 0.0
.bss 74156 74156 0 0.0
.data 3304 3304 0 0.0
.rodata 102939 102939 0 0.0
.text 555672 555672 0 0.0
pump-app LP_CC2652R7 (read only) 687403 687403 0 0.0
(read/write) 162004 162004 0 0.0
.bss 78420 78420 0 0.0
.data 3296 3296 0 0.0
.rodata 90531 90531 0 0.0
.text 596388 596388 0 0.0
pump-controller-app LP_CC2652R7 (read only) 671895 671903 8 0.0
(read/write) 177624 177616 -8 -0.0
.bss 78532 78532 0 0.0
.data 3292 3292 0 0.0
.rodata 86087 86087 0 0.0
.text 585328 585336 8 0.0
shell LP_CC2652R7 (read only) 667606 667606 0 0.0
(read/write) 186240 186240 0 0.0
.bss 83540 83540 0 0.0
.data 3376 3376 0 0.0
.rodata 86262 86262 0 0.0
.text 581028 581028 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 587578 587578 0 0.0
.app_xip_area 464204 464204 0 0.0
.bss 65792 65792 0 0.0
.data 760 760 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 591834 591834 0 0.0
.app_xip_area 463140 463140 0 0.0
.bss 71104 71104 0 0.0
.data 768 768 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 543346 543346 0 0.0
.app_xip_area 425028 425028 0 0.0
.bss 60784 60784 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A+rpc (read/write) 973636 973636 0 0.0
.bss 152236 152236 0 0.0
.data 2248 2248 0 0.0
.text 819132 819132 0 0.0
BRD4161A+rs911x (read/write) 1031000 1031000 0 0.0
.bss 186640 186640 0 0.0
.data 2092 2092 0 0.0
.text 842248 842248 0 0.0
BRD4187C (read/write) 1145924 1145924 0 0.0
.bss 138616 138616 0 0.0
.data 2596 2596 0 0.0
.text 980116 980116 0 0.0
lock-app BRD4161A+wf200 (read/write) 1157200 1157200 0 0.0
.bss 158192 158192 0 0.0
.data 2100 2100 0 0.0
.text 996884 996884 0 0.0
window-app BRD4187C (read/write) 1137924 1137924 0 0.0
.bss 140056 140056 0 0.0
.data 2620 2620 0 0.0
.text 970652 970652 0 0.0
esp32 all-clusters-app c3devkit (read only) 1223088 1223088 0 0.0
(read/write) 1788126 1788118 -8 -0.0
.dram0.bss 76944 76944 0 0.0
.dram0.data 13840 13840 0 0.0
.flash.rodata 257696 257688 -8 -0.0
.flash.text 1223088 1223088 0 0.0
.iram0.text 65204 65204 0 0.0
m5stack (read only) 1233111 1233111 0 0.0
(read/write) 564004 564004 0 0.0
.dram0.bss 82304 82304 0 0.0
.dram0.data 34296 34296 0 0.0
.flash.rodata 314736 314736 0 0.0
.flash.text 1227727 1227727 0 0.0
.iram0.text 123939 123939 0 0.0
k32w light k32w0+release (read/write) 641472 641472 0 0.0
.bss 74816 74816 0 0.0
.data 2064 2064 0 0.0
.text 561864 561864 0 0.0
lock k32w0+release (read/write) 632768 632768 0 0.0
.bss 75592 75592 0 0.0
.data 2080 2080 0 0.0
.text 552368 552368 0 0.0
linux chip-tool-ipv6only arm64 (read only) 10427436 10427436 0 0.0
(read/write) 706401 706401 0 0.0
.bss 33953 33953 0 0.0
.data 2864 2864 0 0.0
.data.rel.ro 650584 650584 0 0.0
.dynamic 560 560 0 0.0
.got 13904 13904 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 517436 517436 0 0.0
.text 8254692 8254692 0 0.0
thermostat-no-ble arm64 (read only) 2388164 2388164 0 0.0
(read/write) 143681 143681 0 0.0
.bss 55361 55361 0 0.0
.data 1912 1912 0 0.0
.data.rel.ro 77208 77208 0 0.0
.dynamic 560 560 0 0.0
.got 5184 5184 0 0.0
.init 24 24 0 0.0
.init_array 440 440 0 0.0
.rodata 143676 143676 0 0.0
.text 2001840 2001840 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2451792 2451792 0 0.0
.bss 215028 215028 0 0.0
.data 5872 5872 0 0.0
.text 1414436 1414436 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1182927 1182927 0 0.0
bss 144433 144433 0 0.0
rodata 144208 144208 0 0.0
text 815376 815376 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1161611 1161611 0 0.0
bss 143660 143660 0 0.0
rodata 135780 135780 0 0.0
text 803272 803268 -4 -0.0
psoc6 all-clusters cy8ckit_062s2_43012 0 0 0 0.0
(read only) 841968 841968 0 0.0
(read/write) 1744436 1744436 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 188712 188712 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1229446 1229446 0 0.0
.debug_aranges 111824 111824 0 0.0
.debug_frame 373368 373368 0 0.0
.debug_info 26826793 26826792 -1 -0.0
.debug_line 3670561 3670561 0 0.0
.debug_loc 3585589 3585589 0 0.0
.debug_ranges 338960 338960 0 0.0
.debug_str 3440244 3440244 0 0.0
.heap 841968 841968 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 569480 569480 0 0.0
.symtab 421056 421056 0 0.0
.text 1544672 1544672 0 0.0
.zero.table 8 8 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 0 0 0 0.0
(read only) 842704 842704 0 0.0
(read/write) 1687044 1687044 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 187976 187976 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1221245 1221245 0 0.0
.debug_aranges 111296 111296 0 0.0
.debug_frame 376448 376448 0 0.0
.debug_info 26563574 26563574 0 0.0
.debug_line 3691277 3691277 0 0.0
.debug_loc 3573226 3573226 0 0.0
.debug_ranges 337576 337576 0 0.0
.debug_str 3429257 3429257 0 0.0
.heap 842704 842704 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 533569 533569 0 0.0
.symtab 407488 407488 0 0.0
.text 1488016 1488016 0 0.0
.zero.table 8 8 0 0.0
light cy8ckit_062s2_43012 0 0 0 0.0
(read only) 850896 850896 0 0.0
(read/write) 1605572 1605572 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 179992 179992 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2456 2456 0 0.0
.debug_abbrev 1055301 1055301 0 0.0
.debug_aranges 103504 103504 0 0.0
.debug_frame 346780 346780 0 0.0
.debug_info 22027216 22027217 1 0.0
.debug_line 3261182 3261182 0 0.0
.debug_loc 3271203 3271203 0 0.0
.debug_ranges 302880 302880 0 0.0
.debug_str 3234789 3234789 0 0.0
.heap 850896 850896 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 469946 469946 0 0.0
.symtab 375936 375936 0 0.0
.text 1414736 1414736 0 0.0
.zero.table 8 8 0 0.0
lock cy8ckit_062s2_43012 0 0 0 0.0
(read only) 845880 845880 0 0.0
(read/write) 1639492 1639492 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 184992 184992 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2472 2472 0 0.0
.debug_abbrev 1057404 1057404 0 0.0
.debug_aranges 103904 103904 0 0.0
.debug_frame 348672 348672 0 0.0
.debug_info 22265887 22265885 -2 -0.0
.debug_line 3259043 3259043 0 0.0
.debug_loc 3299781 3299781 0 0.0
.debug_ranges 304824 304824 0 0.0
.debug_str 3254303 3254303 0 0.0
.heap 845880 845880 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 472353 472353 0 0.0
.symtab 377600 377600 0 0.0
.text 1443640 1443640 0 0.0
.zero.table 8 8 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1148140 1148132 -8 -0.0
.bss 110548 110548 0 0.0
.data 832 832 0 0.0
.text 595240 595232 -8 -0.0
lock-app qpg6105+debug (read/write) 1113204 1113204 0 0.0
.bss 106364 106364 0 0.0
.data 836 836 0 0.0
.text 560300 560300 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 788804 788804 0 0.0
bss 72472 72472 0 0.0
noinit 43520 43520 0 0.0
text 552776 552774 -2 -0.0
lighting-app tlsr9518adk80d (read/write) 810916 810916 0 0.0
bss 73320 73320 0 0.0
noinit 43520 43520 0 0.0
text 570998 570998 0 0.0
ota-requestor-app tlsr9518adk80d (read/write) 818852 818852 0 0.0
bss 74228 74228 0 0.0
noinit 43520 43520 0 0.0
text 577178 577178 0 0.0

@andy31415
Copy link
Contributor Author

Will be included as part of #23017

@andy31415 andy31415 closed this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

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

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

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

@gjc13 gjc13 Awaiting requested review from gjc13

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

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@hawk248 hawk248 Awaiting requested review from hawk248

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@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

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

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

@msandstedt msandstedt Awaiting requested review from msandstedt

@rgoliver rgoliver Awaiting requested review from rgoliver

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@tehampson tehampson Awaiting requested review from tehampson

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@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

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

Assignees
No one assigned
Labels
review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andy31415 @Damian-Nordic