Skip to content

[DNSSD] Prevent failure when Dnssd::ServiceAdvertiser is not initialized#42994

Open
jepenven-silabs wants to merge 5 commits into
project-chip:masterfrom
jepenven-silabs:dnssd_bootup_error_cleanup
Open

[DNSSD] Prevent failure when Dnssd::ServiceAdvertiser is not initialized#42994
jepenven-silabs wants to merge 5 commits into
project-chip:masterfrom
jepenven-silabs:dnssd_bootup_error_cleanup

Conversation

@jepenven-silabs
Copy link
Copy Markdown
Contributor

Summary

Upon booting a non-commissionned device DNSSD::StartServer is called twice triggering 6 logs of errors.

These errors comes from the fact that calling the Dnssd::ServiceAdvertiser::Instance().Init() functions does not put the ServiceAdvertiser in the Initialized state but in the Initializing state cause subsequent function to fail automatically when they check if the state is Initialized. This PR prevent this failure by doing the check before calling said functions.

Related issues

Relates to #13844 as this is is more of a workaround to alleviate the symptoms of a confusing Init sequence for the DNSSD.

Testing

Tested with a Silicon Labs EFR32MG24 light-switch app. Errors logs do not show up with this fix and the device is able to advertise and commission as expected.

Upon Power-cycle, the device is still able to advertise as expected

Copilot AI review requested due to automatic review settings February 6, 2026 12:53
Copy link
Copy Markdown
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 addresses spurious error logging that occurs when DNSSD::StartServer is called on non-commissioned devices. The root cause is that ServiceAdvertiser::Instance().Init() transitions to an Initializing state rather than Initialized, causing subsequent operations to fail their state checks. The fix adds conditional checks using IsInitialized() before calling operations that require full initialization.

Changes:

  • Added IsInitialized() guards around RemoveServices(), AdvertiseCommissionableNode(), and FinalizeServiceUpdate() calls
  • Changed log level from ChipLogProgress to ChipLogDetail for commission parameter advertising

Comment thread src/app/server/Dnssd.cpp
@jepenven-silabs jepenven-silabs force-pushed the dnssd_bootup_error_cleanup branch from 6584829 to 66efbd0 Compare February 6, 2026 12:55
Copy link
Copy Markdown
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 aims to prevent error logs when Dnssd::ServiceAdvertiser is not yet initialized by adding checks before calling its methods. While this is a good step, the current implementation is incomplete, as several other calls that depend on the advertiser are still made without these checks. I've provided a suggestion for a more comprehensive and cleaner fix that wraps all dependent calls in a single initialization check, aligning with the principle of ensuring robust initialization and preventing unstable states. This will ensure all potential errors are caught and improves the code's readability and robustness.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 2026

PR #42994: Size comparison from 6d8d7f7 to 66efbd0

Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
platform target config section 6d8d7f7 66efbd0 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1106672 1106536 -136 -0.0
RAM 179058 179058 0 0.0
bl702 lighting-app bl702+eth FLASH 661392 661256 -136 -0.0
RAM 135081 135081 0 0.0
bl702+wifi FLASH 837294 837158 -136 -0.0
RAM 124597 124597 0 0.0
bl706+mfd+rpc+littlefs FLASH 1071542 1071406 -136 -0.0
RAM 117501 117501 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 903996 903860 -136 -0.0
RAM 106076 106076 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 984444 984308 -136 -0.0
RAM 109988 109988 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 771704 771936 232 0.0
RAM 103504 103504 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 784732 784964 232 0.0
RAM 108824 108824 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 729796 729828 32 0.0
RAM 97580 97580 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 714340 714372 32 0.0
RAM 97780 97780 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 555372 555408 36 0.0
RAM 205528 205528 0 0.0
lock CC3235SF_LAUNCHXL FLASH 589520 589556 36 0.0
RAM 205816 205816 0 0.0
efr32 lock-app BRD4187C FLASH 967928 967760 -168 -0.0
RAM 125796 125796 0 0.0
BRD4338a FLASH 761324 761364 40 0.0
RAM 238072 238072 0 0.0
window-app BRD4187C FLASH 1064576 1064600 24 0.0
RAM 126920 126920 0 0.0
esp32 all-clusters-app c3devkit DRAM 98340 98340 0 0.0
FLASH 1586730 1586556 -174 -0.0
IRAM 93514 93514 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 937652 937688 36 0.0
RAM 162127 162127 0 0.0
nxp contact mcxw71+release FLASH 745376 745432 56 0.0
RAM 67240 67240 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1698204 1698252 48 0.0
RAM 214636 214636 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1599500 1599548 48 0.0
RAM 211620 211620 0 0.0
light cy8ckit_062s2_43012 FLASH 1461076 1461124 48 0.0
RAM 197904 197904 0 0.0
lock cy8ckit_062s2_43012 FLASH 1495132 1495180 48 0.0
RAM 225784 225784 0 0.0
qpg lighting-app qpg6200+debug FLASH 839284 839124 -160 -0.0
RAM 128112 128112 0 0.0
lock-app qpg6200+debug FLASH 777500 777356 -144 -0.0
RAM 119052 119052 0 0.0
realtek light-switch-app rtl8777g FLASH 702320 702352 32 0.0
RAM 113652 113652 0 0.0
lighting-app rtl8777g FLASH 744240 744280 40 0.0
RAM 114864 114864 0 0.0
stm32 light STM32WB5MM-DK FLASH 471224 471068 -156 -0.0
RAM 141512 141512 0 0.0
telink bridge-app tl7218x FLASH 712454 712490 36 0.0
RAM 93184 93184 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 798024 798060 36 0.0
RAM 41368 41368 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 789360 789396 36 0.0
RAM 96268 96268 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 717990 718026 36 0.0
RAM 54360 54360 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 753800 753836 36 0.0
RAM 73384 73384 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 722366 722402 36 0.0
RAM 35088 35088 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 610354 610390 36 0.0
RAM 118056 118056 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 822864 822904 40 0.0
RAM 94656 94656 0 0.0

Comment thread src/app/server/Dnssd.cpp Outdated
Comment thread src/app/server/Dnssd.cpp Outdated
@jepenven-silabs jepenven-silabs force-pushed the dnssd_bootup_error_cleanup branch from 66efbd0 to d040112 Compare June 3, 2026 20:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.57%. Comparing base (652855f) to head (0378c0b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/app/server/Dnssd.cpp 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42994      +/-   ##
==========================================
- Coverage   55.57%   55.57%   -0.01%     
==========================================
  Files        1630     1630              
  Lines      111223   111228       +5     
  Branches    13409    13411       +2     
==========================================
+ Hits        61812    61814       +2     
- Misses      49411    49414       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

PR #42994: Size comparison from 9cf1485 to 0378c0b

Full report (21 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
platform target config section 9cf1485 0378c0b change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1094324 1094202 -122 -0.0
RAM 144882 144882 0 0.0
bl616 lighting-app bl616+thread FLASH 1105636 1105516 -120 -0.0
RAM 104280 104280 0 0.0
bl616+wifi+shell FLASH 1593424 1593304 -120 -0.0
RAM 98176 98176 0 0.0
bl702 lighting-app bl702+eth FLASH 1057326 1057204 -122 -0.0
RAM 108509 108509 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 896162 896214 52 0.0
RAM 105884 105884 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 776976 777224 248 0.0
RAM 103388 103388 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 789744 789984 240 0.0
RAM 108676 108676 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 738868 738916 48 0.0
RAM 97596 97596 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 719032 719080 48 0.0
RAM 97636 97636 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 568818 568874 56 0.0
RAM 205056 205056 0 0.0
lock CC3235SF_LAUNCHXL FLASH 596298 596354 56 0.0
RAM 205256 205256 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 834276 834320 44 0.0
RAM 157540 157540 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1733676 1733724 48 0.0
RAM 215260 215260 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1622756 1622804 48 0.0
RAM 211548 211548 0 0.0
light cy8ckit_062s2_43012 FLASH 1469988 1470028 40 0.0
RAM 197420 197420 0 0.0
lock cy8ckit_062s2_43012 FLASH 1503428 1503476 48 0.0
RAM 225252 225252 0 0.0
qpg lighting-app qpg6200+debug FLASH 844200 844060 -140 -0.0
RAM 127948 127948 0 0.0
lock-app qpg6200+debug FLASH 782276 782120 -156 -0.0
RAM 118856 118856 0 0.0
realtek light-switch-app rtl8777g FLASH 688760 688808 48 0.0
RAM 101764 101764 0 0.0
lighting-app rtl8777g FLASH 729712 729760 48 0.0
RAM 102044 102044 0 0.0
stm32 light STM32WB5MM-DK FLASH 478416 478268 -148 -0.0
RAM 141476 141476 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.

7 participants