Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usbx cdc acm, missing zlp #21

Open
chr-btz opened this issue Mar 1, 2022 · 3 comments
Open

usbx cdc acm, missing zlp #21

chr-btz opened this issue Mar 1, 2022 · 3 comments
Assignees

Comments

@chr-btz
Copy link

chr-btz commented Mar 1, 2022

Board: Nucleo F429zi
IDE: CubeIDE

When the endpoints max packet size equals the data to send size, usbx does not send a zlp, therefore the host does not recognize the transfer as complete.

Reproduce:

  1. Ux_Device_CDC_ACM example

  2. Change usbx_cdc_acm_write_thread_entry to something like this:

     /* Get the device */
     device = &_ux_system_slave->ux_system_slave_device;
    
     /* Get the data interface */
     data_interface = device->ux_slave_device_first_interface->ux_slave_interface_next_interface;
    
     /* Get the cdc Instance */
     cdc_acm = data_interface->ux_slave_interface_class_instance;
    
     cdc_acm -> ux_slave_class_cdc_acm_transmission_status = UX_FALSE;
    
     buffsize = 0;
     
     for(int i = 0; i < testlength; i++)
     {
     	UserTxBufferFS[i] = '1';
     	buffsize++;
     }
    
     /* Send data over the class cdc_acm_write */
     ux_status = ux_device_class_cdc_acm_write(cdc_acm,
     								(UCHAR *)UserTxBufferFS,
     								buffsize, &actual_length);
    
     /* Check if dataset is correctly transmitted */
     if (ux_status)
     {
     	tx_thread_sleep(100);
     }
     tx_thread_sleep(100);
    
  3. Set testlength to endpoints max packet size

  4. Host wont recognize data

Analysis:

After investigation i found several issues,

host_length != slave_length will be false even if zlp is needed

/* See if we need to force a zero length packet at the end of the transfer.
This happens on a DATA IN and when the host requested length is not met
and the last packet is on a boundary. If slave_length is zero, then it is
a explicit ZLP request, no need to force ZLP. */
if ((transfer_request -> ux_slave_transfer_request_phase == UX_TRANSFER_PHASE_DATA_OUT) &&
(slave_length != 0) && (host_length != slave_length) &&
(slave_length % endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize) == 0)
{
/* If so force Zero Length Packet. */
transfer_request -> ux_slave_transfer_request_force_zlp = UX_TRUE;
}
else
{
/* Condition is not met, do not force a Zero Length Packet. */
transfer_request -> ux_slave_transfer_request_force_zlp = UX_FALSE;
}

even if
transfer_request -> ux_slave_transfer_request_force_zlp gets set to UX_TRUE the zlp wont be sended because its exclusive to endpoint 0, see:

void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
{
UX_SLAVE_DCD *dcd;
UX_DCD_STM32 *dcd_stm32;
UX_DCD_STM32_ED *ed;
UX_SLAVE_TRANSFER *transfer_request;
ULONG transfer_length;
UX_SLAVE_ENDPOINT *endpoint;
/* Get the pointer to the DCD. */
dcd = &_ux_system_slave -> ux_system_slave_dcd;
/* Get the pointer to the STM32 DCD. */
dcd_stm32 = (UX_DCD_STM32 *) dcd -> ux_slave_dcd_controller_hardware;
/* Fetch the address of the physical endpoint. */
ed = &dcd_stm32 -> ux_dcd_stm32_ed[epnum & 0xF];
/* Get the pointer to the transfer request. */
transfer_request = &(ed -> ux_dcd_stm32_ed_endpoint -> ux_slave_endpoint_transfer_request);
/* Endpoint 0 is different. */
if (epnum == 0U)
{
/* Get the pointer to the logical endpoint from the transfer request. */
endpoint = transfer_request -> ux_slave_transfer_request_endpoint;
/* Check if we need to send data again on control endpoint. */
if (ed -> ux_dcd_stm32_ed_state == UX_DCD_STM32_ED_STATE_DATA_TX)
{
/* Arm Status transfer. */
HAL_PCD_EP_Receive(hpcd, 0, 0, 0);
/* Are we done with this transfer ? */
if (transfer_request -> ux_slave_transfer_request_in_transfer_length <= endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
{
/* There is no data to send but we may need to send a Zero Length Packet. */
if (transfer_request -> ux_slave_transfer_request_force_zlp == UX_TRUE)
{
/* Arm a ZLP packet on IN. */
HAL_PCD_EP_Transmit(hpcd,
endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 0, 0);
/* Reset the ZLP condition. */
transfer_request -> ux_slave_transfer_request_force_zlp = UX_FALSE;
}
else
{
/* Set the completion code to no error. */
transfer_request -> ux_slave_transfer_request_completion_code = UX_SUCCESS;
/* The transfer is completed. */
transfer_request -> ux_slave_transfer_request_status = UX_TRANSFER_STATUS_COMPLETED;
/* We are using a Control endpoint, if there is a callback, invoke it. We are still under ISR. */
if (transfer_request -> ux_slave_transfer_request_completion_function)
transfer_request -> ux_slave_transfer_request_completion_function (transfer_request) ;
/* State is now STATUS RX. */
ed -> ux_dcd_stm32_ed_state = UX_DCD_STM32_ED_STATE_STATUS_RX;
}
}
else
{
/* Get the size of the transfer. */
transfer_length = transfer_request -> ux_slave_transfer_request_in_transfer_length - endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;
/* Check if the endpoint size is bigger that data requested. */
if (transfer_length > endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
{
/* Adjust the transfer size. */
transfer_length = endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;
}
/* Adjust the data pointer. */
transfer_request -> ux_slave_transfer_request_current_data_pointer += endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;
/* Adjust the transfer length remaining. */
transfer_request -> ux_slave_transfer_request_in_transfer_length -= transfer_length;
/* Transmit data. */
HAL_PCD_EP_Transmit(hpcd,
endpoint->ux_slave_endpoint_descriptor.bEndpointAddress,
transfer_request->ux_slave_transfer_request_current_data_pointer,
transfer_length);
}
}
}
else
{
/* Set the completion code to no error. */
transfer_request -> ux_slave_transfer_request_completion_code = UX_SUCCESS;
/* The transfer is completed. */
transfer_request -> ux_slave_transfer_request_status = UX_TRANSFER_STATUS_COMPLETED;
/* Non control endpoint operation, use semaphore. */
_ux_utility_semaphore_put(&transfer_request -> ux_slave_transfer_request_semaphore);
}
}

when removed the host_length != slave_length in issue one and use this code for HAL_PCD_DataInStageCallback its working:

void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
{

UX_SLAVE_DCD            *dcd;
UX_DCD_STM32            *dcd_stm32;
UX_DCD_STM32_ED         *ed;
UX_SLAVE_TRANSFER       *transfer_request;
ULONG                   transfer_length;
UX_SLAVE_ENDPOINT       *endpoint;


    /* Get the pointer to the DCD.  */
    dcd =  &_ux_system_slave -> ux_system_slave_dcd;

    /* Get the pointer to the STM32 DCD.  */
    dcd_stm32 = (UX_DCD_STM32 *) dcd -> ux_slave_dcd_controller_hardware;

    /* Fetch the address of the physical endpoint.  */
    ed =  &dcd_stm32 -> ux_dcd_stm32_ed[epnum & 0xF];

    /* Get the pointer to the transfer request.  */
    transfer_request =  &(ed -> ux_dcd_stm32_ed_endpoint -> ux_slave_endpoint_transfer_request);

    /* Endpoint 0 is different.  */
    if (epnum == 0U)
    {

        /* Get the pointer to the logical endpoint from the transfer request.  */
        endpoint =  transfer_request -> ux_slave_transfer_request_endpoint;

        /* Check if we need to send data again on control endpoint. */
        if (ed -> ux_dcd_stm32_ed_state == UX_DCD_STM32_ED_STATE_DATA_TX)
        {

            /* Arm Status transfer.  */
            HAL_PCD_EP_Receive(hpcd, 0, 0, 0);

            /* Are we done with this transfer ? */
            if (transfer_request -> ux_slave_transfer_request_in_transfer_length <= endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
            {

                /* There is no data to send but we may need to send a Zero Length Packet.  */
                if (transfer_request -> ux_slave_transfer_request_force_zlp ==  UX_TRUE)
                {

                    /* Arm a ZLP packet on IN.  */
                    HAL_PCD_EP_Transmit(hpcd, 
                            endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 0, 0);

                    /* Reset the ZLP condition.  */
                    transfer_request -> ux_slave_transfer_request_force_zlp =  UX_FALSE;

                }
                else
                {

                    /* Set the completion code to no error.  */
                    transfer_request -> ux_slave_transfer_request_completion_code =  UX_SUCCESS;

                    /* The transfer is completed.  */
                    transfer_request -> ux_slave_transfer_request_status =  UX_TRANSFER_STATUS_COMPLETED;

                    /* We are using a Control endpoint, if there is a callback, invoke it. We are still under ISR.  */
                    if (transfer_request -> ux_slave_transfer_request_completion_function)  
                        transfer_request -> ux_slave_transfer_request_completion_function (transfer_request) ;

                    /* State is now STATUS RX.  */
                    ed -> ux_dcd_stm32_ed_state = UX_DCD_STM32_ED_STATE_STATUS_RX;
                }
            }
            else
            {

                /* Get the size of the transfer.  */
                transfer_length = transfer_request -> ux_slave_transfer_request_in_transfer_length - endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;

                /* Check if the endpoint size is bigger that data requested. */
                if (transfer_length > endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize)
                {

                    /* Adjust the transfer size.  */
                    transfer_length =  endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;
                }

                /* Adjust the data pointer.  */
                transfer_request -> ux_slave_transfer_request_current_data_pointer += endpoint -> ux_slave_endpoint_descriptor.wMaxPacketSize;

                /* Adjust the transfer length remaining.  */
                transfer_request -> ux_slave_transfer_request_in_transfer_length -= transfer_length;

                /* Transmit data.  */
                HAL_PCD_EP_Transmit(hpcd, 
                            endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 
                            transfer_request->ux_slave_transfer_request_current_data_pointer,
                            transfer_length);
            }
        }
    }
    else
    {
	    if (transfer_request -> ux_slave_transfer_request_force_zlp ==  UX_TRUE)
	    {
            /* Get the pointer to the logical endpoint from the transfer request.  */
            endpoint =  transfer_request -> ux_slave_transfer_request_endpoint;

		    /* Arm a ZLP packet on IN.  */
		    HAL_PCD_EP_Transmit(hpcd,
				    endpoint->ux_slave_endpoint_descriptor.bEndpointAddress, 0, 0);

		    /* Reset the ZLP condition.  */
		    transfer_request -> ux_slave_transfer_request_force_zlp =  UX_FALSE;

	    }else{
		    /* Set the completion code to no error.  */
		    transfer_request -> ux_slave_transfer_request_completion_code =  UX_SUCCESS;

		    /* The transfer is completed.  */
		    transfer_request -> ux_slave_transfer_request_status =  UX_TRANSFER_STATUS_COMPLETED;

		    /* Non control endpoint operation, use semaphore.  */
		    _ux_utility_semaphore_put(&transfer_request -> ux_slave_transfer_request_semaphore);
	    }
    }
}
@fhorinek
Copy link

I am having the same issue using PIMA class. The ZLP is not transmitted.
Using your modified callback function that send ZLP for other endpoint than 0 works for me.
Thank you!

@AYEDMSTM
Copy link

AYEDMSTM commented May 5, 2022

Hi @chr-btz,

Thanks for your report, The issue you pointed out is confirmed. This will be fixed in the next release.

BR.

@AYEDMSTM
Copy link

AYEDMSTM commented Dec 7, 2022

hi @chr-btz ,
This issue fixed in V3.0.0 release, you need to define UX_DEVICE_CLASS_CDC_ACM_WRITE_AUTO_ZLP

Best Regards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants