mdns: add txt filtering option for client connections#93
Closed
mhei wants to merge 3 commits into
Closed
Conversation
Sometimes there are multiple devices using the same mDNS instance name on the network. Then the name is not stable due to conflict resolution and it is thus not really usable. However, when unique text records are present, it is possible to filter for them. This change implements this and makes it for example possible to use: gensiot 'mdns(type=_console._tcp,txt=serial:1234)' I used the ':' character to split the mDNS txt key and value, to keep the existing parsing. It is also possible to specify more than one txt filter, then all must match (AND logic). Signed-off-by: Michael Heimpold <mhei@heimpold.de>
Contributor
Author
|
I don't have a test ready yet, so I mark this PR as draft for now. |
Owner
|
This is good, it makes sense. I don't have a lot of practical experience with MDNS; following the specs doesn't always tell you everything. I tried to make it easy to use. |
mdns_timeout() was calling mdns_handle_err() on the GE_NOTFOUND path, but mdns_handle_err() already releases the lock. The timeout callback then fell through to its own mdnsn_deref_and_unlock(), which meant the timeout path was effectively unlocking twice. And this triggered a pthread_mutex_destroy() assertion. Signed-off-by: Michael Heimpold <mhei@heimpold.de>
Signed-off-by: Michael Heimpold <mhei@heimpold.de>
Contributor
Author
I think it couldn't be easier from user perspective. I've now added tests, but to be honest - Python skills are not really existing on my side. So thanks for carefully checking. |
Owner
|
Tests look good. The patches look good, good use of existing infrastructure and style. I tested on MacOS and Windows and it works fine. Those are completely different stacks and they have some different behavior, so it's good they worked. Thanks for doing all this work. It's a good improvement. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Sometimes there are multiple devices using the same mDNS instance name on the network. Then the name is not stable due to conflict resolution and it is thus not really usable.
However, when unique text records are present, it is possible to filter for them. This change implements this and makes it for example possible to use:
gensiot 'mdns(type=_console._tcp,txt=serial:1234)'
I used the ':' character to split the mDNS txt key and value, to keep the existing parsing. It is also possible to specify more than one txt filter, then all must match (AND logic).