pylessard / python-udsoncan

Python implementation of UDS (ISO-14229) standard.
MIT License
564 stars 195 forks source link

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

Closed Tymon13 closed 6 months ago

Tymon13 commented 6 months ago

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 commented 6 months ago

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

pylessard commented 6 months ago

https://github.com/pylessard/python-udsoncan/releases/tag/v1.21.0

Tymon13 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

pylessard commented 6 months ago

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

pylessard commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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