pylessard / python-udsoncan

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

Add RawCodec support for ECU development process #137

Closed keelung-yang closed 12 months ago

keelung-yang commented 1 year ago

Hi, Thanks for developing UDSonCAN library! It saves many debugging time.

This is a feature-request for debugging DTC/DID.

The DTC/DID which are in developing are asked for many debugging among teams. They're not defined well before ready for production. So a RawCodec is need for debugging and verifying at least.

Such as in ReadDataByIdentifier.make_request(..., didconfig=), if key is missing in didconfig or even didconfig is None, then RawCodec should be used and logging.warning(response.data.hex(' ')) is called to tell users received DID value is not defined.

pylessard commented 1 year ago

Hmm, I get what you mean, but I'm not so sure it is a good idea. Problem is, the library helps the user to correctly implement the standard. Too much silent default behaviour may lead to unnoticed bad behaviour. Also, a codec specifies the length of the data, if this is missing, it is impossible to find subsequent Dids, as more than one can be encoded in the response, therefore DID and data can't be distinguished.

In your case, you can prefill your config with the default codec you want. By enabling logging in debug mode you will see all payloads. Also, there's a small trick where you can raise an exception in the codec len function to grab all the following data. Check the documentation for it. You can make all your dids use a codec that raise this exception, like that, the udsoncan client won't complain.

keelung-yang commented 1 year ago

I see. It's the distinction between End User and ECU developper. For the end users, all DTC/DID should be well defined. But for developer & validation engineer, there are always some DTC/DID are not meet the requirements almostly.

And yes, it's not a good idea to pass undefined DTC/DID silently & defautly even if logging.warning() called, no matter who is an end user or developer or validation engineer.

Anyway, I still think a common RawCodec is needed if users want to use it, means passing it to make_request() explicitly. So if there is a RawCodec, it shouldn't be a user defined codec since it's generic to all users.

I'll check the documentation & think & practice more once I'm back to our UDS project after about 2 weeks, since I'm in another project and no more time to this.

harambe-88 commented 1 year ago

I was looking to do something that is (maybe) similar to this.

We're utilizing a small part of the UDS-2020 standard (the $29 Service for PKI-based Authentication). But creating an entire class to handle it is a little more than I can easily start with. I was hoping to make a few manually-packed UDS commands first.

For example, sending the boiler-plate start "Request Authentication Configuration" ("29 08") and getting back the response for "Authentication Configuration APCE" ("69 08 02").

However, when I try to pack raw UDS messages like that I get back the "Service unknown" exception. I was wondering if there's a way for me to generate something similar to Request.from_payload([0x29, 0x08]) that temporarily overrides this and allows it.

Maybe what I want is just the underlying CAN-TP with python-can, I'm not sure. Thank you in advance, love the library!

pylessard commented 1 year ago

@harambe-88 : HAve you looked at the latest release. You can extend the base service with your own class since this PR

Otherwise, I suggest you interract directly with the ISOTP layer, because that is what you want to do: sending arbitrary non-standard payloads. You can even make some nice wrapper around it that make it as beautiful as the uds client :)

pylessard commented 1 year ago

I think we should close this issue.