-
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
[Silabs] Continue WifiInterface headers clean up #37107
base: master
Are you sure you want to change the base?
[Silabs] Continue WifiInterface headers clean up #37107
Conversation
PR #37107: Size comparison from 4fd7215 to 8be646b Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
@@ -261,7 +261,7 @@ CHIP_ERROR BaseApplication::Init() | |||
* Wait for the WiFi to be initialized | |||
*/ | |||
ChipLogProgress(AppServer, "APP: Wait WiFi Init"); | |||
while (!wfx_hw_ready()) | |||
while (!IsHardwareReady()) |
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.
without the class name, it's not evident what hardware we are waiting on. In mind, it also only makes sense that we are waiting on hardware setup for ncp/rcp cases. Do you know what we are waiting on for 917 soc case?
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.
I think the name is deceiving - we aren't really waiting for the hardware. We are waiting for the Wifi init to have completed. I'll renamed the function.
* @param btn which button was pressed | ||
* @param btnAction the action that triggered the buttone vent | ||
*/ | ||
void sl_button_on_change(uint8_t btn, uint8_t btnAction); |
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.
not a strong ask, but this should probably be on the wifi SPAM section. This comes from sisdk for efr32
@@ -830,20 +742,19 @@ void ProcessEvent(WifiPlatformEvent event) | |||
* None | |||
**********************************************************************************/ | |||
/* ARGSUSED */ | |||
void sl_matter_wifi_task(void * arg) | |||
void MatterWifiTask(void * arg) |
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 keep the SL prefix in there?
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.
I don't think so - it is only used on the Silabs directories and it will be in a Silabs namespace in a follow up PR.
} | ||
} | ||
// TODO: We need to clean up this casting for the extended information | ||
rsi_wlan_ext_stats_t * test = (rsi_wlan_ext_stats_t *) buff; |
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.
imo this should all be the other way around
buff (or a new name) should be of type rsi_wlan_ext_stats_t
. and casted a a uint8_t*
for rsi_wlan_get
Ideally rsi_wlan_get
API gets changed to expect a rsi_wlan_ext_stats_t pointer or reference type.
ChipLogError(DeviceLayer, "Failed, Error Code : 0x%lX", status)); | ||
|
||
// TODO: We need to clean up this casting for the extended information | ||
rsi_wlan_ext_stats_t * test = (rsi_wlan_ext_stats_t *) buff; |
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.
same comment for this
Description
The goal of the PR is to continue the clean up of the WifiInterface header files aiming for a clean interface. The main focus was finishing the first version of the WiseconnectWifiInterface header.
wifi_extra
member and thesl_wfx_context
state.PR adds several TODOs that will be addressed in a follow since they require the class inheritance structure and to avoid growing this PR out of hand.
Testing