UDS: implementation custom function working 4959 time but not more

[Benoit_V]

Hello,

I try to implement a custom service to transfert a custom data via UDS protocol.

My data takes 15 CAN frames to transfert (in consequence multiframe)

All works good during almost 4959 exchanges but in the number 4960 the software crashes.
And I don't understand why? What's wrong with my code.

I'm thinking a trouble inside the function: UDS_Read_2013()?
Or an internal variable not reinitialised.

I have updated this week with the latest:
- PCAN-BASIC.
- PCAN-UDS.
- PCAN-ISO-TP.
- PEAK CAN drivers

My code is:

Function

Code: Select all

bool uds::requestDatalog()
{
    uds_status status;
    uds_msg _msg_write;
    uds_msg response;
    uds_msg confirmation;
    uds_msg loopback_msg;
    bool _function_return = false;
    QString var_debug="";

    cantp_handle channel = s_tp_handle;
    uds_msgconfig config = s_config_physical;

    memset(&response, 0, sizeof(uds_msg));
    memset(&confirmation, 0, sizeof(uds_msg));
    memset(&_msg_write, 0, sizeof(uds_msg));
    memset(&loopback_msg, 0, sizeof(loopback_msg));

    //qDebug() << "\n*** UDS Service: requestDatalog ***\n";

    memset(&config, 0, sizeof(uds_msgconfig));
    config.can_msgtype = PCANTP_CAN_MSGTYPE_EXTENDED;
    config.nai.protocol = PUDS_MSGPROTOCOL_NONE;//PUDS_MSGPROTOCOL_ISO_15765_2_11B_NORMAL;
    config.nai.target_type = PCANTP_ISOTP_ADDRESSING_PHYSICAL;
    config.type = PUDS_MSGTYPE_UUDT;

    status = UDS_MsgAlloc_2013(&_msg_write,config,8);
    //qDebug() << "status alloc msgwrite = " << status;
    status = UDS_MsgAlloc_2013(&response,config,8);
    //qDebug() << "status alloc response = " << status;

    //msg_request.msg.msgdata.any->flags = PCANTP_MSGFLAG_ISOTP_FRAME;
    _msg_write.msg.can_info.can_id = (uint32_t)CAN_ID_UDS_REQUEST;
    _msg_write.msg.type = PCANTP_MSGTYPE_CAN;
    _msg_write.msg.msgdata.isotp->length = (uint32_t)8;
    _msg_write.msg.msgdata.isotp->data[0] = (uint8_t)2;
    _msg_write.msg.msgdata.isotp->data[1] = (uint8_t)0xBA;
    _msg_write.msg.msgdata.isotp->data[2] = (uint8_t)0x02;

    if(s_datalog_current_record_number >= 4959)
    {
        qDebug() << "BreakPoint !!! ";
    }

    _function_return = sentAloneUDSRequest(channel,&_msg_write,&response);

    qDebug() << " position = " + QString::number(s_datalog_current_record_number) + "\n";

    if(_function_return == true)
    {
        //qDebug() << "All is ok, start decode data";
        printf(" Tout s'est bien passé, on peut décoder la trame.  \n ");
        fflush(stdout);
        printf(" response.msg.msgdata.isotp length = %d  \n ", response.msg.msgdata.isotp->length);
        fflush(stdout);
        printf(" response.msg.msgdata.isotp->data[0] = %d  \n ", response.msg.msgdata.isotp->data[0]);
        fflush(stdout);
        printf(" response.msg.msgdata.isotp->data[1] = %d  \n ", response.msg.msgdata.isotp->data[1]);
        fflush(stdout);

        _function_return = false;

        if((response.msg.msgdata.isotp->data[0] == 0xFA) && (response.msg.msgdata.isotp->data[1] == 0x02))
        {
            s_datalog_current_record_number++;
			// DATA PROCESS
			//
        }
        else
        {
            var_debug = "\nERROR: Request a record has been failed: Service not iniialized or not data available.\n";
            emit valueChanged(var_debug);
        }
    }
    else
    {
        var_debug = "\nERROR: Request a record has been failed: No response.\n";
        emit valueChanged(var_debug);
    }
    status = UDS_MsgFree_2013(&_msg_write);
    //printf(" Free request message: %i\n", status);
    status = UDS_MsgFree_2013(&response);
    //printf(" Free response message: %i\n", status);
    status = UDS_MsgFree_2013(&confirmation);
    //printf(" Free confirmation message: %i\n", status);
    status = UDS_MsgFree_2013(&loopback_msg);
   // printf(" Free loopback_msg message: %i\n", status);

    return _function_return;
}

Function

Code: Select all

bool uds::sentAloneUDSRequest(cantp_handle channel, uds_msg *_msg_write, uds_msg *response)
{
    bool _ret = false;
    TPCANStatus     _status_result = (TPCANStatus)PUDS_STATUS_OK;
    uds_status status;
    uint32_t _timeout_recep = 0;
    uds_msg _uds_temp;
    uint32_t test_id_frame = 0;

    if(s_datalog_current_record_number >= 4959)
    {
        qDebug() << "BreakPoint !!! ";
    }

    memset(&_uds_temp, 0, sizeof(uds_msg));

    _status_result = (TPCANStatus)UDS_Write_2013(channel,_msg_write);
    status = (uds_status)_status_result;
    //qDebug() << "UDS_Write_2013 1  =  " << status << "\n";

    _timeout_recep = 0;
    while((test_id_frame != CAN_ID_UDS_ANSWER && (_timeout_recep < TIMEOUT_MAX_RECEPTION_FRAME)))
    {
        status = UDS_MsgFree_2013(&_uds_temp);
        memset(&_uds_temp, 0, sizeof(uds_msg));
        _status_result = UDS_Read_2013((cantp_handle)channel,&_uds_temp,NULL,NULL);


        tempo_ms(TEMPO_BETWEEN_TEST_MS,qApp);
        _timeout_recep++;
        test_id_frame = _uds_temp.msg.can_info.can_id;

    }
    _uds_temp.msg.can_info.can_id = 0;

    qDebug() << "Sortie while";

    if(_timeout_recep < TIMEOUT_MAX_RECEPTION_FRAME)    // Tout s'est bien passé, on peut continuer
    {

        if(_uds_temp.msg.msgdata.isotp->length > 7)  // Multiframe
        {
            //qDebug() << "Trame sur du multiframe !!!";
            printf("Trame sur du multiframe !!! \n ");
            fflush(stdout);
            _timeout_recep = 0;
            while((_uds_temp.msg.can_info.can_id != CAN_ID_UDS_ANSWER && (_timeout_recep < TIMEOUT_MAX_RECEPTION_FRAME)))
            {
                status = UDS_MsgFree_2013(&_uds_temp);
                memset(&_uds_temp, 0, sizeof(uds_msg));
                _status_result = UDS_Read_2013((cantp_handle)channel,&_uds_temp,NULL,NULL);
                //qDebug() << "Status result 1 = " << _status_result << "  ---  UDS_Read_2013 reponse ID  1 =  " << response.msg.can_info.can_id;
                tempo_ms(TEMPO_BETWEEN_TEST_MS,qApp);
                _timeout_recep++;
            }
        }
        else
        {
            printf("Single frame !!! \n ");
            fflush(stdout);
            printf("_uds_temp.msg.msgdata.isotp->length = %d \n ", _uds_temp.msg.msgdata.isotp->length);
            fflush(stdout);
        }
        if(_timeout_recep < TIMEOUT_MAX_RECEPTION_FRAME)
        {
            response->msg.msgdata.isotp->length =_uds_temp.msg.msgdata.isotp->length;
            response->msg.msgdata.isotp->data = _uds_temp.msg.msgdata.isotp->data;
            _ret = true;
        }
        else
        {
            qDebug() << "Erreur alone function timeout ";
        }

    }
    else
    {
        qDebug() << "Erreur alone function timeout ";
    }
    status = UDS_MsgFree_2013(&_uds_temp);

    return _ret;
}

Thank you for your help !!!

[F.Vergnaud]

Hello,

From your code it feels like you're sending a UDS request and expects multiple UDS responses ?! (in sentAloneUDSRequest, there is while loop around UDS_Read_2013(...)).
This is not valid UDS communication: a physical UDS request is answered by only one UDS response. A UDS message handles segmentation which is done by the ISO-TP layer.
This means that you don't have to handle each segmented frames.

Even with a custom service, you should use UDS_WaitForService_2013 to read the response of a request. You should check the samples (06_client_all_request) to see how to use WaitForService function.

Regarding your crash, you access "_uds_temp.msg.msgdata.isotp" without checking if uds_temp.msg is actually a message with the type PCANTP_MSGTYPE_ISOTP:
* msgdata is a union, it can store either a CAN frame, CAN FD frame or ISOTP message.
* there will be an access exception if you access isotp information in a can/canfd object...

[Benoit_V]

Hello,

I have adapted my code to use the UDS_WaitForService_2013.
I have saw the example inside the client all request.

Unfortunately, my code doesn't work.
With the uds_status, I have an error (PCANTP_ERRSTATUS_MAPPING_INVALID = 0x0EU) and I don't understood why.

Do you have an idea?

I use the function UDS_Write_2013 to send the uds request. It's the good process?

Thank for your help!

Code: Select all

bool uds::sentAloneUDSRequest(cantp_handle channel, uds_msg *_msg_write, uds_msg *response)
{
    bool _ret = false;
    TPCANStatus     _status_result = (TPCANStatus)PUDS_STATUS_OK;
    uds_status status;
    uint32_t _timeout_recep = 0;
    uds_msg _uds_temp;
    uds_msg request;
    uds_msg confirmation;
    uds_msg local_response;
    uds_msgconfig config = s_config_physical;

    memset(&_uds_temp, 0, sizeof(uds_msg));
    memset(&request, 0, sizeof(uds_msg));
    memset(&confirmation, 0, sizeof(uds_msg));
    memset(&local_response, 0, sizeof(uds_msg));

    _status_result = (TPCANStatus)UDS_Write_2013(channel,_msg_write);
    status = (uds_status)_status_result;
    qDebug() << "UDS_Write_2013 1  =  " << status << "\n";


    uds_mapping request_mapping = {};
//    request_mapping.can_id = CAN_ID_UDS_REQUEST;
    request_mapping.can_id_flow_ctrl = CAN_ID_UDS_REQUEST+1;
    request_mapping.can_msgtype = PCANTP_CAN_MSGTYPE_EXTENDED;
    request_mapping.nai.extension_addr = 0;
    request_mapping.nai.protocol = PUDS_MSGPROTOCOL_ISO_15765_2_29B_FIXED_NORMAL;
    request_mapping.can_tx_dlc = 8;
    request_mapping.nai.source_addr = PUDS_ISO_15765_4_ADDR_TEST_EQUIPMENT;
    request_mapping.nai.target_addr = PUDS_ISO_15765_4_ADDR_ECU_1;
    request_mapping.nai.target_type = PCANTP_ISOTP_ADDRESSING_PHYSICAL;
    status = UDS_AddMapping_2013(channel, &request_mapping);
    
	qDebug() << "Status Mapping = " << status;

    _status_result = UDS_WaitForService_2013(channel, _msg_write, &local_response, &confirmation);


    qDebug() << "Status wait for service = " << _status_result;

    status = UDS_MsgFree_2013(&_uds_temp);
    status = UDS_MsgFree_2013(&request);
    status = UDS_MsgFree_2013(&confirmation);
    status = UDS_MsgFree_2013(&local_response);

    qDebug() << "FIN alone function ";
    return _ret;
}

[F.Vergnaud]

Hello,

First of all in your code, you first write a message (UDS_Write) then call UDS_AddMapping. This is wrong: Initialization and configuration must be done only once and before any communication.

You want to communicate using 29 bit Fixed normal addressing, this network addressing information does not require a mapping (CAN ID contains the source/target information) as stated in the PCAN-UDS documentation in §4.3
"UDS and ISO-TP Network Addressing Information".
You can check example "12_client_request_ReadDataByPeriodicIdentifier" for a code sample.

To help you further, here is a draft on how your code should look like (please check the inline comments):

Code: Select all

main_function(...)
{
[...]
    uds_msgconfig config = s_config_physical;
    memset(&_msg_write, 0, sizeof(uds_msg));
    memset(&response, 0, sizeof(uds_msg));
    
    // configure a UDS request 29 bit Fixed Normal, CHANGE source and target address 
    memset(&config, 0, sizeof(uds_msgconfig));    
    config.can_id = -1;
    config.can_msgtype = PCANTP_CAN_MSGTYPE_EXTENDED;
    config.nai.protocol = PUDS_MSGPROTOCOL_ISO_15765_2_29B_NORMAL;
    config.nai.target_type = PCANTP_ISOTP_ADDRESSING_PHYSICAL;
    config.type = PUDS_MSGTYPE_USDT;
    config.nai.source_addr = 0x10; //TODO change
    config.nai.target_addr = 0x20; //TODO change
    
    uds_status status = UDS_MsgAlloc_2013(_msg_write, config, size); // CANTP Alloc + Init
    if (UDS_StatusIsOk(status)) {
        // configure data
        _msg_write.msg.msgdata.isotp->length = (uint32_t)8;
        _msg_write.msg.msgdata.isotp->data[0] = (uint8_t)2;
        _msg_write.msg.msgdata.isotp->data[1] = (uint8_t)0xBA;
        _msg_write.msg.msgdata.isotp->data[2] = (uint8_t)0x02;
         
         // write and read response
        _function_return = sentAloneUDSRequest(channel,&_msg_write,&response);
    }
[...]
}

Code: Select all

bool uds::sentAloneUDSRequest(cantp_handle channel, uds_msg *_msg_write, uds_msg *response)
{
    bool _ret = false;
    uds_status status;
    uint32_t _timeout_recep = 0;

    status = (TPCANStatus)UDS_Write_2013(channel,_msg_write);
    qDebug() << "UDS_Write_2013 1  =  " << status << "\n";

    // assert status is OK before trying to read response
    if (UDS_StatusIsOk(status)) {
        uds_msg confirmation;
        memset(&confirmation, 0, sizeof(uds_msg));
        status = UDS_WaitForService_2013(channel, _msg_write, response, &confirmation);
        
        // assert status is OK, you should also check confirmation (especially result status)
        _ret = UDS_StatusIsOk(status);       
        qDebug() << "Status wait for service = " << status;

        status = UDS_MsgFree_2013(&confirmation);
        qDebug() << "FIN alone function ";
    }
    
    return _ret;
}

[Benoit_V]

Hello,

I had use your code. Unfortunately, the problem is still there.
I had discuss with my colleagues and after some tests. We thinking a trouble inside the function UDS_MsgFree_2013() but we are not sure.

To solve our problem, do you have an example for implementation or a precise description for one of your internal function, like par exemple, UDS_SvcECUReset_2013, please?

Best regard.

[F.Vergnaud]

Hello,

You can check sample 01 where the sample writes the response to ECUReset request.
This is similar to what is is done internally, here is an overview:

Code: Select all

// Used by the client to request a server reset
uds_status SvcECUReset_2013(cantp_handle channel, uds_msgconfig request_config, uds_msg *out_msg_request, uint8_t reset_type)
{
    const uint32_t size = 2;

    uds_status status = UDS_MsgAlloc_2013(out_msg_request, request_config, size);
    if (UDS_StatusIsOk_2013(status))
    {
        *out_msg_request->links.service_id = PUDS_SI_ECUReset;
        out_msg_request->links.param[0] = reset_type;
        status = UDS_Write_2013(channel, out_msg_request);
    }
    return status;
}

If you think you found an issue, please contact support at peak-system.com mentioning this thread and attach a minimal and working sample showing the problem.

[Benoit_V]

Hello,

I use your function "uds_status uds::SvcECUReset_2013" and it's work.

I have adaptated to my function.
"
*out_msg_request->links.service_id = PUDS_SI_ECUReset;
out_msg_request->links.param[0] = reset_type;
"
But unfortunately, it's doesn't work.

Do you have an example with a function with a multiframes, please?

Thanks

[F.Vergnaud]

Hello,

Simply change size to a value greater than 7 and copy some data in the buffer:

Code: Select all

uds_status SvcECUReset_2013(cantp_handle channel, uds_msgconfig request_config, uds_msg *out_msg_request, uint8_t reset_type)
{
    // add some data to the UDS request
    uint8_t data[20];
    memset(data, 0xAB, sizeof(data));
        
    const uint32_t size = 2 + sizeof(data);

    uds_status status = UDS_MsgAlloc_2013(out_msg_request, request_config, size);
    if (UDS_StatusIsOk_2013(status))
    {
        *out_msg_request->links.service_id = PUDS_SI_ECUReset;
        out_msg_request->links.param[0] = reset_type;
        
        // copy data
        memcpy(&out_msg_request->links.param[1], data, sizeof(data));

        status = UDS_Write_2013(channel, out_msg_request);
    }
    return status;
}

[Benoit_V]

Hello,

After some tests, we have found the problem.

The problem was a recursive function to call the UDS Function.
No problem inside the library.

Thank you for help!!