[Silabs] Adds fix for BTN low power mode for SiWx917 SoC and power manager #35808
+58
−34
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
|
Review changes with SemanticDiff. Analyzed 1 of 6 files.
|
rosahay-silabs
force-pushed
the
bugfix/siwx917-sleep-btn0-fix
branch
from
5d3dda5
to
8af7b11
Compare
|
PR #35808: Size comparison from a6f44fd to 8af7b11 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35808: Size comparison from a6f44fd to f0b4baa Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
rosahay-silabs
force-pushed
the
bugfix/siwx917-sleep-btn0-fix
branch
from
465c040
to
4e1d918
Compare
|
PR #35808: Size comparison from af1b0ee to 4e1d918 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jmartinez-silabs
added
the
changing-submodules-on-purpose
rosahay-silabs
force-pushed
the
bugfix/siwx917-sleep-btn0-fix
branch
from
4e1d918
to
5be0a39
Compare
|
PR #35808: Size comparison from 579b1b1 to 5be0a39 Full report (82 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35808: Size comparison from d7abcbf to ccd1897 Full report (65 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
| @@ -262,23 +260,36 @@ sl_status_t join_callback_handler(sl_wifi_event_t event, char * result, uint32_t | |||
|
|
|||
| #if CHIP_CONFIG_ENABLE_ICD_SERVER | |||
| #if SLI_SI91X_MCU_INTERFACE | |||
| // Required to invoke button press event during sleep as falling edge is not detected | |||
| void sl_si91x_invoke_btn_press_event(void) | |||
| // This function is called when the button is pressed when in sleep then the idleTask is handling the button press event | |||
There was a problem hiding this comment.
Can you adapt these comments to the appropriate format?
This comment applies to all functions.
/**
* @brief invoked when enhanced connection complete event is received
* @param[out] resp_conn, connected remote device information
* @return none.
*/
Can you add to the comment that the signature is important since it is a weak function.
There was a problem hiding this comment.
Again, resolved but not addressed.
Comment on lines
+109
to
+112
| void sl_si91x_btn_event_handler(); | ||
| void sl_si91x_uart_power_requirement_handler(); | ||
| // this is callback from the Wiseconnect SDK | ||
| void gpio_uulp_pin_interrupt_callback(uint32_t pin_intr); |
There was a problem hiding this comment.
These functions seem to be specific to the 917 but are in a header file that is common to all wi-fi platforms.
Can we start moving these function out of the "global" header?
There was a problem hiding this comment.
@mkardous-silabs , do you suggest keeping something like sl_si91x_hal.h instead? Since we do not have anything like that now on this.
There was a problem hiding this comment.
Call it SiWxPlatformInterface.h for now and we can see later its finale name.
Put these functions inside a SiWxPlatform namespace.
| @@ -66,7 +66,7 @@ namespace Silabs { | |||
| namespace { | |||
| uint8_t sButtonStates[SL_SI91x_BUTTON_COUNT] = { 0 }; | |||
| #if CHIP_CONFIG_ENABLE_ICD_SERVER | |||
| bool btn0_pressed = false; | |||
| bool sl_btn0_pressed = false; | |||
There was a problem hiding this comment.
I'm not sure i see the benefit of renaming this variable with sl_ when it is defined in a silabs specific file.
There was a problem hiding this comment.
it is still being used in the global anonymous namespace right? and has the possibility to collide in future right?
There was a problem hiding this comment.
global anonymous namespace does not exist. There are only three types or namespaces : global, named and anounymous. When something is in an anonymous namespace, it limits the scope of the element to the file were it was declared.
In this case, the only way for a collission, would be add a second variable btn0Pressed in the .cpp.
Comment on lines
+154
to
+162
| else if ((btnAction == BUTTON_PRESSED) && (sl_btn0_pressed == true)) | ||
| { | ||
| // if the btn was already pressed and another press event came, ignore it | ||
| return; | ||
| } | ||
| else if ((btnAction == BUTTON_RELEASED) && (btn0_pressed == true)) | ||
| else if ((btnAction == BUTTON_RELEASED) && (sl_btn0_pressed == false)) | ||
| { | ||
| btn0_pressed = false; | ||
| // if the btn was not pressed and only a release event came, ignore it | ||
| return; |
There was a problem hiding this comment.
These conditions should be added at the start of the function with the VerifyOrReturn structure.
There was a problem hiding this comment.
@rosahay-silabs This marked resolved but was not addressed nor commented.
third_party/silabs/matter_support
Outdated
There was a problem hiding this comment.
There a multiple PRs updating the matter_support submodule, we need to make sure that we don't cause ourselves a incorrect hash based on the merged order.
|
PR #35808: Size comparison from 4269ff5 to a91f0af Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
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.
After integrating the power manager on the SiWx917 SoC, sometimes when the BTN 0 is pressed there is no change in the QR code and there is a delay in factory reset as well i.e., taking more than 5 seconds to factory reset the device.