[FEATURE] Add a timeout to TransportArbitrator for clientReceive #367

Activity

1. adadaadadade

   added

   enhancement

   on Jul 17, 2023
2. github-actions commented on Jul 17, 2023

   

   github-actionsbot

   on Jul 17, 2023

   More actions

   Hi eRPC user. Thank you for your interest and welcome. We hope you will enjoy this framework well.
3. djmuhlestein commented on Apr 14, 2025

   

   djmuhlestein

   on Apr 14, 2025

   More actions

   Related to this request, Serial Transport doesn't actually ever return a timeout error code when the read times out. Is the following a good change to erpc_serial_transport.cpp?

    erpc_status_t SerialTransport::underlyingReceive(uint8_t *data, uint32_t size)
    {
        uint32_t bytesRead = serial_read(m_serialHandle, reinterpret_cast<char *>(data), size);
   -
   -    return (size != bytesRead) ? kErpcStatus_ReceiveFailed : kErpcStatus_Success;
   +    return (size != bytesRead) ? 
   +           ((bytesRead == 0 && m_vtime > 0) ? kErpcStatus_Timeout : kErpcStatus_ReceiveFailed) :
   +           kErpcStatus_Success;
    }
   

   I added m_vtime to capture the vtime parameter from init. The idea being if vtime is > 0, the read can return with less bytes than were requested if it times out.

   Part of the issue is that code in the Transport Arbitrator checks the return code for timeout to to unblock the other client requests. So maybe a better fix is to just patch that to check anything other than success to run the clean up code.
4. xianyo commented on Dec 5, 2025

   

   xianyo

   on Dec 5, 2025

   More actions

   Will this problem cause the whole system to lock up and not communicate? Only in Transport Arbitrator does it appear up there? Nothing else Arbitrator it's normal, right?
5. adadaadadade commented on Dec 6, 2025

   

   adadaadadade

   on Dec 6, 2025

   Last edited by adadaadadade

   Author

   More actions

   Will this problem cause the whole system to lock up and not communicate? Only in Transport Arbitrator does it appear up there? Nothing else Arbitrator it's normal, right?

   @xianyo

   I use TransportArbitrator to create a bidirectional connection. Device and PC both has a client and a server. The way erpc implements arbitrator on one side is like: 1. Client call a function, send a request to hardware transport and wait for semaphore from server. 2. Server is polling, so it is always trying to receive a message , and if it is a call request from client, then process the related function and send the reply, and if it receives a reply, then this reply is must for a client, it will find the client and active it by emit the semaphore. 3. Client receives semaphore and reply from server and it can handles the return value of the function.

   The problem is that sometimes transport is unreliable, so if call request or the reply which the client is waiting for is lost, the client will hold up, because it is waiting for a semaphore which is not going to be emitted. And the server will still be OK, because it is not blocked.

   The ways to tackle the problem:

   1. The best way is to make sure that the hardware transport is reliable enough.
   2. Use only oneway function on device client, because it will not wait for reply, but you can not make sure if this call is successful.
   3. Add a timeout and set request's status to timeout. but it maybe a bad way to fix it, and may cause more errors. And I didn't try it.
6. adadaadadade commented on Dec 6, 2025

   

   adadaadadade

   on Dec 6, 2025

   Author

   More actions

   Related to this request, Serial Transport doesn't actually ever return a timeout error code when the read times out. Is the following a good change to erpc_serial_transport.cpp?

    erpc_status_t SerialTransport::underlyingReceive(uint8_t *data, uint32_t size)
    {
        uint32_t bytesRead = serial_read(m_serialHandle, reinterpret_cast<char *>(data), size);
   -
   -    return (size != bytesRead) ? kErpcStatus_ReceiveFailed : kErpcStatus_Success;
   +    return (size != bytesRead) ? 
   +           ((bytesRead == 0 && m_vtime > 0) ? kErpcStatus_Timeout : kErpcStatus_ReceiveFailed) :
   +           kErpcStatus_Success;
    }
   

   I added m_vtime to capture the vtime parameter from init. The idea being if vtime is > 0, the read can return with less bytes than were requested if it times out.

   Part of the issue is that code in the Transport Arbitrator checks the return code for timeout to to unblock the other client requests. So maybe a better fix is to just patch that to check anything other than success to run the clean up code.

   I am sorry to reply to your comment so late. I read your comment at April but I forgot to reply.

   @djmuhlestein

   I think your way is better to add timeout for this case, it is more flexible. And when timeout is occurred the transport must receives all bytes the caller asks for or return zero bytes to make sure there is no data is unused.

   And it's necessary to unblock all pending client(s). Because if there is a timeout happened, which mean the request or reply is lost. And caller of client need to know what is happened.

   erpc_transport_arbitrator.cpp

   @@ -70,6 +70,7 @@ erpc_status_t TransportArbitrator::receive(MessageBuffer *message)
                    {
                        if (client->m_isValid)
                        {
   +                        client->m_request->getCodec()->updateStatus(kErpcStatus_Timeout);
                            client->m_sem.put();
                        }
                    }
   @@ -195,7 +196,7 @@ erpc_status_t TransportArbitrator::clientReceive(client_token_t token)
    
        removePendingClient(info);
    
   -    return kErpcStatus_Success;
   +    return info->m_request->getCodec()->getStatus();
    }

   Then client manager will call the error handler to notify client that there is a error happened.
7. xianyo commented on Dec 8, 2025

   

   xianyo

   on Dec 8, 2025

   More actions

   #469 fix？
8. adadaadadade commented on Dec 8, 2025

   

   adadaadadade

   on Dec 8, 2025

   Author

   More actions

   #469 fix？

   @xianyo yes