Respect updated DNS SD broadcast names in TV app #25355
Merged
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
cliffamzn
force-pushed
the
tv_friendly_name
branch
from
6643b86
to
e808d61
Compare
|
PR #25355: Size comparison from e807d2d to e808d61 Full report (1 build for cc32xx)
|
cliffamzn
force-pushed
the
tv_friendly_name
branch
from
e808d61
to
70c8576
Compare
|
PR #25355: Size comparison from 11c2f5b to 70c8576 Decreases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
pullapprove
bot
requested review from
amitnj,
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
CodeChronos928,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy and
jtung-apple
|
PR #25355: Size comparison from f62853e to fdfdeb5 Decreases (1 build for cc32xx)
Full report (3 builds for cc32xx, qpg)
|
|
PR #25355: Size comparison from 3889a44 to bd5e3eb Decreases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
|
PR #25355: Size comparison from ab87102 to 13517a7 Decreases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
|
PR #25355: Size comparison from 6bb8917 to 875d5d8 Increases (1 build for qpg)
Decreases (1 build for cc32xx)
Full report (5 builds for cc32xx, linux, qpg)
|
andy31415
reviewed
Mar 8, 2023
examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java
Outdated
Show resolved
Hide resolved
andy31415
approved these changes
Mar 8, 2023
There was a problem hiding this comment.
Checkmark for the purposes of updating a sample app with changes and checkmarks from app owner - I have not deeply checked the logic and without a review for 1 week this probably qualifies for fast track.
Had a comment about oddness of having a hash/equals that only considers partial class data (and I can understand usage in the array adapter ... but it is odd/unexpected that if (contains) { remove(); } add is not actually a noop but rather replaces same;fkey data).
leaving up to app maintainer to decide what to do with it, I am giving the checkmark in advance.
Thanks! - To clarify the behavior, it does the contains to check if it is "effectively" equal, but in actual use there are some elements that might change on a new broadcast update (such as the broadcast name), and in those cases i still want the button updated. |
|
PR #25355: Size comparison from 43d45ef to 203dd6f Increases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
cliffamzn
commented
Mar 8, 2023
examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java
Show resolved
Hide resolved
…casting/TvCastingApp.java
|
PR #25355: Size comparison from 43d45ef to 5cd98c4 Increases (6 builds for cc13x2_26x2, cyw30739, psoc6, qpg)
Decreases (9 builds for bl602, bl702, cc13x2_26x2, mbed, psoc6)
Full report (23 builds for bl602, bl702, cc13x2_26x2, cyw30739, linux, mbed, psoc6, qpg)
|
This clarifies that the objects are "effectively equal" rather than truly equal.
|
PR #25355: Size comparison from 43d45ef to f83e33c Full report (1 build for cc32xx)
|
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
* Respect updated DNS SD broadcast names When a device is advertising over dns-sd and the TX bits change (device name) a message is sent out to indicate that the old adverisment should be expired and the new one should be used. In the current implementation of the casting app, we stop listening to updates after the `discoveryDurationSeconds` timeout. This causes us to miss messages and Android seems to fall back to the cached value of the DNS advertisment. We need to change the usage pattern here so that clients stop the discovery manually after it is started and keep it active for the lifetime they need it for. I also updated the test apps to behave a bit nicer with discovered commissioners.
mwswartwout
pushed a commit
to mwswartwout/connectedhomeip
that referenced
this pull request
Mar 27, 2023
* Respect updated DNS SD broadcast names When a device is advertising over dns-sd and the TX bits change (device name) a message is sent out to indicate that the old adverisment should be expired and the new one should be used. In the current implementation of the casting app, we stop listening to updates after the `discoveryDurationSeconds` timeout. This causes us to miss messages and Android seems to fall back to the cached value of the DNS advertisment. We need to change the usage pattern here so that clients stop the discovery manually after it is started and keep it active for the lifetime they need it for. I also updated the test apps to behave a bit nicer with discovered commissioners.
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.
When a device is advertising over dns-sd and the TX bits change (device name) a message is sent out to indicate that the old advertisement should be expired and the new one should be used.
In the current implementation of the casting app, we stop listening to updates after the
discoveryDurationSecondstimeout. This causes us to miss messages and Android seems to fall back to the cached value of the DNS advertisement.We need to change the usage pattern here so that clients stop the discovery manually after it is started and keep it active for the lifetime they need it for.
I also updated the test apps to behave a bit nicer with discovered commissioners.
Fixes #25492