Skip to content

TXT record method and safety check on netServiceDidResolveAddress()#3

Open
artificiel wants to merge 13 commits into
2bbb:masterfrom
artificiel:master
Open

TXT record method and safety check on netServiceDidResolveAddress()#3
artificiel wants to merge 13 commits into
2bbb:masterfrom
artificiel:master

Conversation

@artificiel
Copy link
Copy Markdown

Difficult to reproduce but switching network configurations can produce this odd situation (and hard crash when there is no objectAtIndex:0).

@artificiel artificiel changed the title netServiceDidResolveAddress(): check that netService.addresses is not empty TXT record method and safety check on netServiceDidResolveAddress() Dec 17, 2021
@artificiel
Copy link
Copy Markdown
Author

Added a method to set the TXT record of the mDNS service to push arbitrary data alongside the discovered services.

@artificiel
Copy link
Copy Markdown
Author

The two commits are not part of a common plan but being in the same branch github includes both...

Comment thread src/ofxBonjourPublisher.mm Outdated

bool ofxBonjourPublisher::setTextRecord(std::vector<std::pair<std::string, std::string>> key_values = {}) {
NSMutableDictionary * record = [NSMutableDictionary dictionary];
for (const auto & [key, value]: key_values) [record setValue:@(value.c_str()) forKey: @(key.c_str())];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a feature from C++17.

please rewrite as C++11 like:

    for(const auto &kv : key_values) [record setValue:@(kv.second.c_str()) forKey: @(kv.first.c_str())];

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey! now that C++17 is OK, what about these? note a few more changes below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants