Skip to content

Fix timestamp handling in udp_multicast on macOS #1278

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

Merged
merged 10 commits into from
Apr 29, 2022
Merged

Conversation

felixdivo
Copy link
Collaborator

Fix #1275.

@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #1278 (e20e47b) into develop (af55b0a) will decrease coverage by 0.02%.
The diff coverage is 56.09%.

@@             Coverage Diff             @@
##           develop    #1278      +/-   ##
===========================================
- Coverage    66.02%   65.99%   -0.03%     
===========================================
  Files           86       86              
  Lines         8915     8943      +28     
===========================================
+ Hits          5886     5902      +16     
- Misses        3029     3041      +12     

@felixdivo
Copy link
Collaborator Author

@Gudoghe Can you check the fix? 🤔

You should be able to install it with pip install git https://github.com/hardbyte/python-can.git@felixdivo-fix-1275?

@felixdivo
Copy link
Collaborator Author

You should now also be able to run the tests, e.g. with:

pip install tox
tox -e py

Could you post the results for the test suite parts BasicTestUdpMulticastBusIPv4 and BasicTestUdpMulticastBusIPv6? They should now run on your system (and not be skipped any more).

@Gudoghe
Copy link

Gudoghe commented Mar 10, 2022

@felixdivo The command to install the fix seems to return an error 404, maybe there's some problem with the url?

@felixdivo
Copy link
Collaborator Author

Ah, jup, there's a missing +, sorry. This works for me: pip install git+https://github.com/hardbyte/python-can.git@felixdivo-fix-1275.

@Gudoghe
Copy link

Gudoghe commented Mar 10, 2022

The patch and tox were installed successfully but the tox -e py command gives me the following error:
ERROR: tox config file (either pyproject.toml, tox.ini, setup.cfg) not found

Sorry I'm quite new to this tool and environment.

@felixdivo
Copy link
Collaborator Author

Strange ... there should be one. Are you sure that you are running it from the repository root? pwd (on Unix) should give you something ending in /python-can.

Alternatively, you can also try tox -e gh one it works.

@Gudoghe
Copy link

Gudoghe commented Mar 11, 2022

Alright I managed to clone the patched repo and do a clean install of python-can from there. I then ran the tests on the cloned repository and all the tests regarding BasicTestUdpMulticastBusIPv4 and BasicTestUdpMulticastBusIPv6 failed, I'm attaching a screenshot.

Moreover I tried to run again the example from the Multicast page and the error i get back changed to the following:

Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/can/interfaces/udp_multicast/bus.py", line 264, in _create_socket
sock.bind(("", self.port))
OSError: [Errno 48] Address already in use

Schermata 2022-03-11 alle 16 09 12

@felixdivo
Copy link
Collaborator Author

Uhm, that's strange. Can you find out what is using that address, or use another one (suitable for multicast, like the default one)? I can't really debug it from here 🙃. But at least the exception from last time is avoided. I suppose the tests fail due to similar problems?

@Gudoghe
Copy link

Gudoghe commented Mar 15, 2022

I've tried looking at all processes running with the command ps -fA but nothing seems to be taking up that port.
I've tried changing the port to be assigned in the range of 43001-43187, as to my understanding this seems to be the proper range (https://www.speedguide.net/port.php?port=43113), but I now run into a new error:

Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/can/interfaces/udp_multicast/bus.py", line 273, in _create_socket
sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_JOIN_GROUP, request)
OSError: [Errno 49] Can't assign requested address

I'm really at a blank for a solution. If you had any suggestions on what to try next that'd be greatly appreciated.
Thank you again for all the support!

@felixdivo
Copy link
Collaborator Author

You're welcome. 😃 I am currently retreating from this repo but since it wrote that interface I figured that I should have a look at this.

Hm ... Are you sure that multicast works elsewhere? I cold not find a simple test command, but I did find this article. It's from 2009, but looks relevant.

@Gudoghe
Copy link

Gudoghe commented Mar 15, 2022

I've followed the instructions and I do find a couple of addresses in the multicasting range (239.255.255.250, 224.0.0.251) and if I ping them I get a response, so I assume the multicasting is working.

@felixdivo
Copy link
Collaborator Author

I'm a bit confused: In the exception above you use IPv6 (and it fails) and now you checked IPv4 multicast (and it works). Maybe, you can try IPv4 multicast by using something like DEFAULT_GROUP_IPv4? Maybe macOS only has problems with IPv6 multicast?

@felixdivo
Copy link
Collaborator Author

Okay, so this became kinda stale. I suggest merging at lest these changes, since they at least resolve some problems. If more occur, we can still keep discussing them and add more patches.

Side note: @zariiii9003 I wrote about reducing my contributions and why I do that on the mailing list 🙂

@felixdivo felixdivo marked this pull request as ready for review March 27, 2022 20:24
@felixdivo
Copy link
Collaborator Author

Black has a problem. We should just update (psf/black#2964). I added a commit for that to this PR.

@zariiii9003
Copy link
Collaborator

@felixdivo Do you know why the UdpMulticast tests are failing randomly?

@felixdivo
Copy link
Collaborator Author

Nope ... the type of error also confuses me.

@hardbyte hardbyte merged commit 19cf49a into develop Apr 29, 2022
@hardbyte hardbyte deleted the felixdivo-fix-1275 branch April 29, 2022 23:22
@felixdivo felixdivo modified the milestones: 4.0.1, 4.1.0 Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error with Multicast IP Interface
4 participants