Fix error for minimal mDNS and commissioner discovery on TV's.

[sweetymhaiske]

Problem

• Making Clear() call in Advertising API, will lead to failure of commissioner discovery on TV's.

Change overview

Made the appropriate call to Clear() API.

Testing

• Flashed all-clusters-app on a device and connected a device to the router using python controller.
• Turned off the router and device disconnects
• When router is turned on again, then device tries to reconnect and create the entry for DNS records successfully.

[bzbarsky-apple]

Making Clear() call in Advertising API, will lead to failure of commissioner discovery on TV's.

What's the failure? Why does it happen?

[sweetymhaiske]

Making Clear() call in Advertising API, will lead to failure of commissioner discovery on TV's.

What's the failure? Why does it happen?

@chrisdecenzo removed those API calls in this PR https://github.com/project-chip/connectedhomeip/pull/6298/files#diff-5d7e3705190aa2e6b56687bd55d965a4cf12f22ed6fe28e760202e4759d41c56L294. Had a discussion with him he mentioned that it leads to failure in case of commissioner discovery on TV's, and suggested the appropriate changes.

[cecille]

Not sure about TVs, just having the clear in the Advertise call means we clear out operational records when we start commissioning and vice-versa. For now, this is fine, because we haven't started multi-admin scenarios yet. operational and commissioning together need a few more tweaks anyway to keep them separated from each other. I'm good with either method.

[sweetymhaiske]

@chrisdecenzo Can you please take a look at this?

[woody-apple]

/rebase

[sweetymhaiske]

/rebase

Done.

[bzbarsky-apple]

I would really like a clear explanation of what this change is doing and why it's needed. My attempt to look at history here seems to show that we had the Clear() calls in Advertise(), then they were removed in PR #6298 with no explanation, then PR #7356 put them back. Now this PR is removing them again, but adding the call in Start.

We should stop having an "edit war" here, and it seems to me that the way to do that is to add documentation at the places that should be calling Clear explaining why they are doing it and to add documentation at the places that should not be calling Clear explaining why they are not doing it. And that latter explanation needs to cover exactly what goes wrong if you call Clear there.

Would it be possible to add such comments?

[sweetymhaiske]

I would really like a clear explanation of what this change is doing and why it's needed. My attempt to look at history here seems to show that we had the Clear() calls in Advertise(), then they were removed in PR #6298 with no explanation, then PR #7356 put them back. Now this PR is removing them again, but adding the call in Start.

We should stop having an "edit war" here, and it seems to me that the way to do that is to add documentation at the places that should be calling Clear explaining why they are doing it and to add documentation at the places that should not be calling Clear explaining why they are not doing it. And that latter explanation needs to cover exactly what goes wrong if you call Clear there.

Would it be possible to add such comments?

The Clear() method is added to Start() as recommended but was also added back to the Advertise() methods which will be a problem on some devices.
The problem is that there are conditions where a device will call the Advertise(...) method 3 times and want the result to be additive. In other words, advertise operational, commissionable, and commissioner. If clear() is called at the beginning of these Advertise methods, then only the last call to Advertise will take effect.

[bzbarsky-apple]

Thank you. That is much clearer. Can that please go in a comment in the code? So in the Advertise methods document that they must not call Clear() because .... etc.

[bzbarsky-apple]

Suggested change

	* Clear() method should not be called in Advertise()
	* because for the devices which call Advertise() method more than 1 times i.e. advertise operational, commissionable, and
	* commissioner needs the result to be additive. If Clear() is called at the beginning of these Advertise methods, then only the
	* last call to Advertise will take effect.
	* Clear() should not be called in Advertise() because some devices want to advertise
	* multiple things (operational advertisement, commissionable advertisement, commissioner
	* advertisement). If Clear() were called at the beginning of these Advertise methods, then
	* only the last call to Advertise would take effect.

[bzbarsky-apple]

Suggested change

	* Clear() method should not be called in Advertise()
	* because for the devices which call Advertise() method more than 1 times i.e. advertise operational, commissionable, and
	* commissioner needs the result to be additive. If Clear() is called at the beginning of these Advertise methods, then only the
	* last call to Advertise will take effect.
	* Clear() should not be called in Advertise() because some devices want to advertise
	* multiple things (operational advertisement, commissionable advertisement, commissioner
	* advertisement). If Clear() were called at the beginning of these Advertise methods, then
	* only the last call to Advertise would take effect.```

[bzbarsky-apple]

@sweetymhaiske Please note the unresolved review comments (that used to be resolved until the latest update was pushed).

[github-actions]

Size increase report for "nrfconnect-example-build" from 7124079

File	Section	File	VM
chip-lock.elf	device_handles	-4	-4
chip-lock.elf	text	-12	-12
chip-shell.elf	text	56	56
chip-shell.elf	rodata	24	20
chip-shell.elf	device_handles	8	8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,185
.debug_abbrev,0,44
.debug_line,0,19
.debug_ranges,0,8
.debug_frame,0,4
.debug_str,0,4
device_handles,-4,-4
text,-12,-12
.debug_loc,0,-432

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_loc,0,357
.debug_str,0,107
text,56,56
rodata,20,24
device_handles,8,8
.debug_ranges,0,-48
.debug_info,0,-65
.debug_line,0,-79

[sweetymhaiske]

1. The initial problem was the mDNS records were duplicating which was leading to NO Memory error.
2. Hence Clear() call was added in Advertise() method which will clear all records and add new ones.
3. This call to Clear() method should not be made in Advertise() method as we wanted the result to be additive for devices that want to advertise multiple things (operational advertisement, commissionable advertisement, commissioner)
4. However here(Minimal mdns: move responders into allocator class. #7528) Clear() was moved to query responder, which is able to filter the mDNS record based upon the advertisement so that we can easily filter the records and clear them.