Skip to content

Commit 28d14db

Browse files
simonbarenclaude
andcommitted
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) <[email protected]>
1 parent 7b5538a commit 28d14db

6 files changed

Lines changed: 25 additions & 39 deletions

File tree

Development/cmake/NmosCppLibraries.cmake

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1133,7 +1133,6 @@ set(NMOS_CPP_NMOS_HEADERS
11331133
nmos/control_protocol_ws_api.h
11341134
nmos/device_type.h
11351135
nmos/did_sdid.h
1136-
nmos/dns_sd_browse_mode.h
11371136
nmos/event_type.h
11381137
nmos/events_api.h
11391138
nmos/events_resources.h

Development/nmos-cpp-node/config.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@
8686
//"domain": "",
8787

8888
// dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15
89-
// "both" (default) = unicast DNS first, mDNS fallback if unsuccessful
90-
// "unicast" = unicast DNS only
91-
// "mdns" = mDNS only
89+
// both(0) (default) = unicast DNS first, mDNS fallback if unsuccessful
90+
// unicast(1) = unicast DNS only
91+
// mdns(2) = mDNS only
9292
// Expected resolve behaviour for each (mode, domain) combination:
9393
// mode | domain | behaviour
9494
// --------+-------------+--------------------
@@ -98,7 +98,7 @@
9898
// unicast | local. | mdns
9999
// mdns | example.com | mdns
100100
// mdns | local. | mdns
101-
//"dns_sd_browse_mode": "both",
101+
//"dns_sd_browse_mode": 0,
102102

103103
// 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
104104
//"host_address": "127.0.0.1",

Development/nmos/dns_sd_browse_mode.h

Lines changed: 0 additions & 19 deletions
This file was deleted.

Development/nmos/mdns.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "cpprest/uri_builder.h"
1313
#include "mdns/service_advertiser.h"
1414
#include "mdns/service_discovery.h"
15-
#include "nmos/dns_sd_browse_mode.h"
1615
#include "nmos/is09_versions.h"
1716
#include "nmos/is10_versions.h"
1817
#include "nmos/random.h"
@@ -302,6 +301,14 @@ namespace nmos
302301

303302
namespace experimental
304303
{
304+
// DNS-SD browse method per TR-10-9 Section 15; selected via the dns_sd_browse_mode setting
305+
enum dns_sd_browse_mode
306+
{
307+
dns_sd_browse_mode_both = 0, // unicast DNS first, mDNS fallback if unsuccessful
308+
dns_sd_browse_mode_unicast = 1, // unicast DNS only
309+
dns_sd_browse_mode_mdns = 2 // mDNS only
310+
};
311+
305312
namespace details
306313
{
307314
inline int service_port(const nmos::service_type& service, const nmos::settings& settings)
@@ -744,13 +751,13 @@ namespace nmos
744751
// returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services
745752
// with the same priority ordered randomly.
746753
// The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15:
747-
// - "both" (default): unicast DNS first, mDNS fallback if unsuccessful
748-
// - "unicast" : unicast DNS only
749-
// - "mdns" : mDNS only
754+
// - both (default): unicast DNS first, mDNS fallback if unsuccessful
755+
// - unicast : unicast DNS only
756+
// - mdns : mDNS only
750757
pplx::task<std::list<resolved_service>> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token)
751758
{
752759
const auto browse_domain = utility::us2s(nmos::get_domain(settings));
753-
const auto browse_mode = nmos::dns_sd_browse_mode(nmos::fields::dns_sd_browse_mode(settings));
760+
const auto browse_mode = dns_sd_browse_mode(nmos::fields::dns_sd_browse_mode(settings));
754761
const auto versions = details::service_versions(service, settings);
755762
const auto priorities = details::service_priorities(service, settings);
756763
const auto protocols = std::set<nmos::service_protocol>{ nmos::get_service_protocol(service, settings) };
@@ -762,8 +769,8 @@ namespace nmos
762769
const auto timeout_dur = std::chrono::duration_cast<std::chrono::steady_clock::duration>(std::chrono::seconds(timeout));
763770

764771
// determine primary browse domain based on mode
765-
const auto primary_domain = (browse_mode == nmos::dns_sd_browse_modes::mdns) ? std::string("local.") : browse_domain;
766-
const bool has_fallback = (browse_mode == nmos::dns_sd_browse_modes::both) && !is_local_domain(browse_domain);
772+
const auto primary_domain = (browse_mode == dns_sd_browse_mode_mdns) ? std::string("local.") : browse_domain;
773+
const bool has_fallback = (browse_mode == dns_sd_browse_mode_both) && !is_local_domain(browse_domain);
767774

768775
auto primary_task = resolve_service_(discovery, service, primary_domain, versions, priorities, protocols, authorization, true, timeout_dur, token);
769776

Development/nmos/mdns.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,9 @@ namespace nmos
198198
// returning ((api_version, priority), uri) tuples. Highest version and highest priority first, services
199199
// with the same priority ordered randomly.
200200
// The browse method is selected by the dns_sd_browse_mode setting per TR-10-9 Section 15:
201-
// - "both" (default): unicast DNS first, mDNS fallback if unsuccessful
202-
// - "unicast" : unicast DNS only
203-
// - "mdns" : mDNS only
201+
// - both (default): unicast DNS first, mDNS fallback if unsuccessful
202+
// - unicast : unicast DNS only
203+
// - mdns : mDNS only
204204
pplx::task<std::list<resolved_service>> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token = pplx::cancellation_token::none());
205205
}
206206
}

Development/nmos/settings.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,9 @@ namespace nmos
8383
const web::json::field_as_string_or domain{ U("domain"), U("") };
8484

8585
// dns_sd_browse_mode [node]: DNS-SD browse method per TR-10-9 Section 15
86-
// (see nmos::dns_sd_browse_modes for valid values)
87-
// "both" (default) = unicast DNS first, mDNS fallback if unsuccessful
88-
// "unicast" = unicast DNS only
89-
// "mdns" = mDNS only
86+
// both(0) (default) = unicast DNS first, mDNS fallback if unsuccessful
87+
// unicast(1) = unicast DNS only
88+
// mdns(2) = mDNS only
9089
// Expected resolve behaviour for each (mode, domain) combination:
9190
// mode | domain | behaviour
9291
// --------+-------------+--------------------
@@ -96,7 +95,7 @@ namespace nmos
9695
// unicast | local. | mdns
9796
// mdns | example.com | mdns
9897
// mdns | local. | mdns
99-
const web::json::field_as_string_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), U("both") };
98+
const web::json::field_as_integer_or dns_sd_browse_mode{ U("dns_sd_browse_mode"), 0 };
10099

101100
// 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
102101
const web::json::field_as_string_or host_address{ U("host_address"), U("127.0.0.1") };

0 commit comments

Comments
 (0)