[Silabs] [EFR32] Adds translation between spec WiFi Security Types #25458

rosahay-silabs · 2023-03-03T10:02:57Z

Encoding the enum provided by driver in scan result callback to CHIP spec based enum, decoding them back to driver based enum on the connect API.

github-actions · 2023-03-03T10:18:00Z

PR #25458: Size comparison from af3fdc5 to 8f8544d

Full report (1 build for cc32xx)

platform	target	config	section	`af3fdc5`	`8f8544d`
cc32xx	lock	CC3235SF_LAUNCHXL		0	0
			(read only)	643465	643465
			(read/write)	203688	203688
			.ARM.attributes	44	44
			.ARM.exidx	8	8
			.bss	197088	197088
			.comment	194	194
			.data	1480	1480
			.debug_abbrev	930213	930213
			.debug_aranges	87336	87336
			.debug_frame	300024	300024
			.debug_info	20262928	20262928
			.debug_line	`2657820`	`2657820`
			.debug_loc	2800026	2800026
			.debug_ranges	282240	282240
			.debug_str	3023883	3023883
			.ramVecs	780	780
			.resetVecs	64	64
			.rodata	105929	105929
			.shstrtab	232	232
			.stab	204	204
			.stabstr	441	441
			.stack	2048	2048
			.strtab	378514	378514
			.symtab	256560	256560
			.text	535412	535412

github-actions · 2023-03-03T11:37:55Z

PR #25458: Size comparison from af3fdc5 to f507bca

Full report (1 build for cc32xx)

platform	target	config	section	`af3fdc5`	`f507bca`
cc32xx	lock	CC3235SF_LAUNCHXL		0	0
			(read only)	643465	643465
			(read/write)	203688	203688
			.ARM.attributes	44	44
			.ARM.exidx	8	8
			.bss	197088	197088
			.comment	194	194
			.data	1480	1480
			.debug_abbrev	930213	930213
			.debug_aranges	87336	87336
			.debug_frame	300024	300024
			.debug_info	20262928	20262928
			.debug_line	`2657820`	`2657820`
			.debug_loc	2800026	2800026
			.debug_ranges	282240	282240
			.debug_str	3023883	3023883
			.ramVecs	780	780
			.resetVecs	64	64
			.rodata	105929	105929
			.shstrtab	232	232
			.stab	204	204
			.stabstr	441	441
			.stack	2048	2048
			.strtab	378514	378514
			.symtab	256560	256560
			.text	535412	535412

github-actions · 2023-03-03T12:30:12Z

PR #25458: Size comparison from af3fdc5 to 060b49e

Decreases (1 build for cc32xx)

platform	target	config	section	`af3fdc5`	`060b49e`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20262928	20262927	-1	-0.0

Full report (4 builds for cc32xx, mbed, qpg)

platform	target	config	section	`af3fdc5`	`060b49e`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0	0.0
			(read only)	643465	643465	0	0.0
			(read/write)	203688	203688	0	0.0
			.ARM.attributes	44	44	0	0.0
			.ARM.exidx	8	8	0	0.0
			.bss	197088	197088	0	0.0
			.comment	194	194	0	0.0
			.data	1480	1480	0	0.0
			.debug_abbrev	930213	930213	0	0.0
			.debug_aranges	87336	87336	0	0.0
			.debug_frame	300024	300024	0	0.0
			.debug_info	20262928	20262927	-1	-0.0
			.debug_line	`2657820`	`2657820`	0	0.0
			.debug_loc	2800026	2800026	0	0.0
			.debug_ranges	282240	282240	0	0.0
			.debug_str	3023883	3023883	0	0.0
			.ramVecs	780	780	0	0.0
			.resetVecs	64	64	0	0.0
			.rodata	105929	105929	0	0.0
			.shstrtab	232	232	0	0.0
			.stab	204	204	0	0.0
			.stabstr	441	441	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	378514	378514	0	0.0
			.symtab	256560	256560	0	0.0
			.text	535412	535412	0	0.0
mbed	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	`2467384`	`2467384`	0	0.0
			.bss	215804	215804	0	0.0
			.data	5880	5880	0	0.0
			.text	1430028	1430028	0	0.0
qpg	lighting-app	qpg6105+debug	(read/write)	1151852	1151852	0	0.0
			.bss	99812	99812	0	0.0
			.data	852	852	0	0.0
			.text	598948	598948	0	0.0
	lock-app	qpg6105+debug	(read/write)	`1118892`	`1118892`	0	0.0
			.bss	96292	96292	0	0.0
			.data	864	864	0	0.0
			.text	565992	565992	0	0.0

examples/platform/silabs/SiWx917/SiWx917/rsi_if.c

examples/platform/silabs/efr32/rs911x/rsi_if.c

jmartinez-silabs

Wouldn't it be simpler to just do the conversion in a single location, in the WiFi Diagnostic function
DiagnosticDataProviderImpl::GetWiFiSecurityType ?

Converting your WFX or RSI security enums to the Matter WiFiNetworkDiagnostics::SecurityTypeEnum ?

rosahay-silabs · 2023-03-03T13:46:58Z

Wouldn't it be simpler to just do the conversion in a single location, in the WiFi Diagnostic function DiagnosticDataProviderImpl::GetWiFiSecurityType ?

Converting your WFX or RSI security enums to the Matter WiFiNetworkDiagnostics::SecurityTypeEnum ?

Yes agreed, but that is something I want to pick up without blocking the spec. We have a task to clean up the code here, including the global variables and functions. Including and not limiting only this, it also includes a proper structure to save AP details which seems to be different for WF200 and RS9116, we can remove quite a few duplicate codes with a clean-up. I will also introduce guard style if statement instead of if-else littered everywhere.

jmartinez-silabs · 2023-03-03T13:48:44Z

ok, I'll approve then so you can continue your clean up

github-actions · 2023-03-03T14:00:15Z

PR #25458: Size comparison from af3fdc5 to 9091af6

Full report (1 build for cc32xx)

platform	target	config	section	`af3fdc5`	`9091af6`
cc32xx	lock	CC3235SF_LAUNCHXL		0	0
			(read only)	643465	643465
			(read/write)	203688	203688
			.ARM.attributes	44	44
			.ARM.exidx	8	8
			.bss	197088	197088
			.comment	194	194
			.data	1480	1480
			.debug_abbrev	930213	930213
			.debug_aranges	87336	87336
			.debug_frame	300024	300024
			.debug_info	20262928	20262928
			.debug_line	`2657820`	`2657820`
			.debug_loc	2800026	2800026
			.debug_ranges	282240	282240
			.debug_str	3023883	3023883
			.ramVecs	780	780
			.resetVecs	64	64
			.rodata	105929	105929
			.shstrtab	232	232
			.stab	204	204
			.stabstr	441	441
			.stack	2048	2048
			.strtab	378514	378514
			.symtab	256560	256560
			.text	535412	535412

examples/platform/silabs/efr32/rs911x/rsi_if.c

jmeg-sfy

sound like a bit of factorization improvement is needed
unless you wish to copy/paste everything again when SiWx918 then SiWx920 comes up....

brosahay · 2023-03-03T16:09:58Z

sound like a bit of factorization improvement is needed unless you wish to copy/paste everything again when SiWx918 then SiWx920 comes up....

Yes, agreed that we are replicating code for now, and a refactor effort is required. This was done to meet the spec requirement without changing the logic much, the refactor effort will be done in a phased manner since it requires quite a good amount of testing.

Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>

github-actions · 2023-03-06T04:50:41Z

PR #25458: Size comparison from 32fb896 to edecd39

Increases (1 build for cc32xx)

platform	target	config	section	`32fb896`	`edecd39`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20262927	20262929	2	0.0

Full report (2 builds for cc32xx, mbed)

platform	target	config	section	`32fb896`	`edecd39`	change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0
			(read only)	643465	643465	0
			(read/write)	203688	203688	0
			.ARM.attributes	44	44	0
			.ARM.exidx	8	8	0
			.bss	197088	197088	0
			.comment	194	194	0
			.data	1480	1480	0
			.debug_abbrev	930213	930213	0
			.debug_aranges	87336	87336	0
			.debug_frame	300024	300024	0
			.debug_info	20262927	20262929	2
			.debug_line	`2657822`	`2657822`	0
			.debug_loc	2800026	2800026	0
			.debug_ranges	282240	282240	0
			.debug_str	3023883	3023883	0
			.ramVecs	780	780	0
			.resetVecs	64	64	0
			.rodata	105929	105929	0
			.shstrtab	232	232	0
			.stab	204	204	0
			.stabstr	441	441	0
			.stack	2048	2048	0
			.strtab	378514	378514	0
			.symtab	256560	256560	0
			.text	535412	535412	0
mbed	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	`2467384`	`2467384`	0
			.bss	215804	215804	0
			.data	5880	5880	0
			.text	1430028	1430028	0

bzbarsky-apple · 2023-03-06T21:13:56Z

src/platform/silabs/SiWx917/wifi/wfx_host_events.h

@@ -149,28 +148,19 @@ typedef enum
 /* Note that these are same as RSI_security */


Are these in fact still the same as RSI_security?

But also, is there a reason this is not just using the spec-defined enum?

now, they are not same as rsi_security, it was a comment which I plan to clean up during the cleanup activity.

bzbarsky-apple · 2023-03-06T21:17:09Z

src/platform/silabs/efr32/wifi/wfx_host_events.h

@@ -274,7 +264,7 @@ typedef enum
 typedef struct wfx_wifi_scan_result
 {
    char ssid[32 + 1];
-    uint8_t security;
+    wfx_sec_t security;


This does not look right. For a scan result, the security type is a bitmap, not an enum, in the spec. See "11.8.5.1. WiFiSecurityBitmap", which is not the same thing as the "11.14.5.1. SecurityTypeEnum" used by network diagnostics. It looks to me like the enums above match the latter, but not the scan result bitmap.

The translation for this is here src/platform/silabs/NetworkCommissioningWiFiDriver.cpp#L197

I don't see how that can work. That ConvertSecuritytype function expects a bitmap as input (not the bit test it's doing) and converts to a different bitmap. But wfx_sec_t is not a bitmap....

…roject-chip#25458) * Adds fix for project-chip#25118 * Adds fix for unused-variable-warning * Adds fix for rsi_wlan_connect_async invoke * Adding break statements for default case. * Update examples/platform/silabs/efr32/rs911x/rsi_if.c Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com> --------- Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>

github-actions bot added examples platform labels Mar 3, 2023

rosahay-silabs force-pushed the bugfix/efr32-wifi-sec-type-spec branch from 5860517 to 8f8544d Compare March 3, 2023 10:03

rosahay-silabs marked this pull request as ready for review March 3, 2023 13:06

pullapprove bot requested review from vivien-apple, woody-apple and xylophone21 March 3, 2023 13:06

pullapprove bot added the review - pending label Mar 3, 2023

mkardous-silabs reviewed Mar 3, 2023

View reviewed changes

examples/platform/silabs/SiWx917/SiWx917/rsi_if.c Show resolved Hide resolved

examples/platform/silabs/efr32/rs911x/rsi_if.c Show resolved Hide resolved

jmartinez-silabs reviewed Mar 3, 2023

View reviewed changes

jmartinez-silabs approved these changes Mar 3, 2023

View reviewed changes

jmeg-sfy reviewed Mar 3, 2023

View reviewed changes

examples/platform/silabs/efr32/rs911x/rsi_if.c Outdated Show resolved Hide resolved

jmeg-sfy reviewed Mar 3, 2023

View reviewed changes

examples/platform/silabs/efr32/rs911x/rsi_if.c Show resolved Hide resolved

jmeg-sfy reviewed Mar 3, 2023

View reviewed changes

examples/platform/silabs/efr32/rs911x/rsi_if.c Show resolved Hide resolved

jmeg-sfy approved these changes Mar 3, 2023

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Mar 3, 2023

rosahay-silabs and others added 5 commits March 6, 2023 10:00

Adds fix for project-chip#25118

d4cb92b

Adds fix for unused-variable-warning

52eaae0

Adds fix for rsi_wlan_connect_async invoke

1013767

Adding break statements for default case.

dfa6084

Update examples/platform/silabs/efr32/rs911x/rsi_if.c

edecd39

Co-authored-by: Jonathan Mégevand <77852424+jmeg-sfy@users.noreply.github.com>

rosahay-silabs force-pushed the bugfix/efr32-wifi-sec-type-spec branch from 9b0455d to edecd39 Compare March 6, 2023 04:30

jmartinez-silabs enabled auto-merge (squash) March 6, 2023 13:04

jmartinez-silabs merged commit 7e7b7af into project-chip:master Mar 6, 2023

rosahay-silabs deleted the bugfix/efr32-wifi-sec-type-spec branch March 6, 2023 17:27

bzbarsky-apple reviewed Mar 6, 2023

View reviewed changes

bzbarsky-apple mentioned this pull request Mar 6, 2023

[Platform][Silabs][WIFI] Security types still do not match spec #25500

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Silabs] [EFR32] Adds translation between spec WiFi Security Types #25458

[Silabs] [EFR32] Adds translation between spec WiFi Security Types #25458

rosahay-silabs commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

jmartinez-silabs left a comment •

edited

Loading

rosahay-silabs commented Mar 3, 2023

jmartinez-silabs commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

jmeg-sfy left a comment

brosahay commented Mar 3, 2023

github-actions bot commented Mar 6, 2023

bzbarsky-apple Mar 6, 2023

rosahay-silabs Mar 7, 2023

bzbarsky-apple Mar 6, 2023

rosahay-silabs Mar 7, 2023

bzbarsky-apple Mar 7, 2023

		@@ -149,28 +148,19 @@ typedef enum
		/* Note that these are same as RSI_security */

[Silabs] [EFR32] Adds translation between spec WiFi Security Types #25458

[Silabs] [EFR32] Adds translation between spec WiFi Security Types #25458

Conversation

rosahay-silabs commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

jmartinez-silabs left a comment • edited Loading

Choose a reason for hiding this comment

rosahay-silabs commented Mar 3, 2023

jmartinez-silabs commented Mar 3, 2023

github-actions bot commented Mar 3, 2023

jmeg-sfy left a comment

Choose a reason for hiding this comment

brosahay commented Mar 3, 2023

github-actions bot commented Mar 6, 2023

bzbarsky-apple Mar 6, 2023

Choose a reason for hiding this comment

rosahay-silabs Mar 7, 2023

Choose a reason for hiding this comment

bzbarsky-apple Mar 6, 2023

Choose a reason for hiding this comment

rosahay-silabs Mar 7, 2023

Choose a reason for hiding this comment

bzbarsky-apple Mar 7, 2023

Choose a reason for hiding this comment

jmartinez-silabs left a comment •

edited

Loading