Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[DNS-SD] Refresh services on RemoveFabric command #10499

Merged
merged 1 commit into from
Oct 15, 2021
Merged

[DNS-SD] Refresh services on RemoveFabric command #10499

merged 1 commit into from
Oct 15, 2021

Conversation

Damian-Nordic
Copy link
Contributor

Problem

AddNOC and UpdateNOC commands both cause refreshing DNS-SD services, but RemoveFabric does not remove the obsolete
operational node service.

Change overview

Refresh DNS-SD services on RemoveFabric command from OperationalCredentials cluster.

Testing

Tested manually using nRF Connect examples.

Fixes #10491 although not sure if it's still possible to cherry-pick patches to the TE6 branch...

@Damian-Nordic
[DNS-SD] Refresh services on RemoveFabric command
8d3c4e8 
AddNOC and UpdateNOC commands both cause refreshing DNS-SD
services, but RemoveFabric does not remove the obsolete
operational node service.
@boring-cyborg boring-cyborg bot added the app label Oct 14, 2021
@github-actions
Copy link

PR #10499: Size comparison from 267951c to 8d3c4e8

34 builds
platform target config section 267951c 8d3c4e8 change % change
efr32 lighting-app BRD4161A .bss 118004 118004 0 0.0
.data 1800 1800 0 0.0
.text 780924 780924 0 0.0
lock-app BRD4161A .bss 115876 115876 0 0.0
.data 1760 1760 0 0.0
.text 760172 760188 16 0.0
window-app BRD4161A .bss 116196 116196 0 0.0
.data 1764 1764 0 0.0
.text 761088 761104 16 0.0
lighting-app BRD4161A+rpc .bss 131332 131332 0 0.0
.data 1852 1852 0 0.0
.text 760656 760656 0 0.0
esp32 all-clusters-app c3devkit .dram0.bss 60248 60248 0 0.0
.dram0.data 16232 16232 0 0.0
.flash.rodata 198144 198144 0 0.0
.flash.text 868962 868982 20 0.0
.iram0.text 57330 57330 0 0.0
m5stack .dram0.bss 62752 62752 0 0.0
.dram0.data 32084 32084 0 0.0
.flash.rodata 206732 206732 0 0.0
.flash.text 900003 900015 12 0.0
.iram0.text 125115 125115 0 0.0
k32w lock-app k32w061+debug .bss 69036 69036 0 0.0
.data 1864 1864 0 0.0
.text 514056 514072 16 0.0
shell k32w061+debug .bss 55080 55080 0 0.0
.data 672 672 0 0.0
.text 356808 356808 0 0.0
lighting-app k32w061+se05x+release .bss 78552 78552 0 0.0
.data 1900 1900 0 0.0
.text 612732 612748 16 0.0
linux all-clusters-app debug .bss 52144 52144 0 0.0
.data 978 978 0 0.0
.data.rel.ro 58352 58352 0 0.0
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 512 0 0.0
.rodata 134773 134773 0 0.0
.text 1318226 1318242 16 0.0
chip-tool debug .bss 17584 17584 0 0.0
.data 1584 1584 0 0.0
.data.rel.ro 77392 77392 0 0.0
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 416 0 0.0
.rodata 173664 173664 0 0.0
.text 2425109 2425109 0 0.0
ota-provider-app debug .bss 37472 37472 0 0.0
.data 752 752 0 0.0
.data.rel.ro 23112 23112 0 0.0
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 108824 108824 0 0.0
.text 1005410 1005426 16 0.0
ota-requestor-app debug .bss 205728 205728 0 0.0
.data 752 752 0 0.0
.data.rel.ro 24408 24408 0 0.0
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 520 520 0 0.0
.rodata 126864 126864 0 0.0
.text 1125042 1125058 16 0.0
shell debug .bss 16104 16104 0 0.0
.data 242 242 0 0.0
.data.rel.ro 35184 35184 0 0.0
.dynamic 592 592 0 0.0
.got 3504 3504 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 71343 71343 0 0.0
.text 574322 574322 0 0.0
tv-app debug .bss 216368 216368 0 0.0
.data 2032 2032 0 0.0
.data.rel.ro 55536 55536 0 0.0
.dynamic 592 592 0 0.0
.got 4408 4408 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 150536 150536 0 0.0
.text 1463730 1463746 16 0.0
bridge-app debug+rpc .bss 52880 52880 0 0.0
.data 976 976 0 0.0
.data.rel.ro 25480 25480 0 0.0
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 109492 109492 0 0.0
.text 1047493 1047509 16 0.0
lighting-app debug+rpc .bss 42200 42200 0 0.0
.data 1106 1106 0 0.0
.data.rel.ro 52192 52192 0 0.0
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 126641 126641 0 0.0
.text 1248642 1248674 32 0.0
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172132 172132 0 0.0
.data 5464 5464 0 0.0
.heap 858848 858848 0 0.0
.text 1218656 1218656 0 0.0
lock-app CY8CPROTO_062_4343W+release .bss 171068 171068 0 0.0
.data 5432 5432 0 0.0
.heap 859944 859944 0 0.0
.text 1196648 1196648 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 bss 112304 112304 0 0.0
rodata 97160 97160 0 0.0
text 576856 576872 16 0.0
lock-app nrf52840dk_nrf52840 bss 111336 111336 0 0.0
rodata 93656 93656 0 0.0
text 558300 558316 16 0.0
pigweed-app nrf52840dk_nrf52840 bss 51772 51772 0 0.0
rodata 45772 45772 0 0.0
text 339392 339392 0 0.0
pump-app nrf52840dk_nrf52840 bss 111404 111404 0 0.0
rodata 94640 94640 0 0.0
text 561468 561484 16 0.0
pump-controller-app nrf52840dk_nrf52840 bss 111344 111344 0 0.0
rodata 93716 93716 0 0.0
text 558076 558092 16 0.0
shell nrf52840dk_nrf52840 bss 107320 107320 0 0.0
rodata 71336 71336 0 0.0
text 518964 518964 0 0.0
lighting-app nrf52840dk_nrf52840+rpc bss 108544 108544 0 0.0
rodata 87940 87940 0 0.0
text 550060 550076 16 0.0
nrf5340dk_nrf5340_cpuapp bss 113676 113676 0 0.0
rodata 92400 92400 0 0.0
text 506316 506332 16 0.0
lock-app nrf5340dk_nrf5340_cpuapp bss 112708 112708 0 0.0
rodata 88916 88916 0 0.0
text 487752 487768 16 0.0
shell nrf5340dk_nrf5340_cpuapp bss 108304 108304 0 0.0
rodata 65980 65980 0 0.0
text 439564 439564 0 0.0
p6 lock-app default .bss 68208 68208 0 0.0
.data 2416 2416 0 0.0
.heap 962720 962720 0 0.0
.text 1124840 1124856 16 0.0
qpg lighting-app qpg6100+debug .bss 53520 53520 0 0.0
.data 996 996 0 0.0
.text 485252 485268 16 0.0
lock-app qpg6100+debug .bss 52472 52472 0 0.0
.data 952 952 0 0.0
.text 461436 461452 16 0.0
persistent-storage-app qpg6100+debug .bss 17778 17778 0 0.0
.data 280 280 0 0.0
.text 102704 102704 0 0.0
telink lighting-app tlsr9518adk80d bss 70972 70972 0 0.0
noinit 33216 33216 0 0.0
text 457428 457450 22 0.0

@arunbharadwaj
Copy link
Contributor

arunbharadwaj commented Oct 14, 2021 •
edited
Loading

I tried testing using this PR. I am using chip-tool and esp32 for my testing. It's not quite working as expected. From chip-tool I send chip-tool operationalcredentials remove-fabric 1 0 but on the esp32 side I see this error:

I (58669) chip[EM]: Received message of type 0x10 with protocolId (0, 0) and MessageCounter:3759770196 on exchange 16183r
E (58719) chip[IN]: Data received on an unknown connection (3). Dropping it!!
E (58719) chip[EM]: Error receiving message from UDP:10.0.0.176:5541: Error CHIP:0x0000007F
E (63629) chip[EM]: Crit-err 3 when sending CHIP MessageCounter:1 on exchange 16184r, send tries: 0

After this, when I try to send onoff toggle command, it doesn't succeed but the message still goes to the esp32 while I expect it to not even receive it.

msandstedt
msandstedt approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@msandstedt msandstedt left a comment

Choose a reason for hiding this comment

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

I really wish the DNS-SD advertisement code could just use the fabric table directly as its backing store.

That would alleviate the need to know when to 'push' updates.

Understood though that the structure of this code doesn't support such a thing.

bzbarsky-apple
bzbarsky-apple approved these changes Oct 15, 2021
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

We need a better name for the "redo all advertising" API than "StartServer"....

@bzbarsky-apple bzbarsky-apple merged commit 39e2636 into project-chip:master Oct 15, 2021
@Damian-Nordic
Copy link
Contributor Author

I really wish the DNS-SD advertisement code could just use the fabric table directly as its backing store.

That would alleviate the need to know when to 'push' updates.

Understood though that the structure of this code doesn't support such a thing.

I think that's only possible with minimal mDNS, but most real devices will probably use their platform APIs like Avahi, OpenThread's SRP or mDNS Responder in which case services must be updated when they change.

@bzbarsky-apple bzbarsky-apple mentioned this pull request Mar 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@msandstedt msandstedt msandstedt approved these changes

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@emargolis emargolis Awaiting requested review from emargolis

@franck-apple franck-apple Awaiting requested review from franck-apple

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@LuDuda LuDuda Awaiting requested review from LuDuda

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@shana-apple shana-apple Awaiting requested review from shana-apple

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@turon turon Awaiting requested review from turon

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
app review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TE6] Operational Service is not removed after FabricRemove
5 participants
@Damian-Nordic @arunbharadwaj @andy31415 @msandstedt @bzbarsky-apple