driftregion / iso14229

ISO 14229 (UDS) server and client for embedded systems
MIT License
227 stars 79 forks source link

Implementations of function static int LinuxSockBind() and static int LinuxSockTpOpen() do not allow uint32_t rxid and txid #12

Closed muehlke closed 1 year ago

muehlke commented 1 year ago

Thanks for the library. I appreciate your effort into this report @driftregion .

While trying to use your library to send/receive CAN frames with a 29 bit CAN ID, I realized that they would be casted to a unit16_t and this would then, off course, be a wrong address. I realized that it is because of how you implemented static int LinuxSockBind(const char *if_name, uint16_t rxid, uint16_t txid) and static int LinuxSockTpOpen(UDSTpHandle_t *hdl, const char *if_name, uint16_t phys_rxid, uint16_t phys_txid, uint16_t func_rxid, uint16_t func_txid). The rxid and txid just allow for 16 bit IDs even though the physical and functional send and receive IDs of UDSClientConfig_t are uint32_t.

My question is: Is there a special reason for it? CAN allows for the EFF (Extended Frame Format) flag to be set on the MSB of the 32 bit CAN ID to allow 29 bits for the CAN ID of the CAN frame. I then changed the function definition to static int LinuxSockBind(const char *if_name, uint32_t rxid, uint32_t txid) and static int LinuxSockTpOpen(UDSTpHandle_t *hdl, const char *if_name, uint32_t phys_rxid, uint32_t phys_txid, uint32_t func_rxid, uint32_t func_txid) and it worked as needed. I was wondering if that is something that could be changed in the repo for future users or if the rxid and txid are uint16_t for a specific reason that I'm not able to deduce.

Thanks in advance and great job implementing the ISO14229 standard! It's been of great help.

driftregion commented 1 year ago

Hi @muehlke , no special reason. Let's use uint32_t!

Does this work? https://github.com/driftregion/iso14229/pull/13

muehlke commented 1 year ago

This is perfect! Thanks a lot! So fast! You can merge it then.

muehlke commented 1 year ago

Sorry, I realized something is still missing in the commit 28a3b54. Please check the comments on the commit.

muehlke commented 1 year ago

thanks for the last modification!