tinyusb icon indicating copy to clipboard operation
tinyusb copied to clipboard

Extract common macros to separate header file

Open tomas-pecserke opened this issue 1 year ago • 3 comments

Describe the PR This PR moves helper macros from tusb_common.h to separate file so it can be included from tusb_types.h. This is needed for situations when user needs to include only types without the rest of the stack. It is useful mainly for writing libraries using TinyUSB as a dependency. Curently there is no way to import only types from a library without having a stack set up.

Additional context I am writing a small library helping with implementation of custom vendor interfaces for RP2040, and I find myself in need of importing types, specifically for tusb_control_request_t.

My library provides implementation for custom reset interface via callback function:

bool reset_interface_cb(uint8_t stage, tusb_control_request_t const * request);

It is meant to be called like this:

bool tud_vendor_control_xfer_cb(__unused uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) {
    switch (request->wIndex) {
        case ITF_NUM_VENDOR_RESET:
            return reset_interface_cb(stage, request);
        // If you have more vendor interfaces, forward their calls here
    }
    return false;
}

But when I include tusb_types.h, I get compiler errors:

In file included from ~/projects/personal/pico_tusb_reset_interface/pico_tusb_reset_interface.h:46,
                 from ~/projects/personal/pico_tusb_reset_interface/pico_tusb_reset_interface.c:26:
~pico-sdk/lib/tinyusb/src/common/tusb_types.h:229:40: warning: implicit declaration of function 'TU_BIT' [-Wimplicit-function-declaration]
  229 |   TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP = TU_BIT(5),
      |                                        ^~~~~~
~/pico-sdk/lib/tinyusb/src/common/tusb_types.h:229:3: error: enumerator value for 'TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP' is not an integer constant
  229 |   TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP = TU_BIT(5),
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~/pico-sdk/lib/tinyusb/src/common/tusb_types.h:230:3: error: enumerator value for 'TUSB_DESC_CONFIG_ATT_SELF_POWERED' is not an integer constant
  230 |   TUSB_DESC_CONFIG_ATT_SELF_POWERED  = TU_BIT(6),
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ~/projects/personal/pico_tusb_reset_interface/pico_tusb_reset_interface.h:46,
                 from ~/projects/personal/pico_tusb_reset_interface/pico_tusb_reset_interface.c:26:
~/pico-sdk/lib/tinyusb/src/common/tusb_types.h: In function 'tu_edpt_packet_size':
~/pico-sdk/lib/tinyusb/src/common/tusb_types.h:526:48: warning: implicit declaration of function 'TU_GENMASK' [-Wimplicit-function-declaration]
  526 |   return tu_le16toh(desc_ep->wMaxPacketSize) & TU_GENMASK(10, 0);
      |                                                ^~~~~~~~~~

This is caused by helper macros from tusb_common.h are not included and including this header tries to pull in tusb_config.h, which is not present in the library, since it's up to the users to define their stack.

I have a workaround of redefining these 2 macros specifically for my library:

// Workaround for missing macros
// It needs to be defined before including tusb_types.h
// TODO remove once https://github.com/hathach/tinyusb/pull/2097 is merged
#ifndef TU_BIT
#define TU_BIT(n)             (1UL << (n))
#endif
#ifndef TU_GENMASK
#define TU_GENMASK(h, l)      ( (UINT32_MAX << (l)) & (UINT32_MAX >> (31 - (h))) )
#endif

#include <tusb_types.h>

However as code duplication I would like to eliminate it, and it would be nice to solve universally, so others can write their libraries and contribute to the ecosystem.

tomas-pecserke avatar Jun 05 '23 19:06 tomas-pecserke

can you elaborate your code and the issue you are encountering.

hathach avatar Jun 06 '23 02:06 hathach

filled in more context

tomas-pecserke avatar Jun 07 '23 09:06 tomas-pecserke

thanks for the context, I will try to revise the include hierachy and/or review this later on.

hathach avatar Jun 09 '23 12:06 hathach

Superseded by #2586 which is lighter than this PR. Thank you for bringing this up, and sorry for the late response.

hathach avatar Apr 10 '24 08:04 hathach