-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update receiver_socket_udp.py #51
Conversation
Proposed fix for Hundemeier#47 - allowing for differences between Windows and Linux. Noting that I'm not clear that the Windows implementation is needed; but I don't have the ability to test that.
Thanks for the PR! However, it might be sufficient to simply use the sacn.sACNreceiver(bind_address='') You PR would disable the |
This definitely does not work! There are a few reasons. Specifically, the join multicast socket code requires the correct interface IP to be specified. Even specifying 0.0.0.0 will cause the multicast group membership to be listed on the first interface on the box and not on any others. Secondly, the aton call requires an IP - and a blank string doesn't meet the requirements for this so it will fail. It's questionable, to be honest, if the bind address works at all on Linux in the current form. It may indeed work for unicast sACN, but this is not technically within standard. Certainly - on a single interface box it works because you are binding to the first interface and the multicast group membership is there; but if your receive interface is not the first on the box it definitely does not work in its current form. |
But for clarity - this was one of the first things I tried, because a fix that did not require a patch or a different library was ideal. The change that I've proposed also still works on single IP interfaces; but I don't know the impact of it on unicast scenarios as I do not have any equipment that will send a unicast sACN stream. That said, I believe that is the only circumstance where this fix wouldn't be ideal; because in all other cases the multicast group join socket option binds the IP. |
After reading #47 as well I understand the issue a bit better. Maybe this whole issue is a regression from #42 , that changed something different. sacn.sACNreceiver(bind_address="1.2.3.4") combined with a reverted #42?
to socket.SOL_IP instead of socket.IPPROTO_IP . The same with
|
Sorry for the multiple messages - one more comment is that the bind_address is still used; in the building of the struct that is passed with the multicast address. In the scenario with multiple interfaces it is necessary to bind the listener to the correct interface by passing the correct IP there. It is only when the socket is also created bound to that IP explicitly that it fails. |
I tried this change in an earlier attempt to fix it/understand the problem and it behaves identically to the current non-working behaviour; as in it still does not work. |
It seems that your tests and this PR are currently the best fix for this behavior. Then I will merge this PR, create a new version, and upload it to PyPI. |
Update README to indicate that the bind_address parameter does not bind to the socket on Linux (but is still used to determine the correct interface to join the Multicast group) as per request from @Hundemeier
Apologies for taking some time to get back to this - but I've just done this today! |
Looks good to me, even if the checks fail with
Which is unrelated to this PR and I don't understand why it gets flagged.
No worries, I'm not one to respond fast either. |
I think the linting error is because the if statement is inside square brackets, so the line formatting doesn't need the \ escape. |
Yes, that particular flake8 error was added recently. I've changed sacn/sacn/sending/sender_handler.py Lines 47 to 48 in 8d807be
So the new version |
Proposed fix for #47 - allowing for differences between Windows and Linux.
Noting that I'm not clear that the difference between the Linux and Windows implementation is needed; but I don't have the ability to test that; so I've added use of the platform module to behave differently between the two operating systems.