pylessard / python-udsoncan

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

Performance tests #200

Closed pompushko closed 8 months ago

pompushko commented 8 months ago

Hello

I use IsoTPSocketConnection for own project. My CanBus speed is 500kb\s

When I trying to make some tests with one request from ECU, I have such speed 100%|###########################################| 100 of 100 Time: 0:00:00 107.7 it/s

response = client.read_data_by_identifier(0x1000)

image

image

With PythonIsoTpConnection too slow :D

image

But with cangen I can load almost fully my CAN-bus: image

Main question, is that possible to make faster transmission? As I know, Python is limited by 1 code only. But I have 4. I tried to compile my test app with Cython to use all my CPU and that didn't helped me :)

Thank you

pylessard commented 8 months ago

Download latest isotp package (v2.0.3) Update udsoncan to 1.21.1 (1.21.2 will release soon)

You should get an improvement

pylessard commented 8 months ago

Just so you know, when you'll update udsoncan, make sure to pass an isotp.Address if you use the IsotpSocketConnection. IT is now required. See https://udsoncan.readthedocs.io/en/latest/udsoncan/questions_answers.html#my-isotpsocketconnection-raises-an-error-after-updating-udsoncan

pompushko commented 8 months ago

Hmm

I have:

udsoncan             1.20.1
can-isotp            1.9

Let me check. Thank you :)

pompushko commented 8 months ago

Just so you know, when you'll update udsoncan, make sure to pass an isotp.Address if you use the IsotpSocketConnection. IT is now required. See https://udsoncan.readthedocs.io/en/latest/udsoncan/questions_answers.html#my-isotpsocketconnection-raises-an-error-after-updating-udsoncan

Well, This is strange, I have this code: conn = IsoTPSocketConnection(args.interface, isotp.Address(isotp.AddressingMode.NormalFixed_29bits, rxid=0xF1, txid=args.ecuaddress), tpsock=tpsock)

And I still get: RuntimeError: Provide an isotp.Address to the IsoTPSocketConnection. The interface has changed in a non-backward compatible way and this is now required.

pylessard commented 8 months ago

Are you sure this line raise the issue. check your stack trace. image

pompushko commented 8 months ago

Well.... Yes 95 line...

image

image

pylessard commented 8 months ago

I don't know what to say. Something else is going on. The test condition is quite straight forward, I even have unit test that validates it.

print kwargs inside the IsotpSocketConenction constructor please.

pompushko commented 8 months ago

I don't know what to say. Something else is going on. The test condition is quite straight forward, I even have unit test that validates it.

print kwargs inside the IsotpSocketConenction constructor please.

Could you, please, tell me how to do that? Thank you

pylessard commented 8 months ago

edit the file defining IsotpSocketConnection. You can find it by looking at udsoncan.connections.__file__

then do print(kwargs) inside of __init__

pompushko commented 8 months ago

Well. I found the problem. Fixed, my bad

Unfortunately, my tests give me same performance :(

pylessard commented 8 months ago

What do you pass to PythonIsoTPConnection? Try a notifierBasedCanStack.

pompushko commented 8 months ago

I use IsotpSocketConnection , not PythonIsoTPConnection

pylessard commented 8 months ago

You opened an issue saying that the PythonIsotpConnection was slower than IsoTPSocketConenction.

What are you asking?

pompushko commented 8 months ago

Im asking, is that possible to use all CPU cores when using IsoTPSocketConnection or PythonIsoTpConnection. Or make faster transmission...

pylessard commented 8 months ago

Using more CPU cores will not give you a faster transmission. The bottleneck is not the CPU.

To have a faster transmission, start by identifying the cause of the slowness. IsoTP protocol has some timing parameter that can be adjusted. Most likely that the receiving party is requiring the sender to not go too fast.

Start by sharing a can log of a transmission and indicate what you consider slow.

pylessard commented 8 months ago

Closing this issue. reopen if needed