search

ffnord / mesh-announce

Discussion at #mesh-announce:irc.hackint.org and (separately) at
https://matrix.to/#/!MjLIHcALOcENXZWQlH:irc.hackint.org/$1547640760901FmKaD:matrix.eclabs.de
13 stars 45 forks source link

Allow for per interface multicast group specification #43

Closed TobleMiner closed 5 years ago

TobleMiner commented 5 years ago

Gluon has started using multiple multicast groups for respondd data querying in 2017. This PR implements flexible per interface multicast group specification allowing for use of an arbitrary number of multicast groups.

Preparation to fix #42

jplitza commented 5 years ago

Shouldn't -g <group> be optional then? Or even deprecated?

TobleMiner commented 5 years ago

@jplitza -g <group> is already optional isn't it? It would now serve the purpose of setting the default multicast group for all interfaces where there is no multicast group set explicitly. Handling -g this way makes the commandline interface fully backward compatible.

jplitza commented 5 years ago

@TobleMiner Neither according to the help message (the [] is around -g <group> and -i <iface>, so if you want to use the latter, you have to use the first - which made sense before this PR) nor according to the code around line 75: https://github.com/ffnord/mesh-announce/pull/43/files#diff-c22b6013c8cb3b1d2909f0f7b7b37d13R75

TobleMiner commented 5 years ago

According to https://github.com/ffnord/mesh-announce/blob/master/respondd.py#L50 there is a default value set for args.group thus it is optional. The documentation is indeed quite missleading. I'll fix that in this PR since using a separate PR would just result in more merge conflicts and this commit already touches the sematics of -g <group>.

TobleMiner commented 5 years ago

I'd like to merge this. Any objections?

TobleMiner commented 5 years ago

Merged since PR is fully backward compatible