ONVIF Discovery Handler - Handle Multicast Response Duplicates #382
Conversation
kate-goldenring
requested review from
bfjelds,
Britel,
jiria and
romoh
as code owners
kate-goldenring
changed the title
| @@ -352,6 +352,7 @@ pub mod util { | |||
| "simple_onvif_discover - uris after filtering by scopes {:?}", | |||
| filtered_uris | |||
| ); | |||
| filtered_uris.dedup(); | |||
There was a problem hiding this comment.
Would it make sense to have broadcast_responses to be a set so that we do not capture duplicates in the first place or dedup on broadcast_responses prior to calling get_scope_filtered_uris_from_discovery_response?
There was a problem hiding this comment.
Good point! Switched to a hashset in 80bcc3b
|
Curious, do we understand why duplicates happen in the first place? |
Might be. Have no way of telling until we run into it or get the logs. |
Fair. Maybe it is worth adding a note in the bug and making sure we have enough logging in place to be able to root cause this if it happens again. |
| @@ -323,7 +323,7 @@ pub mod util { | |||
|
|
|||
| match try_recv_string(socket, time_left).await { | |||
| Ok(s) => { | |||
| broadcast_responses.push(s); | |||
| broadcast_responses.insert(s); | |||
There was a problem hiding this comment.
i wonder if we might want to s.ToLower here (i assume that a device would be consistent in how it describes itself, so it seems like this wouldn't be needed ... but something to think about)
Signed-off-by: vincepnguyen <70007233+vincepnguyen@users.noreply.github.com>
What this PR does / why we need it:
closes #365
Special notes for your reviewer:
Seems like our multicast occasionally returns the same service url twice. Simply deduping the returned ONVIF service URLs seems like a good strategy.
If applicable:
cargo fmt)cargo build)cargo clippy)cargo test)cargo doc)./version.sh)