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

definition of thread flags are uncoordindated between modules #20867

Open
10 tasks
Enoch247 opened this issue Sep 20, 2024 · 6 comments
Open
10 tasks

definition of thread flags are uncoordindated between modules #20867

Enoch247 opened this issue Sep 20, 2024 · 6 comments
Labels
Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented

Comments

@Enoch247
Copy link
Contributor

Enoch247 commented Sep 20, 2024

Description

There is a macro THREAD_FLAG_PREDEFINED_MASK to capture thread flags in-use by RIOT to allow applications to avoid collisions when using flags. However, many flags are in-use inside of modules and are not captured in this macro. Worse, some flags even within RIOT are defined to the same value. All flag definitions should be moved to thread_flags.h and added to the THREAD_FLAG_PREDEFINED_MASK macro.

Flags to add to THREAD_FLAG_PREDEFINED_MASK (found with git grep THREAD_FLAG | grep "#define"):

  • cpu/kinetis/periph/i2c.c:#define THREAD_FLAG_KINETIS_I2C (1u << 8)
  • drivers/kw41zrf/kw41zrf_netdev.c:#define KW41ZRF_THREAD_FLAG_ISR (1u << 8)
  • pkg/lvgl/contrib/lvgl.c:#define LVGL_THREAD_FLAG (1 << 7)
  • pkg/lwip/contrib/netdev/lwip_netdev.c:#define THREAD_FLAG_LWIP_TX_DONE (1U << 11)
  • sys/include/event.h:#define THREAD_FLAG_EVENT (0x1) - Move event thread flag #20868
  • sys/include/usb/usbus.h:#define USBUS_THREAD_FLAG_USBDEV (0x02) /**< usbdev esr needs handling */
  • sys/include/usb/usbus.h:#define USBUS_THREAD_FLAG_USBDEV_EP (0x04) /**< One or more endpoints requires
  • sys/posix/include/sys/select.h:#define POSIX_SELECT_THREAD_FLAG (1U << 3)
  • tests/drivers/cst816s/main.c:#define CST816S_THREAD_FLAG (1 << 8)
  • tests/pkg/libschc/main.c:#define THREAD_FLAG_TX_END (1U << 4)

Versions

  • 2024.10-devel-190-g20f7328df4
@Enoch247 Enoch247 added the Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented label Sep 20, 2024
@mguetschow
Copy link
Contributor

Hum, any idea how we could convey the information that a flag is used only if the respective module is actually selected? Otherwise at some point we will end up having most if not all bits already in the predefined mask and the mask won't be useful anymore.

@Enoch247
Copy link
Contributor Author

Enoch247 commented Oct 8, 2024

Hum, any idea how we could convey the information that a flag is used only if the respective module is actually selected? Otherwise at some point we will end up having most if not all bits already in the predefined mask and the mask won't be useful anymore.

We could try something like (untested, but seems like it would work):

enum _thread_flag_pos {

#ifdef MODULE_FOO
   _THREAD_FLAG_FOO,
  #define THREAD_FLAG_FOO (1 < _THREAD_FLAG_FOO)
#endif

#ifdef MODULE_BAR
   _THREAD_FLAG_BAR,
  #define THREAD_BAR_FOO (1 < _THREAD_FLAG_BAR)
#endif

    THREAD_FLAG_NUMOF
}
static_assert(THREAD_FLAG_NUMOF < 8);

@Enoch247
Copy link
Contributor Author

Enoch247 commented Oct 8, 2024

Adding a link with some valuable comments about this issue from the RIOT chat:
https://matrix.to/#/!pqHdpanAvkJvlCwUDE:matrix.org/$HbXRfijYIzubvzvQExivS1bgxEWdGD6lc7nZ67QQLAQ?via=matrix.org&via=tu-dresden.de&via=utwente.io

To summarize the comments from the link; flags don't have to be unique in all cases, but if they are re-used, it should be done so intentionally.

@mguetschow
Copy link
Contributor

We could try something like (untested, but seems like it would work) ...

Neat trick. And to get the predefined flags something like this?

#ifdef MODULE_FOO
   _THREAD_FLAG_FOO,
  #define THREAD_FLAG_FOO (1 < _THREAD_FLAG_FOO)
#else
  #define THREAD_FLAG_FOO (0)
#endif

#define THREAD_FLAG_PREDEFINED_MASK THREAD_FLAG_FOO | THREAD_FLAG_BAR

@Enoch247
Copy link
Contributor Author

Neat trick. And to get the predefined flags something like this?

#ifdef MODULE_FOO
   _THREAD_FLAG_FOO,
  #define THREAD_FLAG_FOO (1 < _THREAD_FLAG_FOO)
#else
  #define THREAD_FLAG_FOO (0)
#endif

#define THREAD_FLAG_PREDEFINED_MASK THREAD_FLAG_FOO | THREAD_FLAG_BAR

Yes, I believe that will work.

@mguetschow
Copy link
Contributor

Then in my opinion that would be the way to go for #20868 and similar ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented
Projects
None yet
Development

No branches or pull requests

2 participants