From cc1dab4fb2fc49365549f872388a018008838e1a Mon Sep 17 00:00:00 2001 From: Semyon Barenboym Date: Tue, 24 Feb 2026 11:26:24 +0200 Subject: [PATCH 1/4] Implement TR-10-9 Section 15 DNS-SD browse strategy TR-10-9 Section 15 requires IPMX devices to: - Support both mDNS and unicast DNS for DNS-SD browse operations - Default to using both methods - Provide user mechanisms to limit browsing to unicast or mDNS only - When using both: try unicast DNS first, fall back to mDNS only if unicast is unsuccessful (no service discovered) - When multiple results are returned: select by best priority, failing over to next-best if unresponsive - Once a service is selected and responsive: perform no further browse operations for that service type - If the selected service becomes unresponsive: perform a new DNS-SD browse and restart selection Add dns_sd_browse_mode setting ("both"/"unicast"/"mdns") to implement the required dual-discovery strategy. In "both" mode (default), unicast DNS is tried first with half the timeout budget; if no records are found, mDNS fallback gets the remaining half. "unicast" and "mdns" modes restrict to a single method. Also filters browse results by domain class to prevent mDNS results leaking into unicast DNS queries and vice versa. Signed-off-by: Semyon Barenboym --- Development/nmos/mdns.cpp | 58 +++++++++++++++++++++++++++++-------- Development/nmos/settings.h | 6 ++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index 2549e5f43..1c7e98ee9 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -503,6 +503,16 @@ namespace nmos { return discovery.browse([=, &discovery](const mdns::browse_result& resolving) { + // Skip results from a different discovery domain class + // (prevents mDNS results leaking into unicast DNS queries and vice versa) + if (!browse_domain.empty()) + { + if (is_local_domain(browse_domain) != is_local_domain(resolving.domain)) + { + return true; // skip this result, keep browsing + } + } + const bool cancel = pplx::canceled == discovery.resolve([=](const mdns::resolve_result& resolved) { // "The Node [filters] out any APIs which do not support its required API version, protocol and authorization mode (TXT api_ver, api_proto and api_auth)." @@ -712,26 +722,26 @@ namespace nmos // helper function for resolving instances of the specified service (API) based on the specified settings // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly + // TR-10-9 Section 15: supports "both" (unicast DNS first, mDNS fallback), "unicast", and "mdns" browse modes pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { - const auto browse_domain = utility::us2s(nmos::get_domain(settings)); - const auto versions = details::service_versions(service, settings); - const auto priorities = details::service_priorities(service, settings); - const auto protocols = std::set{ nmos::get_service_protocol(service, settings) }; - const auto authorization = std::set{ nmos::get_service_authorization(service, settings) }; - - // use a short timeout that's long enough to ensure the daemon's cache is exhausted - // when no cancellation token is specified - const auto timeout = token.is_cancelable() ? nmos::fields::discovery_backoff_max(settings) : 1; - - return resolve_service(discovery, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::duration_cast(std::chrono::seconds(timeout)), token); + // delegate to resolve_service_ (which has the dual-discovery logic) and transform results + return resolve_service_(discovery, service, settings, token).then([](std::list resolved_services) + { + return boost::copy_range>(resolved_services | boost::adaptors::transformed([](const resolved_service& s) + { + return web::uri_builder(s.second).append_path(U("/") + make_api_version(s.first.first)).to_uri(); + })); + }); } // helper function for resolving instances of the specified service (API) based on the specified settings // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly + // TR-10-9 Section 15: supports "both" (unicast DNS first, mDNS fallback), "unicast", and "mdns" browse modes pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { const auto browse_domain = utility::us2s(nmos::get_domain(settings)); + const auto browse_mode = utility::us2s(nmos::fields::dns_sd_browse_mode(settings)); const auto versions = details::service_versions(service, settings); const auto priorities = details::service_priorities(service, settings); const auto protocols = std::set{ nmos::get_service_protocol(service, settings) }; @@ -740,8 +750,32 @@ namespace nmos // use a short timeout that's long enough to ensure the daemon's cache is exhausted // when no cancellation token is specified const auto timeout = token.is_cancelable() ? nmos::fields::discovery_backoff_max(settings) : 1; + const auto timeout_dur = std::chrono::duration_cast(std::chrono::seconds(timeout)); + + // determine primary browse domain based on mode + const auto primary_domain = (browse_mode == "mdns") ? std::string("local.") : browse_domain; + const bool has_fallback = (browse_mode == "both") && !is_local_domain(browse_domain); + + // when there's a fallback, give the primary browse half the budget + // so the mDNS fallback gets a meaningful allocation + const auto primary_timeout = has_fallback ? timeout_dur / 2 : timeout_dur; + + auto primary_task = resolve_service_(discovery, service, primary_domain, versions, priorities, protocols, authorization, true, primary_timeout, token); + + if (has_fallback) + { + return primary_task.then([&discovery, service, versions, priorities, protocols, authorization, timeout_dur, primary_timeout, token](std::list results) + { + if (!results.empty()) return pplx::task_from_result(std::move(results)); + + // TR-10-9: unicast DNS unsuccessful, fall back to mDNS + // give the fallback at least as much time as the primary browse had + const auto fallback_timeout = timeout_dur - primary_timeout; + return resolve_service_(discovery, service, std::string("local."), versions, priorities, protocols, authorization, true, fallback_timeout, token); + }); + } - return resolve_service_(discovery, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::duration_cast(std::chrono::seconds(timeout)), token); + return primary_task; } } } diff --git a/Development/nmos/settings.h b/Development/nmos/settings.h index 4f1a09328..a3ee32e34 100644 --- a/Development/nmos/settings.h +++ b/Development/nmos/settings.h @@ -82,6 +82,12 @@ namespace nmos // domain [registry, node]: the domain on which to browse for services or an empty string to use the default domain (specify "local." to explictly select mDNS) const web::json::field_as_string_or domain{ U("domain"), U("") }; + // dns_sd_browse_mode [node]: controls DNS-SD browse method per TR-10-9 Section 15 + // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful + // "unicast" = unicast DNS only (requires domain to be set to a non-local value) + // "mdns" = mDNS only + const web::json::field_as_string_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), U("both") }; + // host_address/host_addresses [registry, node]: IP addresses used to construct response headers (e.g. 'Link' or 'Location'), and host and URL fields in the data model const web::json::field_as_string_or host_address{ U("host_address"), U("127.0.0.1") }; const web::json::field_as_array host_addresses{ U("host_addresses") }; From 337a4b5492c402066f23880fb42e558ce511160c Mon Sep 17 00:00:00 2001 From: Semyon Barenboym Date: Wed, 29 Apr 2026 15:46:08 +0300 Subject: [PATCH 2/4] Address review feedback on TR-10-9 DNS-SD browse strategy - Use string_enum for dns_sd_browse_mode (nmos::dns_sd_browse_modes::{both,unicast,mdns}) - Add dns_sd_browse_mode example + behaviour table to nmos-cpp-node config.json - Add expected-behaviour table to settings.h - Mark unused URI-returning resolve_service overload as deprecated (comment) - Refresh resolve_service / resolve_service_ doc comments in mdns.h/.cpp Co-Authored-By: Claude Opus 4.7 (1M context) --- Development/cmake/NmosCppLibraries.cmake | 1 + Development/nmos-cpp-node/config.json | 15 ++++++++++ Development/nmos/dns_sd_browse_mode.h | 19 ++++++++++++ Development/nmos/mdns.cpp | 37 +++++++++++++++--------- Development/nmos/mdns.h | 35 ++++++++++++++-------- Development/nmos/settings.h | 18 +++++++++--- 6 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 Development/nmos/dns_sd_browse_mode.h diff --git a/Development/cmake/NmosCppLibraries.cmake b/Development/cmake/NmosCppLibraries.cmake index 6a8b92456..d0ef8f885 100644 --- a/Development/cmake/NmosCppLibraries.cmake +++ b/Development/cmake/NmosCppLibraries.cmake @@ -1133,6 +1133,7 @@ set(NMOS_CPP_NMOS_HEADERS nmos/control_protocol_ws_api.h nmos/device_type.h nmos/did_sdid.h + nmos/dns_sd_browse_mode.h nmos/event_type.h nmos/events_api.h nmos/events_resources.h diff --git a/Development/nmos-cpp-node/config.json b/Development/nmos-cpp-node/config.json index c529de695..d6aeb2e80 100644 --- a/Development/nmos-cpp-node/config.json +++ b/Development/nmos-cpp-node/config.json @@ -85,6 +85,21 @@ // domain [registry, node]: the domain on which to browse for services or an empty string to use the default domain (specify "local." to explictly select mDNS) //"domain": "", + // dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15 + // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful + // "unicast" = unicast DNS only + // "mdns" = mDNS only + // Expected resolve behaviour for each (mode, domain) combination: + // mode | domain | behaviour + // --------+-------------+-------------------- + // both | example.com | unicast -> mdns + // both | local. | mdns + // unicast | example.com | unicast + // unicast | local. | mdns + // mdns | example.com | mdns + // mdns | local. | mdns + //"dns_sd_browse_mode": "both", + // host_address/host_addresses [registry, node]: IP addresses used to construct response headers (e.g. 'Link' or 'Location'), and host and URL fields in the data model //"host_address": "127.0.0.1", //"host_addresses": array-of-ip-address-strings, diff --git a/Development/nmos/dns_sd_browse_mode.h b/Development/nmos/dns_sd_browse_mode.h new file mode 100644 index 000000000..85b598ee4 --- /dev/null +++ b/Development/nmos/dns_sd_browse_mode.h @@ -0,0 +1,19 @@ +#ifndef NMOS_DNS_SD_BROWSE_MODE_H +#define NMOS_DNS_SD_BROWSE_MODE_H + +#include "nmos/string_enum.h" + +namespace nmos +{ + // DNS-SD browse method per TR-10-9 Section 15 + // Selected via the dns_sd_browse_mode setting in nmos::fields + DEFINE_STRING_ENUM(dns_sd_browse_mode) + namespace dns_sd_browse_modes + { + const dns_sd_browse_mode both{ U("both") }; // unicast DNS first, mDNS fallback if unsuccessful + const dns_sd_browse_mode unicast{ U("unicast") }; // unicast DNS only + const dns_sd_browse_mode mdns{ U("mdns") }; // mDNS only + } +} + +#endif diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index 1c7e98ee9..1a72c221d 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -12,6 +12,7 @@ #include "cpprest/uri_builder.h" #include "mdns/service_advertiser.h" #include "mdns/service_discovery.h" +#include "nmos/dns_sd_browse_mode.h" #include "nmos/is09_versions.h" #include "nmos/is10_versions.h" #include "nmos/random.h" @@ -600,8 +601,9 @@ namespace nmos } } - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and optionally services with the same priority ordered randomly + // Helper function for resolving instances of the specified service (API), returning ((api_version, priority), uri) + // tuples so callers can inspect the matched version and priority. Highest version and highest priority first, + // and optionally services with the same priority ordered randomly. pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) { const auto absolute_timeout = std::chrono::steady_clock::now() + timeout; @@ -706,8 +708,10 @@ namespace nmos }); } - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and optionally services with the same priority ordered randomly + // DEPRECATED: this overload is unused; prefer resolve_service_ which also returns api_ver/priority info. + // Helper function for resolving instances of the specified service (API), returning a list of base URIs + // (with the API version path appended), highest version and highest priority first, and optionally + // services with the same priority ordered randomly. pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) { return resolve_service_(discovery, service, browse_domain, api_ver, priorities, api_proto, api_auth, randomize, timeout, token).then([](std::list resolved_services) @@ -720,12 +724,13 @@ namespace nmos }); } - // helper function for resolving instances of the specified service (API) based on the specified settings - // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly - // TR-10-9 Section 15: supports "both" (unicast DNS first, mDNS fallback), "unicast", and "mdns" browse modes + // Helper function for resolving instances of the specified service (API) based on the specified settings, + // returning a list of base URIs (with the API version path appended), highest version and highest priority + // first, services with the same priority ordered randomly. + // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15 + // (delegates to resolve_service_, which carries the dual-discovery logic). pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { - // delegate to resolve_service_ (which has the dual-discovery logic) and transform results return resolve_service_(discovery, service, settings, token).then([](std::list resolved_services) { return boost::copy_range>(resolved_services | boost::adaptors::transformed([](const resolved_service& s) @@ -735,13 +740,17 @@ namespace nmos }); } - // helper function for resolving instances of the specified service (API) based on the specified settings - // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly - // TR-10-9 Section 15: supports "both" (unicast DNS first, mDNS fallback), "unicast", and "mdns" browse modes + // Helper function for resolving instances of the specified service (API) based on the specified settings, + // returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services + // with the same priority ordered randomly. + // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15: + // - "both" (default): unicast DNS first, mDNS fallback if unsuccessful + // - "unicast" : unicast DNS only + // - "mdns" : mDNS only pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { const auto browse_domain = utility::us2s(nmos::get_domain(settings)); - const auto browse_mode = utility::us2s(nmos::fields::dns_sd_browse_mode(settings)); + const auto browse_mode = nmos::dns_sd_browse_mode(nmos::fields::dns_sd_browse_mode(settings)); const auto versions = details::service_versions(service, settings); const auto priorities = details::service_priorities(service, settings); const auto protocols = std::set{ nmos::get_service_protocol(service, settings) }; @@ -753,8 +762,8 @@ namespace nmos const auto timeout_dur = std::chrono::duration_cast(std::chrono::seconds(timeout)); // determine primary browse domain based on mode - const auto primary_domain = (browse_mode == "mdns") ? std::string("local.") : browse_domain; - const bool has_fallback = (browse_mode == "both") && !is_local_domain(browse_domain); + const auto primary_domain = (browse_mode == nmos::dns_sd_browse_modes::mdns) ? std::string("local.") : browse_domain; + const bool has_fallback = (browse_mode == nmos::dns_sd_browse_modes::both) && !is_local_domain(browse_domain); // when there's a fallback, give the primary browse half the budget // so the mDNS fallback gets a meaningful allocation diff --git a/Development/nmos/mdns.h b/Development/nmos/mdns.h index 8267ab850..39a16a1a5 100644 --- a/Development/nmos/mdns.h +++ b/Development/nmos/mdns.h @@ -157,39 +157,50 @@ namespace nmos // helper function for updating the specified service (API) TXT records void update_service(mdns::service_advertiser& advertiser, const nmos::service_type& service, const nmos::settings& settings, mdns::structured_txt_records add_records = {}); - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly + // DEPRECATED: this overload is unused; prefer resolve_service_ which also returns api_ver/priority info. + // Helper function for resolving instances of the specified service (API), returning a list of base URIs + // (with the API version path appended), highest version and highest priority first, and (by default) + // services with the same priority ordered randomly. pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token = pplx::cancellation_token::none()); - // helper function for resolving instances of the specified service (API) based on the specified options or defaults - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly + // DEPRECATED: convenience wrapper around the deprecated explicit-args resolve_service overload above. + // Same behaviour, with default values for unspecified arguments. template inline pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain = {}, const std::set& api_ver = nmos::is04_versions::all, const std::pair& priorities = { service_priorities::highest_active_priority, service_priorities::no_priority }, const std::set& api_proto = nmos::service_protocols::all, const std::set& api_auth = { false, true }, bool randomize = true, const std::chrono::duration& timeout = std::chrono::seconds(mdns::default_timeout_seconds), const pplx::cancellation_token& token = pplx::cancellation_token::none()) { return resolve_service(discovery, service, browse_domain, api_ver, api_proto, api_auth, randomize, std::chrono::duration_cast(timeout), token); } - // helper function for resolving instances of the specified service (API) based on the specified settings - // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly + // Helper function for resolving instances of the specified service (API) based on the specified settings, + // returning a list of base URIs (with the API version path appended), highest version and highest priority + // first, services with the same priority ordered randomly. + // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15 + // (delegates to resolve_service_, which carries the dual-discovery logic). pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token = pplx::cancellation_token::none()); typedef std::pair api_ver_pri; typedef std::pair resolved_service; - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly + // Helper function for resolving instances of the specified service (API), returning ((api_version, priority), uri) + // tuples so callers can inspect the matched version and priority. Highest version and highest priority first, + // and (by default) services with the same priority ordered randomly. pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token = pplx::cancellation_token::none()); - // helper function for resolving instances of the specified service (API) based on the specified options or defaults - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly + // Convenience wrapper around the explicit-args resolve_service_ overload above, with default values + // for unspecified arguments. template inline pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain = {}, const std::set& api_ver = nmos::is04_versions::all, const std::pair& priorities = { service_priorities::highest_active_priority, service_priorities::no_priority }, const std::set& api_proto = nmos::service_protocols::all, const std::set& api_auth = { false, true }, bool randomize = true, const std::chrono::duration& timeout = std::chrono::seconds(mdns::default_timeout_seconds), const pplx::cancellation_token& token = pplx::cancellation_token::none()) { return resolve_service_(discovery, service, browse_domain, api_ver, api_proto, api_auth, randomize, std::chrono::duration_cast(timeout), token); } - // helper function for resolving instances of the specified service (API) based on the specified settings - // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly + // Helper function for resolving instances of the specified service (API) based on the specified settings, + // returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services + // with the same priority ordered randomly. + // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15: + // - "both" (default): unicast DNS first, mDNS fallback if unsuccessful + // - "unicast" : unicast DNS only + // - "mdns" : mDNS only pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token = pplx::cancellation_token::none()); } } diff --git a/Development/nmos/settings.h b/Development/nmos/settings.h index a3ee32e34..8c4ceb05b 100644 --- a/Development/nmos/settings.h +++ b/Development/nmos/settings.h @@ -82,10 +82,20 @@ namespace nmos // domain [registry, node]: the domain on which to browse for services or an empty string to use the default domain (specify "local." to explictly select mDNS) const web::json::field_as_string_or domain{ U("domain"), U("") }; - // dns_sd_browse_mode [node]: controls DNS-SD browse method per TR-10-9 Section 15 - // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful - // "unicast" = unicast DNS only (requires domain to be set to a non-local value) - // "mdns" = mDNS only + // dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15 + // (see nmos::dns_sd_browse_modes for valid values) + // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful + // "unicast" = unicast DNS only + // "mdns" = mDNS only + // Expected resolve behaviour for each (mode, domain) combination: + // mode | domain | behaviour + // --------+-------------+-------------------- + // both | example.com | unicast -> mdns + // both | local. | mdns + // unicast | example.com | unicast + // unicast | local. | mdns + // mdns | example.com | mdns + // mdns | local. | mdns const web::json::field_as_string_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), U("both") }; // host_address/host_addresses [registry, node]: IP addresses used to construct response headers (e.g. 'Link' or 'Location'), and host and URL fields in the data model From 7b5538af8196e0259168d632601ac369baf68f96 Mon Sep 17 00:00:00 2001 From: Semyon Barenboym Date: Wed, 29 Apr 2026 16:56:18 +0300 Subject: [PATCH 3/4] Use full discovery timeout for both primary and fallback browse Per maintainer review (jonathan-r-thorpe, lo-simon): the primary service timeout should remain unchanged when a fallback is in play, with the same timeout applied per service rather than halved across them. Co-Authored-By: Claude Opus 4.7 (1M context) --- Development/nmos/mdns.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index 1a72c221d..d44ed7bee 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -765,22 +765,16 @@ namespace nmos const auto primary_domain = (browse_mode == nmos::dns_sd_browse_modes::mdns) ? std::string("local.") : browse_domain; const bool has_fallback = (browse_mode == nmos::dns_sd_browse_modes::both) && !is_local_domain(browse_domain); - // when there's a fallback, give the primary browse half the budget - // so the mDNS fallback gets a meaningful allocation - const auto primary_timeout = has_fallback ? timeout_dur / 2 : timeout_dur; - - auto primary_task = resolve_service_(discovery, service, primary_domain, versions, priorities, protocols, authorization, true, primary_timeout, token); + auto primary_task = resolve_service_(discovery, service, primary_domain, versions, priorities, protocols, authorization, true, timeout_dur, token); if (has_fallback) { - return primary_task.then([&discovery, service, versions, priorities, protocols, authorization, timeout_dur, primary_timeout, token](std::list results) + return primary_task.then([&discovery, service, versions, priorities, protocols, authorization, timeout_dur, token](std::list results) { if (!results.empty()) return pplx::task_from_result(std::move(results)); - // TR-10-9: unicast DNS unsuccessful, fall back to mDNS - // give the fallback at least as much time as the primary browse had - const auto fallback_timeout = timeout_dur - primary_timeout; - return resolve_service_(discovery, service, std::string("local."), versions, priorities, protocols, authorization, true, fallback_timeout, token); + // TR-10-9: unicast DNS unsuccessful, fall back to mDNS (full timeout per service) + return resolve_service_(discovery, service, std::string("local."), versions, priorities, protocols, authorization, true, timeout_dur, token); }); } From 09df043e0aa6abaa83ea3193a3304e59c56e0b14 Mon Sep 17 00:00:00 2001 From: Semyon Barenboym Date: Thu, 30 Apr 2026 11:08:39 +0300 Subject: [PATCH 4/4] Switch dns_sd_browse_mode from string to integer enum Per lo-simon's review: use a typed enum (compile-checked) rather than a string to reduce typo risk in user configurations. - enum dns_sd_browse_mode { dns_sd_browse_mode_both = 0, _unicast = 1, _mdns = 2 } defined file-local in mdns.cpp under nmos::experimental, mirroring how href_mode lives in settings.cpp - field becomes field_as_integer_or with default 0 - example config.json now shows //"dns_sd_browse_mode": 0, - doc comments updated; dns_sd_browse_mode.h header removed Used dns_sd_browse_mode_* prefix on the enumerators (rather than the bare both/unicast/mdns suggested in the review) to avoid polluting nmos:: with generic identifiers, matching the href_mode_* convention. Co-Authored-By: Claude Opus 4.7 (1M context) --- Development/cmake/NmosCppLibraries.cmake | 1 - Development/nmos-cpp-node/config.json | 8 ++++---- Development/nmos/dns_sd_browse_mode.h | 19 ------------------- Development/nmos/mdns.cpp | 21 ++++++++++++++------- Development/nmos/mdns.h | 6 +++--- Development/nmos/settings.h | 9 ++++----- 6 files changed, 25 insertions(+), 39 deletions(-) delete mode 100644 Development/nmos/dns_sd_browse_mode.h diff --git a/Development/cmake/NmosCppLibraries.cmake b/Development/cmake/NmosCppLibraries.cmake index d0ef8f885..6a8b92456 100644 --- a/Development/cmake/NmosCppLibraries.cmake +++ b/Development/cmake/NmosCppLibraries.cmake @@ -1133,7 +1133,6 @@ set(NMOS_CPP_NMOS_HEADERS nmos/control_protocol_ws_api.h nmos/device_type.h nmos/did_sdid.h - nmos/dns_sd_browse_mode.h nmos/event_type.h nmos/events_api.h nmos/events_resources.h diff --git a/Development/nmos-cpp-node/config.json b/Development/nmos-cpp-node/config.json index d6aeb2e80..43347c669 100644 --- a/Development/nmos-cpp-node/config.json +++ b/Development/nmos-cpp-node/config.json @@ -86,9 +86,9 @@ //"domain": "", // dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15 - // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful - // "unicast" = unicast DNS only - // "mdns" = mDNS only + // both(0) (default) = unicast DNS first, mDNS fallback if unsuccessful + // unicast(1) = unicast DNS only + // mdns(2) = mDNS only // Expected resolve behaviour for each (mode, domain) combination: // mode | domain | behaviour // --------+-------------+-------------------- @@ -98,7 +98,7 @@ // unicast | local. | mdns // mdns | example.com | mdns // mdns | local. | mdns - //"dns_sd_browse_mode": "both", + //"dns_sd_browse_mode": 0, // host_address/host_addresses [registry, node]: IP addresses used to construct response headers (e.g. 'Link' or 'Location'), and host and URL fields in the data model //"host_address": "127.0.0.1", diff --git a/Development/nmos/dns_sd_browse_mode.h b/Development/nmos/dns_sd_browse_mode.h deleted file mode 100644 index 85b598ee4..000000000 --- a/Development/nmos/dns_sd_browse_mode.h +++ /dev/null @@ -1,19 +0,0 @@ -#ifndef NMOS_DNS_SD_BROWSE_MODE_H -#define NMOS_DNS_SD_BROWSE_MODE_H - -#include "nmos/string_enum.h" - -namespace nmos -{ - // DNS-SD browse method per TR-10-9 Section 15 - // Selected via the dns_sd_browse_mode setting in nmos::fields - DEFINE_STRING_ENUM(dns_sd_browse_mode) - namespace dns_sd_browse_modes - { - const dns_sd_browse_mode both{ U("both") }; // unicast DNS first, mDNS fallback if unsuccessful - const dns_sd_browse_mode unicast{ U("unicast") }; // unicast DNS only - const dns_sd_browse_mode mdns{ U("mdns") }; // mDNS only - } -} - -#endif diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index d44ed7bee..057a799e2 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -12,7 +12,6 @@ #include "cpprest/uri_builder.h" #include "mdns/service_advertiser.h" #include "mdns/service_discovery.h" -#include "nmos/dns_sd_browse_mode.h" #include "nmos/is09_versions.h" #include "nmos/is10_versions.h" #include "nmos/random.h" @@ -300,6 +299,14 @@ namespace nmos const service_type register_{ "_nmos-register._tcp" }; } + // DNS-SD browse method per TR-10-9 Section 15; selected via the dns_sd_browse_mode setting + enum dns_sd_browse_mode + { + dns_sd_browse_mode_both = 0, // unicast DNS first, mDNS fallback if unsuccessful + dns_sd_browse_mode_unicast = 1, // unicast DNS only + dns_sd_browse_mode_mdns = 2 // mDNS only + }; + namespace experimental { namespace details @@ -744,13 +751,13 @@ namespace nmos // returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services // with the same priority ordered randomly. // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15: - // - "both" (default): unicast DNS first, mDNS fallback if unsuccessful - // - "unicast" : unicast DNS only - // - "mdns" : mDNS only + // - both (default): unicast DNS first, mDNS fallback if unsuccessful + // - unicast : unicast DNS only + // - mdns : mDNS only pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { const auto browse_domain = utility::us2s(nmos::get_domain(settings)); - const auto browse_mode = nmos::dns_sd_browse_mode(nmos::fields::dns_sd_browse_mode(settings)); + const auto browse_mode = dns_sd_browse_mode(nmos::fields::dns_sd_browse_mode(settings)); const auto versions = details::service_versions(service, settings); const auto priorities = details::service_priorities(service, settings); const auto protocols = std::set{ nmos::get_service_protocol(service, settings) }; @@ -762,8 +769,8 @@ namespace nmos const auto timeout_dur = std::chrono::duration_cast(std::chrono::seconds(timeout)); // determine primary browse domain based on mode - const auto primary_domain = (browse_mode == nmos::dns_sd_browse_modes::mdns) ? std::string("local.") : browse_domain; - const bool has_fallback = (browse_mode == nmos::dns_sd_browse_modes::both) && !is_local_domain(browse_domain); + const auto primary_domain = (browse_mode == dns_sd_browse_mode_mdns) ? std::string("local.") : browse_domain; + const bool has_fallback = (browse_mode == dns_sd_browse_mode_both) && !is_local_domain(browse_domain); auto primary_task = resolve_service_(discovery, service, primary_domain, versions, priorities, protocols, authorization, true, timeout_dur, token); diff --git a/Development/nmos/mdns.h b/Development/nmos/mdns.h index 39a16a1a5..9d973b3c7 100644 --- a/Development/nmos/mdns.h +++ b/Development/nmos/mdns.h @@ -198,9 +198,9 @@ namespace nmos // returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services // with the same priority ordered randomly. // The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15: - // - "both" (default): unicast DNS first, mDNS fallback if unsuccessful - // - "unicast" : unicast DNS only - // - "mdns" : mDNS only + // - both (default): unicast DNS first, mDNS fallback if unsuccessful + // - unicast : unicast DNS only + // - mdns : mDNS only pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token = pplx::cancellation_token::none()); } } diff --git a/Development/nmos/settings.h b/Development/nmos/settings.h index 8c4ceb05b..02549d927 100644 --- a/Development/nmos/settings.h +++ b/Development/nmos/settings.h @@ -83,10 +83,9 @@ namespace nmos const web::json::field_as_string_or domain{ U("domain"), U("") }; // dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15 - // (see nmos::dns_sd_browse_modes for valid values) - // "both" (default) = unicast DNS first, mDNS fallback if unsuccessful - // "unicast" = unicast DNS only - // "mdns" = mDNS only + // both(0) (default) = unicast DNS first, mDNS fallback if unsuccessful + // unicast(1) = unicast DNS only + // mdns(2) = mDNS only // Expected resolve behaviour for each (mode, domain) combination: // mode | domain | behaviour // --------+-------------+-------------------- @@ -96,7 +95,7 @@ namespace nmos // unicast | local. | mdns // mdns | example.com | mdns // mdns | local. | mdns - const web::json::field_as_string_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), U("both") }; + const web::json::field_as_integer_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), 0 }; // host_address/host_addresses [registry, node]: IP addresses used to construct response headers (e.g. 'Link' or 'Location'), and host and URL fields in the data model const web::json::field_as_string_or host_address{ U("host_address"), U("127.0.0.1") };