{"action":"created","issue":{"url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/6","repository_url":"https://api.github.com/repos/pylessard/python-udsoncan","labels_url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/6/labels{/name}","comments_url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/6/comments","events_url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/6/events","html_url":"https://github.com/pylessard/python-udsoncan/pull/6","id":354042725,"node_id":"MDExOlB1bGxSZXF1ZXN0MjEwOTIwODk2","number":6,"title":"Add support for python-can","user":{"login":"christiansandberg","id":3647996,"node_id":"MDQ6VXNlcjM2NDc5OTY=","avatar_url":"https://avatars3.githubusercontent.com/u/3647996?v=4","gravatar_id":"","url":"https://api.github.com/users/christiansandberg","html_url":"https://github.com/christiansandberg","followers_url":"https://api.github.com/users/christiansandberg/followers","following_url":"https://api.github.com/users/christiansandberg/following{/other_user}","gists_url":"https://api.github.com/users/christiansandberg/gists{/gist_id}","starred_url":"https://api.github.com/users/christiansandberg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/christiansandberg/subscriptions","organizations_url":"https://api.github.com/users/christiansandberg/orgs","repos_url":"https://api.github.com/users/christiansandberg/repos","events_url":"https://api.github.com/users/christiansandberg/events{/privacy}","received_events_url":"https://api.github.com/users/christiansandberg/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-08-25T20:53:59Z","updated_at":"2018-08-31T01:34:25Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/pylessard/python-udsoncan/pulls/6","html_url":"https://github.com/pylessard/python-udsoncan/pull/6","diff_url":"https://github.com/pylessard/python-udsoncan/pull/6.diff","patch_url":"https://github.com/pylessard/python-udsoncan/pull/6.patch"},"body":"This pull request adds support for many CAN interfaces and operating systems through the python-can package + a basic ISO-TP implementation. I realise this library should be transport agnostic but since the majority of UDS implementations are using CAN and ISO-TP, I'm hoping this will be OK to include anyway. Otherwise the isotp module could perhaps be a separate package.\r\n\r\nI've got some more testing to do but I'm opening this now for discussion.\r\n\r\nFixes #2."},"comment":{"url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/comments/417521338","html_url":"https://github.com/pylessard/python-udsoncan/pull/6#issuecomment-417521338","issue_url":"https://api.github.com/repos/pylessard/python-udsoncan/issues/6","id":417521338,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNzUyMTMzOA==","user":{"login":"pylessard","id":2386969,"node_id":"MDQ6VXNlcjIzODY5Njk=","avatar_url":"https://avatars1.githubusercontent.com/u/2386969?v=4","gravatar_id":"","url":"https://api.github.com/users/pylessard","html_url":"https://github.com/pylessard","followers_url":"https://api.github.com/users/pylessard/followers","following_url":"https://api.github.com/users/pylessard/following{/other_user}","gists_url":"https://api.github.com/users/pylessard/gists{/gist_id}","starred_url":"https://api.github.com/users/pylessard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pylessard/subscriptions","organizations_url":"https://api.github.com/users/pylessard/orgs","repos_url":"https://api.github.com/users/pylessard/repos","events_url":"https://api.github.com/users/pylessard/events{/privacy}","received_events_url":"https://api.github.com/users/pylessard/received_events","type":"User","site_admin":false},"created_at":"2018-08-31T01:34:25Z","updated_at":"2018-08-31T01:34:25Z","author_association":"OWNER","body":"Correct, we're not doing any asynchronous exchange and I don't plan to add support for this soon. Still, I believe having the reception in a thread will make the software more robust against race condition, or simply less restrictive for the programmer. Imagine you send a request, then your OS switch task, it may take 16-32ms on a windows machine. An embedded device largely have time to receive the request, start responding and timeout.  Also, it is hard to manually test the connection without this :)\r\n\r\nWhat I suggest is just to move the receive logic into a thread, and put the received data into a queue, then wait_frame() reads that queue. Actually, reception and transmit logic could go in that task. That would imply very few modification to the code and will add robustness timing-wise.\r\n\r\nSocketCAN works like this, you can sends many frames into the socket, each frames are queued on the other side until they are read.\r\n\r\nLast thing, I tested the IsoTP connection against harktopp/can-isotp module through a vcan interface. That worked, but when I slowed down the transmission I started hitting a strange issue. Actually, try setting stmin to 50ms. 4 times out of 5, your stack complains about a missing flow control message, but candump shows that it was there. It seems that python-can receive nothing. I haven't investigated too much yet."}}
{"actor":{"display_login":"pylessard"}}