Skip to content
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

Allow source_interface selection when using get_entity and await_vehicle_announcement #33

Open
HarshaLaxman opened this issue Jul 6, 2023 · 2 comments

Comments

@HarshaLaxman
Copy link
Contributor

HarshaLaxman commented Jul 6, 2023

#17 (comment)

Currently source_interface can only be selected for IPV6 and is only set to INADDR_ANY for IPV4. The recently added get_entity should be able to specify source_interface so you can rely on the default local broadcast addr rather than setting the directed broadcast addr.

HarshaLaxman added a commit to HarshaLaxman/python-doipclient that referenced this issue Jul 6, 2023
@jacobschaer
Copy link
Owner

Was there supposed to be an PR for this? The general annoyance with allowing "by NIC" instead of "by address" operations is it's not super portable. On Linux it would be using SO_BINDTODEVICE and accepts an interface name. On Windows, I don't believe this is supported. In either case you can bind by source IP in the bind() which will choose the NIC that posseses that address. This is the most portable way to do it though it requires that you manually assign an address or rely on the vehicle to lease you an address via DHCP. In the linked commit I see you tried to use a name instead of an IP in the bind() call but I believe that's not supported by AF_INET and you'll get a socket-error.

You should be able to portably do what is desired using purely the OS routing functionality. On newer Windows, use the route command https://www.howtogeek.com/22/adding-a-tcpip-route-to-the-windows-routing-table/ to ensure that the network destination 255.255.255.255 has the lowest metric assigned to your desired network adapter and it will become the default specifically for that broadcast address. I believe similar can be done on Linux. On older Windows there was a GUI for this if memory serves.

I'm ok with adding an option to optionally pass to SO_BINDTODEVICE - I believe this requires elevated privileges by the caller. It would need to be documented that it's only really useful on Linux like OS and may require elevated permissions. Even importing SO_BINDTODEVICE might not be portable (it's normally 25) so that might be annoying too.

@HarshaLaxman
Copy link
Contributor Author

HarshaLaxman commented Jul 10, 2023

Ah oops I rushed it and forgot to test it -> open the PR, and used bind wrong...😅

Didn't realize I was looking at Raw sockets example, I thought Python kindly called SO_BINDTODEVICE for us https://docs.python.org/3/library/socket.html#:~:text=.CAN_RAW)-,s.bind((%27vcan0%27%2C)),-while%20True%3A

Seeing as this is a nice to have and I don't want to get in the Windows weeds, I'm content to leave it as a "won't do" unless someone else wants to do it :)

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

No branches or pull requests

2 participants