-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider all network interfaces when checking HOST option. #245
Conversation
Signed-off-by: Carlos Agüero <[email protected]>
Thanks, it works. I tested both the simple example and a more complex one with the comms model visualization service in SubT. I have just one comment to the implementation: |
What do you suggest? Anything that starts with |
Yes, that's my suggestion. |
Signed-off-by: Carlos Agüero <[email protected]>
Sure, updated in fc1c7da. |
@@ -898,6 +898,10 @@ namespace ignition | |||
this->SendUnicast(msg); | |||
} | |||
|
|||
bool isSenderLocal = (std::find(this->hostInterfaces.begin(), | |||
this->hostInterfaces.end(), _fromIp) != this->hostInterfaces.end()) || | |||
(_fromIp.find("127.") == 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any need to make this also work for ipv6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not from my view, but generally it would be good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably out of scope for now.
Signed-off-by: Carlos Agüero [email protected]
🦟 Bug fix
Fixes the following issue:
One publisher sets
IGN_IP
to127.0.0.1
and advertises a topic withScope_T::Host
. With these settings, a default subscriber is unable to receive any messages.Summary
The problem was that we were only checking if the source IP received was matching the host address used by the receiver. Now, we're checking if the source IP received matches any of the network interfaces or localhost.
To test (from the
examples
directory:Terminal 1:
IGN_IP=127.0.0.1 ./publisher
Terminal 2:
./subscriber
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge