Skip to content

Conversation

@andy31415
Copy link
Contributor

Summary

This is based on
#42200 (review)
Although modeselect uses this method in startup, it is not used only in startup .... so keeping the name the same.

Testing

just API name change ... no functionality behavior (so CI still applies)

This is based on 

project-chip#42200 (review)

Although modeselect uses this method in startup, it is not used only in startup .... so keeping the name the same.
Copilot AI review requested due to automatic review settings December 2, 2025 15:45
@github-actions github-actions bot added the app label Dec 2, 2025
@pullapprove pullapprove bot removed the app label Dec 2, 2025
Copilot finished reviewing on behalf of andy31415 December 2, 2025 15:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR renames the SetStartupOnMode function to UpdateCurrentModeToOnMode in the ModeSelect integration to align with the naming convention already used in ModeBase integration. The change is purely a refactoring to improve naming consistency across the codebase.

Key changes:

  • Renamed SetStartupOnMode to UpdateCurrentModeToOnMode in ModeSelect integration
  • Updated all call sites in the on-off-server implementation
  • Minor formatting adjustment to constructor initialization list

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
mode-select-integration.h Updated function declaration and inline stub to use the new name UpdateCurrentModeToOnMode
mode-select-integration.cpp Updated function implementation to use the new name UpdateCurrentModeToOnMode
on-off-server.cpp Updated all call sites to use the new function name and minor formatting adjustment to constructor initialization list

@github-actions github-actions bot added the app label Dec 2, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the naming of a function in the ModeSelect integration to be more consistent with a similar function in ModeBase integration. The function SetStartupOnMode has been renamed to UpdateCurrentModeToOnMode, which more accurately reflects its usage beyond just startup scenarios. The changes are applied consistently across the function's declaration, definition, and call sites. This is a good improvement for code clarity and consistency.

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

PR #42237: Size comparison from bee7cc7 to 13ab1f5

Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
platform target config section bee7cc7 13ab1f5 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1106692 1106692 0 0.0
RAM 178954 178954 0 0.0
bl702 lighting-app bl702+eth FLASH 661510 661510 0 0.0
RAM 135041 135041 0 0.0
bl702+wifi FLASH 837360 837360 0 0.0
RAM 124477 124477 0 0.0
bl706+mfd+rpc+littlefs FLASH 1071012 1071012 0 0.0
RAM 117349 117349 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 903854 903854 0 0.0
RAM 105900 105900 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 983758 983758 0 0.0
RAM 109828 109828 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 771488 771488 0 0.0
RAM 103392 103392 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 784292 784292 0 0.0
RAM 108712 108712 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 729340 729340 0 0.0
RAM 97452 97452 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 713800 713800 0 0.0
RAM 97660 97660 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 555302 555302 0 0.0
RAM 205808 205808 0 0.0
lock CC3235SF_LAUNCHXL FLASH 589250 589250 0 0.0
RAM 206064 206064 0 0.0
efr32 lock-app BRD4187C FLASH 965348 965348 0 0.0
RAM 123764 123764 0 0.0
BRD4338a FLASH 759892 759892 0 0.0
RAM 254372 254372 0 0.0
window-app BRD4187C FLASH 1060712 1060712 0 0.0
RAM 119992 119992 0 0.0
esp32 all-clusters-app c3devkit DRAM 102772 102772 0 0.0
FLASH 1839700 1839700 0 0.0
IRAM 93540 93540 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 937424 937424 0 0.0
RAM 161576 161576 0 0.0
nxp contact mcxw71+release FLASH 695024 695024 0 0.0
RAM 61736 61736 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1683052 1683052 0 0.0
RAM 214156 214156 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1597372 1597372 0 0.0
RAM 211332 211332 0 0.0
light cy8ckit_062s2_43012 FLASH 1460964 1460964 0 0.0
RAM 197808 197808 0 0.0
lock cy8ckit_062s2_43012 FLASH 1494908 1494908 0 0.0
RAM 225680 225680 0 0.0
qpg lighting-app qpg6200+debug FLASH 839396 839396 0 0.0
RAM 127952 127952 0 0.0
lock-app qpg6200+debug FLASH 776664 776664 0 0.0
RAM 118912 118912 0 0.0
realtek light-switch-app rtl8777g FLASH 709552 709552 0 0.0
RAM 107180 107180 0 0.0
lighting-app rtl8777g FLASH 758264 758264 0 0.0
RAM 127320 127320 0 0.0
stm32 light STM32WB5MM-DK FLASH 470924 470924 0 0.0
RAM 141384 141384 0 0.0
telink bridge-app tl7218x FLASH 704440 704440 0 0.0
RAM 90636 90636 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 790740 790740 0 0.0
RAM 41052 41052 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 782114 782114 0 0.0
RAM 93736 93736 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 710592 710592 0 0.0
RAM 52108 52108 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 746424 746424 0 0.0
RAM 71132 71132 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 721034 721034 0 0.0
RAM 34832 34832 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 602938 602938 0 0.0
RAM 117452 117452 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 815616 815620 4 0.0
RAM 92124 92124 0 0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants