Hi there!

A while ago you integrated my fork of the isotp-c library in this project.
I just wanted to inform you that several updates have been published since, and I thought it would be good to inform you.

The latest version is v1.1.1.

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.

Hello I want to use this library in my C++ program. Does it compatible? Or can I use it in C++ ?

Hello @driftregion again,

after upgrading to the new release v0.6.0 and compiling with the flags -Wall and -Werror I get these two errors:

1. [-Werror=int-to-pointer-cast]

 iso14229/iso14229.c:517:22: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 517 |     *memoryAddress = (void *)tmp;

Regarding this error, does the tmp variable has to be of the type long long unsigned int? When I remove one long to make it long unsigned int, it compiles with no warnings (errors) - when using the -Werror flag warnings are made to errors. I would highly recommend to keep the standard of the library to compile with no warnings, no matter how "trivial".

2. [-Werror=stringop-truncation]

armv7vet2hf-neon-poky-linux-gnueabi/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));

First, I appreciate changing to the more secure variant strncpy(). I could fix this warning by changing the line 239 of iso14229.c from:

strncpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));

to

strncpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name) - 1);

Let me know what you think about these warnings and whether you think these changes are worth making.

hello, @driftregion very big follower of your work sir

Iam having a bit hard time figuring out how to make this 14299.c, 14299. h to work with PCAN,
1)I have pcan installed in my system but I don't know how to make it work with code 14299. c since it does not have main function.

2)My main Aim is to check the data send from PCAN with our code for the respective diagnostic services (UDS) .
3) I would appreciate any help you could give me on how to do it.

Regards.

the following might not be a real issue, but i would like to discuss the observation. We have a situation in which we act as a tester and the other nodes go into reboot/health check after an update. We then poll repeatedly to be aware when the nodes have rebooted. We needed to request idle in this situation, otherwise the lib stayed in UDS_ERR_BUSY even after the nodes had been back - no issue this is according to the examples.
However when we repeatedly send SendRDBI requests to get the status of the nodes while they are still not present, we get an oscillating pattern of errors: UDS_ERR_TIMEOUT, UDS_ERR_TPORT, UDS_ERR_TIMEOUT, UDS_ERR_TPORT etc.
we tried to understand how this happens and enabled the debug outputs:
client state: AwaitSendComplete (2) -> AwaitResponse (3)
read failed: -1 with errno: 70
timeout: 1
client state: AwaitResponse (3) -> Idle (0)
client state: Idle (0) -> Sending (1)
Condition met: client->send_size (3, 3), == ret (3)
Sending (1) -> AwaitSendComplete (2)
client state: AwaitSendComplete (2) -> AwaitResponse (3)
timeout: 1
client state: AwaitResponse (3) -> Idle (0)
awaiting idle failed with err = 1 and msg = 'UDS_ERR_TIMEOUT'
client state: Idle (0) -> Sending (1)
tport err: -1, nsend=3 bytes
Condition met: client->send_size (3, 3), == ret (3)
client state: Sending (1) -> AwaitSendComplete (2)
awaiting idle failed with err = 6 and msg = 'UDS_ERR_TPORT'
in the code imho this happens in:
case kRequestStateSending: {
UDSTpAddr_t ta_type = client->_options_copy & UDS_FUNCTIONAL ? UDS_A_TA_TYPE_FUNCTIONAL
: UDS_A_TA_TYPE_PHYSICAL;
UDSSDU_t info = {
.A_Mtype = UDS_A_MTYPE_DIAG,
.A_TA_Type = ta_type,
};
ssize_t ret = UDSTpSend(client->tp, client->send_buf, client->send_size, &info);
if (ret < 0) {
client->err = UDS_ERR_TPORT;
UDS_DBG_PRINT("tport err: %zd\n", ret);
} else if (0 == ret) {
UDS_DBG_PRINT("send in progress...\n");
; // 等待发送成功
} else if (client->send_size == ret) {
changeState(client, kRequestStateAwaitSendComplete);
} else {
client->err = UDS_ERR_BUFSIZ;
}
break;
and we are puzzled how ret < 0 can be true and shortly after client->send_size == ret??
Any idea from your experience?

Hello.

Is it possible to add https://github.com/mguentner/cannelloni/blob/master/doc/tcp_protocol.md as can-bus interface?
I want to use your library on device(MCU) where is only TCP/IP connectivity.

Thank you

Hi again. I can not unpack request seed data by UnpackSecurityAccessResponse function.

Here is my debug output;

Could you give any example about security access level1 and level 2?

Addition:

I can request seed from the server, but, when I have to send encrypted data, I can not send concurrent data.

Is the problem timing? I think it is very very fast.

Hello again. I am using SequenceRunBlocking method, and I am trying the example codes.

Here are my sequences,

My problem is, I am sending DiagnosticSessionControl, awaitResponse is OK. But whenever I want to send CommunicationControl, It sends DiagnosticSessionControl again. I debugged the code row-by-row, it triggers CommunicationControl function, but it sends DiagnosticSessionControl.

Edit: Plase check CAN_MESSAGE_QUEUE_SIZE. Because in my code It was 1. Try to higher value.

Hello. Thanks for the library.

I have a question about handling responses. Im currently sending uds data and it looks OK. But, How can I handle the response? I mean,
How can I use the mockClientCANRxPoll function?

I modified mockClientSendCAN with HAL library for sending data. But I can not manage mockClientCANRxPoll.

Sorry for my bad English. Have a good day.

Edit1:

I tried something like this but It does not work

static enum Iso14229CANRxStatus mockClientCANRxPoll(uint32_t *arb_id, uint8_t *data, uint8_t *dlc) {
    
	HAL_CAN_GetRxMessage(&hcan1, CAN_RX_FIFO0, &rxHeader, TxData);


	*arb_id = rxHeader.ExtId;
	*dlc = rxHeader.DLC;
	memmove(data,TxData,*dlc);

    return kCANRxNone;
}

Hi.
I have a question about TransferData with blocksize 128. I want to send data from client to server 128 byte packets. But I could not manage to use kISO14229_CLIENT_CALLBACK_PENDING and kISO14229_CLIENT_CALLBACK_DONE. How can I handle 128 byte packet is sent? I want to see positive TransferData response but I could not see.

I'm using SD CARD with FATFS. But TransferDataStream is using iostream. I could not use TransferDataStream with FATFS.

My first question is, Is there any other else TransferData function with blocksize 128?
Second question is, how can I use TransferDataStream with FATFS ?

Edit1: I am using iso14229ClientSequenceRunBlocking.

Hi, I'm trying to receive responses to a functional request (Diagnostic Session Control = Programming Session and RDBI) from different ECUs.

I know it's possible to send a functional request by setting the FUNCTIONAL bit of the options byte of the client before sending a request:

client->options |= FUNCTIONAL;

I can also receive one response, but, how can I receive ALL responses from the different ECUs on the same CAN bus?

Let's say there is three ECUs and I send a functional RDBI request to know the hardware versions of all of them: How could I receive he responses from all of them? Would I have to manually set the state of the client to kRequestStateAwaitSendComplete and wait for it (UDSClientPoll) to be IDLE again after receiving from the socket?

I would highly appreciate some help with this question, thanks in advance!

Hi @driftregion ,

when sending a physically addressed RDBI request to an ECU, that answers with a multi-frame RDBI response, the client sends two flow control frames, one that is physically addressed and the other functionally. Here is a log with candump:

(1688347721.008952) can0 18DAF140#103562FD13584355  /* response to RDBI request */
(1688347721.009126) can0 18DB00F1#301003  /* functionally addressed response even if the request was physically addressed */
(1688347721.009231) can0 18DA40F1#301003 /* physically addressed response */
(1688347721.012025) can0 18DAF140#215F544848000000

Sending two flow control frames makes the ECU answer twice, which is unnecessary and unwanted.

I'm not able to explain myself why this happens since in the function tp_recv the code for reading from the socket reads from the physical link first:

struct {
        int fd;
        UDSTpAddr_t ta_type;
    } arr[] = {{impl->phys_fd, kTpAddrTypePhysical}, {impl->func_fd, kTpAddrTypeFunctional}};
    for (size_t i = 0; i < sizeof(arr) / sizeof(arr[0]); i++) {
        ret = read(arr[i].fd, buf, count);
        ...
        ...

After reading from the physical link, the socket should get the answer it needs and then not read another time from the functional link. Do you have an idea why this could be happening? If you need more info to the context or execute code, just let me know and thanks in advance!

Hi @driftregion iam facing an issue when working with the flexcan to uds currently this is how my initialization looks please guide me how to approach it :
1)Should i link the FLEXCAN to isotp-c to uds ?
2)i have made my custom flow control Should i directly link my flexcan to UDS ? [current approach]
i have been developing the code portable uds since last 6 months if possible can we connect and discuss ? please send me a mail to [email protected] so i can setup a teams meeting.

Hello @driftregion ,

I tested creating some UDS clients with different interface names like "can2" or something like "can1234" to be surprised that the client could still be created and that it could send over the only available interface "can0". Then I realized that the variable struct ifreq ifr in the function static int LinuxSockBind() in the file iso14229.c is declared but its memory is not cleared before using it. After that, the return value of the call to ioctl() is not checked for errors like in the calls to socket(), setsockopt(), and bind() within the same function.

I think it did work because before I did these tests I had chosen the right interface name "can0" so the memory of the struct got the right index for interface but in another call with a wrong interface name, the call to ioctl was not successful but since the uncleared memory still uncleared the valid index for "can0" the client could still be bound to the right interface. I changed the code to:

    struct ifreq ifr;
    memset(&ifr, 0, sizeof(ifr));
    strncpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name) - 1);
    if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0) {
        printf("ioctl: %s %s\n", strerror(errno), if_name);
        return -1;
    }

and this prevents the problem and is a cleaner way of dealing with the struct and the call to ioctl. The warning message pattern was taken from the call to bind() after it.

I see that within the same function you use either perror or printf() to report errors, is there a reason for that? Wouldn't it be better to keep it consistent?

What do you think about this change?