threadx icon indicating copy to clipboard operation
threadx copied to clipboard

tx_event_flags_get doesn't always return the actual flags

Open nekomata-san opened this issue 3 years ago • 4 comments

When calling the tx_event_flags_get API, the actual flags are only set upon return if one of the requested event flags is set. So, if I want to keep a persistent flag Y and clear a different flag X, there is no way for me to do this in one API call if flag_X isn't currently set in the event "current flags." For example:

UINT result = tx_event_flags_get( &event_flags, myflag_X, TX_OR_CLEAR, &actual_flags, TX_NO_WAIT ); if ( result == TX_SUCCESS ) { /* Do something when myflag_X is set. It is also now cleared. / } if ( actual_flags & myflag_Y ) { / Do something when myflag_Y is set but it persists when set. */ }

As far as I can tell, I would need to do this by calling the API twice with different options and incurring twice the overhead when it would work fine if "actual flags" were always what it claims to be.

nekomata-san avatar Sep 20 '22 02:09 nekomata-san

Hi @nekomata-san, I am sorry that the documentation is unclear. We will improve it. The actual_flags parameter may not show set flags that aren't requested. You could use tx_event_flags_info_get(&event_flags, TX_NULL, &actual_flags, TX_NULL, TX_NULL, TX_NULL); to get the actual flags that are set.

goldscott avatar Sep 30 '22 18:09 goldscott

Hi Scott! Thanks for your reply.

This is a simple one liner fix within ThreadX to do the right thing by just setting the flags in either case. Then we don't need to incur unnecessary interrupt disabled overhead and it will match the documentation. Wouldn't it be better to just fix the bug rather than change the documentation to match buggy behavior?

Making an entirely separate API call just to get a 32 bit value you already have in the first API call seems like the wrong approach. Especially since you store the flags if one of them is set.

nekomata-san avatar Sep 30 '22 18:09 nekomata-san

Hi @nekomata-san - I just reviewed the history of ThreadX and it has always behaved this way. I agree with you that we should set the actual_flags in all cases, so I will modify the code and it should be available in our next release, scheduled for the end of October.

goldscott avatar Sep 30 '22 22:09 goldscott

Thanks Scott! I appreciate your help.

nekomata-san avatar Sep 30 '22 23:09 nekomata-san

Hi @nekomata-san - as of this 6.2.0 release, this issue has been fixed. See updated files here: https://github.com/azure-rtos/threadx/blob/master/common/src/tx_event_flags_get.c https://github.com/azure-rtos/threadx/blob/master/common_smp/src/tx_event_flags_get.c

goldscott avatar Oct 28 '22 15:10 goldscott