pylessard / python-udsoncan

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

ReadDataByIdentifier: no codec.encode() ? #213

Closed m31L3r closed 7 months ago

m31L3r commented 7 months ago

Hi,

I have the issue, that i need to send a ReadDataByIdentifier service request, but with data encoded in order to read specific dio-pins and memory adresses. In the ReadDataByIdentifier.make_request() method is, unlike in the WriteDataByIdentifier.make_request() method, no call to codec.encode().

req.data+=codec.encode(None)

Is this on purpose or is there another way implemented for ReadDataByIdentifier I did not recognize? If not, I will try to add this functionality.

Cheers

pylessard commented 7 months ago

No need to encode the data when requesting for a read. You need to decode when receiving the response

If you need to do a read and pass data, then what you are doing is not standard UDS. Assign a DID to each IO, or use DynamicDID.

m31L3r commented 7 months ago

You are probably right that it's non standard UDS. But that's the way it was implemented on the ECU, so I have to deal with it. I programmed a workaround, but thanks for your reply anyways.

pylessard commented 7 months ago

You may want to use this feature

https://udsoncan.readthedocs.io/en/latest/udsoncan/client.html#overriding-the-output

m31L3r commented 7 months ago

Thanks for the hint. I tried it but it doesn't really satisfy me like adding this snippet from WriteDataByIdentifier to ReadDataByIdentifier:

req.data = struct.pack('>H', did)  # encode DID number
if codec.__class__ == DidCodec and isinstance(value, tuple):
    req.data += codec.encode(*value)    # Fixes issue #29
else:
    req.data += codec.encode(value, cls._sid)
return req

Simply for the reason that I can add additional arguments to the encode function

pylessard commented 7 months ago

Okay sounds good. Drawback is that you have to maintain it and you'll eventually get out of sync with updates. You could get something not so far by passing a function to the override_payload method. Using a partial function, you can always make sure that the list of codec is available to that function. Then you recraft your payload with your custom logic and the client interpret the response like if it was a normal response.

Anyhow, you know what's best for your case. Cheers

m31L3r commented 7 months ago

Haven't thought about that tbh. But this seems to work quite nicely and not having to maintain it is actually a big plus.

Thanks again