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

IsoTPSocketConnection doesn't work with can-isotp versions 2 and up #199

Closed
Tymon13 opened this issue Jan 15, 2024 · 9 comments
Closed

IsoTPSocketConnection doesn't work with can-isotp versions 2 and up #199

Tymon13 opened this issue Jan 15, 2024 · 9 comments

Comments

@Tymon13
Copy link

Tymon13 commented Jan 15, 2024

can-isotp version 2.0 introduced a backwards incompatible change in tpsock.socket.open method (commit 1fff9e7ab5bd8404798200804ef0d482ac1dc536). Thus, IsoTPSocketConnection.open doesn't work (it's still using the old interface). There is no requirement in documentation that can-isotp module must be installed in any specific versions, so I guess it's a bug?

@pylessard
Copy link
Owner

udsoncan supports isotp v2 since 1.21 (released yesterday). Can you give it a try?

@pylessard
Copy link
Owner

@Tymon13
Copy link
Author

Tymon13 commented Jan 16, 2024

Exception we get:

TypeError: socket.bind() got an unexpected keyword argument 'rxid'
  At:
      /usr/local/lib/python3.10/dist-packages/udsoncan/connections.py(285): open
      /usr/local/lib/python3.10/dist-packages/udsoncan/client.py(134): open
# our own code goes there

The problem is on line 329 in connections.py: https://github.com/pylessard/python-udsoncan/blob/1622f251bfd8cbe7fcf7572dc737f6a125481257/udsoncan/connections.py#L329C1-L329C124
tpsock.bind is called with argument rxid, which it doesn't accept anymore. txid and rxid would have to be put into an instance of isotp.Address if I'm not mistaken, and the keyword arguments somehow unpacked and put there as well? (if they are to be still supported)

@pylessard
Copy link
Owner

Yes, that is one of the breaking change I did with isotp v2.
the bind interface was backward compatible with my first draft of the code, which goes back to 2017 and had no consideration for addressing type.

instead of socket.bind(txid=123, rxid=456). Do socket.bind(isotp.Address(AddressingMode.Normal_11bits, txid=123, rxid=456))

@Tymon13
Copy link
Author

Tymon13 commented Jan 16, 2024

Yes, that needs to be done in udsoncan/connections.py:329 :)

@pylessard
Copy link
Owner

pylessard commented Jan 16, 2024

You are absolutely right. I missed that. Will publish 1.21.2 tonight.

@pylessard
Copy link
Owner

I'm a bit busy, pushing that few days forward.
Reviewing my work, I did a bad manipulation with git and seems to have lost some work in the Connection module. There will be be more than just that that will change in the next release

@pylessard
Copy link
Owner

pylessard commented Jan 20, 2024

Give this a try please : https://github.com/pylessard/python-udsoncan/tree/fix-broken-isotp-socket-connection

Need to pass the isotp.Address object to the connection
Didn't have other choices than to break the interface.

@pylessard
Copy link
Owner

I did not wait on your test and went ahead.
Fixed in #201 Released in v1.21.2

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