Bugfix parsing control transfer requests

[SeanHowsonAdvCo]

Hi Folks,

I've been porting a product over to USBX from a previous driver & noticed what i'm fairly confident is a small error in the function "_ux_device_stack_control_request_process"

An if statement toward the top of the function evaluates the request type being sent by the host to determine if it is something other than a standard request (class or vendor).

But the statement evaluates "request_value", which usually contains information specific/contextual to each request.
The request type is stored in bits 6 & 7 of bmRequestType & as far as i can see this is stored in request_type...

Several other statements below evaluate what i believe to be the correct variable.

I've modified the file & added a note to the top too. This is actually the first time I've contributed to an open-source project! So apologies in advance if i've missed anything, i've tried to pattern match things like branch-names etc, it seemed most of the bug-fix type PR's i could see were being merged to the dev branch, but if there's anything that needs tweaking let me know.

Best regards
Sean

P.S. If it turns out i'm mistaken, no offence will be taken to a declined PR!

[SeanHowsonAdvCo]

...currently working on the failed check! Appreciate you folks could probably just raise a parallel PR and sort the issue, but i'm keen to contribute here... IOW, please bear with me!

[fdesbiens]

Hi @SeanHowsonAdvCo.

Thank you for this contribution!

Before we can accept it, you need to sign the Eclipse Contributor Agreement (ECA). The purpose of the ECA is to provide a written record that you have agreed to provide your code and documentation contributions under the licenses used by the Eclipse ThreadX project. It also makes it clear that you are promising that what you are contributing to Eclipse is code you wrote, and you have the necessary rights to contribute it to our projects. And finally, it documents a commitment from you that your open source contributions will be permanently on the public record.

Signing the ECA requires an Eclipse Foundation account if you do not already have one. You can create one for free at https://accounts.eclipse.org.

Be sure to use the same email address when you register for the account that you intend to use on Git commit records.

Here is the link to sign the ECA:
https://accounts.eclipse.org/user/login?destination=user/eca

[fdesbiens]

Once you sort this out, @SeanHowsonAdvCo, I will ask a team member to review the PR.

If it passes muster, I will merge to dev, and this should ship with our Q4 2025 release.

[SeanHowsonAdvCo]

Once you sort this out, @SeanHowsonAdvCo, I will ask a team member to review the PR.

If it passes muster, I will merge to dev, and this should ship with our Q4 2025 release.

Hey! Thanks for the quick response, yep, i'm sorting the agreement now (like i say, first time i've done this!). Thanks for the guidance above too.

Hopefully it measures up! But if i am simply mistaken... then that's good all the same!

[SeanHowsonAdvCo]

@fdesbiens sorted! (turns out if you press the same button enough times, it just works)

[fdesbiens]

@SeanHowsonAdvCo I just forced the check to run. All is good. Assigning a knowledgeable committer to review now.

[SeanHowsonAdvCo]

@SeanHowsonAdvCo I just forced the check to run. All is good. Assigning a knowledgeable committer to review now.

Ha, i did think just pressing the same button again and getting a different result seemed a bit odd! Thanks! :)

[SeanHowsonAdvCo]

Hi Folks, just checking back in here, wondering is there's any movement on this? Not sure how often you guys go through PRs, so i don't want to hassle anyone! Just keen to get a bit of feedback, that way i can also filter this into my own use of USBX.
@rahmanih @fdesbiens

[fdesbiens]

Hi @SeanHowsonAdvCo.

Thank you for your patience. If you check our public roadmap (see link below), you will see your PR is in the "in review" column. I also assigned knowledgeable team members to review your contribution.

ThreadX Roadmap

However, we received a flurry of USBX and FileX bug fixes in the last two weeks ahead of our December release. Let's say that @rahmanih has a lot to review right now...

Do not worry: you will get a notification once we have reviewed your PR. I wish I could do it myself, but I am a literal beginner when it comes to USBX.

[ABOUSTM]

using request_type is not correct in this context especially with >> 8 shift, since the request_type takes the first byte of transfer_request_setup, then request_type >> 8 = 0

Could you please provide more details on the issue you faced to identify the root cause
which host request is causing the issue? Class req, vendor req?

[SeanHowsonAdvCo]

using request_type is not correct in this context especially with >> 8 shift, since the request_type takes the first byte of transfer_request_setup, then request_type >> 8 = 0

Could you please provide more details on the issue you faced to identify the root cause which host request is causing the issue? Class req, vendor req?

Hey pal,
Thanks for getting back to me.

My application only enters the function i've queried for vendor specific requests, (the Renesas driver that is used for my DCD layer handles standard requests) so my application never actually reaches line 142. I do agree it wouldnt be right to shift request_type (think i meant to remove this, oops, sorry, i'll push another commit), but neither do i think request_value should be used. Having stepped line by line through the function it really does appear to be using the wrong variable.

request_type is the variable used to store bmRequestType. bits 5 & 6 of this are used to indicate standard/class/vendor (UX_REQUEST_TYPE = 0x60u).

Could i ask you to please evaluate the code below as a comparison against the area i've changed? As this existing code does look at request_type (not shifted) to perform the same check, using the same macros, to establish the type of request being sent from the host:

To try and illustrate what i'm referring to below is a snippet from a reliable site i've used in the past, why would we want to evaluate request_value to deduce request type? (when request_value isn't used to store standard/class/vendor request type).

Thanks ,
Sean