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

Add host OS detection quirk. #2628

Closed
wants to merge 3 commits into from
Closed

Conversation

HiFiPhile
Copy link
Collaborator

@HiFiPhile HiFiPhile commented May 7, 2024

Describe the PR
Due to the nature of quirk I'm not sure where to put...

This 1st OS detection quirk mainly resolve UAC class issue. Windows and OSX demand different feedback endpoint size for FS as Windows doesn't implement UAC spec correctly (Linux is compatible with both).
So there needs a way to adjust Configuration Descriptor on the fly.

Tested on:

  • Windows 10 - 11
  • Linux 3.10 - 6.8
  • OS X Ventura - Sonoma

@HiFiPhile
Copy link
Collaborator Author

@Marcus2060 @kf6gpe @rhysmorgan134

A quirk is come out which (hopefully) resolve UAC feedback format issue, test welcomed.

Now it's possible to use different Config descriptors:

uint8_t const desc_configuration_default[] =
{
    // Config number, interface count, string index, total length, attribute, power in mA
    TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL, 0, CONFIG_TOTAL_LEN, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, 500),
    // Interface number, Alternate count, starting string index, attributes, detach timeout, transfer size
    TUD_DFU_DESCRIPTOR(ITF_NUM_DFU, 1, 4, DFU_ATTR_CAN_UPLOAD | DFU_ATTR_CAN_DOWNLOAD | DFU_ATTR_MANIFESTATION_TOLERANT, 100, CFG_TUD_DFU_XFER_BUFSIZE),
};

uint8_t const desc_configuration_osx[] =
{
    // Config number, interface count, string index, total length, attribute, power in mA
    TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL_OSX, 0, CONFIG_TOTAL_LEN_OSX, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, 500),
     // 1st CDC: Interface number, string index, EP notification address and size, EP data address (out, in) and size.
    TUD_CDC_DESCRIPTOR(ITF_NUM_CDC_0, 4, EPNUM_CDC_0_NOTIF, 8, EPNUM_CDC_0_OUT, EPNUM_CDC_0_IN, 64),
};

// Invoked when received GET CONFIGURATION DESCRIPTOR
// Application return pointer to descriptor
// Descriptor contents must exist long enough for transfer to complete
uint8_t const * tud_descriptor_configuration_cb(uint8_t index)
{
    (void) index; // for multiple configurations

    return  (tud_quirk_host_os_hint() == TUD_QUIRK_OS_HINT_OSX) ? desc_configuration_osx: desc_configuration_default;
}

@rhysmorgan134
Copy link

I'll have a go at building into my current project, may take a while as I few bits I think I need to get my head around to get it in there working. Thanks for looking into it!

Copy link
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, this is not really stable. These things can change, and it is not a good idea to give any suggestion if we aren't sure. I guess we can leave this as is for now until there is better approach.

Note: if the order of descriptors can be used to determinne the OS, this can be done by application since these callback are executed in application space. User can keep track of the order and do the guess work there.

@HiFiPhile
Copy link
Collaborator Author

I agree it's not stable and could break in the future.

But facing the UAC OS compatibility issue I can't come up a better way, at least it will calm down the issue for a moment.

I can move the quirk to application space of uac2_speaker_fb example in #2328 , but since callbacks are separated functions the quirk will become clustered and harder to understand.

@hathach
Copy link
Owner

hathach commented Jul 22, 2024

@HiFiPhile that is OK, I still prefer not puttin this in the stack.. the commennt will help to illustrate the point of guess work. just use bool flag for each descriptor callback.

@HiFiPhile
Copy link
Collaborator Author

Already implemented in #2328

@HiFiPhile HiFiPhile closed this Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants