Inconsistent Timer API

[OortJacek]

Describe the bug
Inconsistent Timer API. _txe_timer_change checks for system state, while other timer functions do not.

https://github.com/azure-rtos/threadx/blob/4e62226eeaf870827facbeef40bd1767db5cf9f0/common/src/txe_timer_change.c#L110

To Reproduce
Call _txe_timer_change within tx_application_define(). It will return TX_CALLER_ERROR

Expected behavior
It is possible to call _txe_timer_change within tx_application_define() with success.

Impact
Inconsistent Timer API

Logs and console output
Not needed. Trivial to understand and reproduce.

[goldscott]

Hi @OortJacek, please see the documentation for tx_timer_change here: https://learn.microsoft.com/en-us/azure/rtos/threadx/chapter4#tx_timer_change
Note that tx_timer_change is not allowed to be called from initialization (i.e. tx_application_define). I'm not sure why it can't be called from initialization, as it does work with some minor modifications to txe_timer_change. You can build with TX_DISABLE_ERROR_CHECKING defined (described here: https://learn.microsoft.com/en-us/azure/rtos/threadx/chapter2#detailed-configuration-options). This will allow you to call tx_timer_change from initialization.

[OortJacek]

Hi @goldscott

I'm not sure why it can't be called from initialization, as it does work with some minor modifications to txe_timer_change

That is why I'm asking to change the txe_timer_change as this constraint seems like a bug (legacy leftover).

Currently, I apply a patch that comments out this if statement, and it works fine, because why wouldn't it?

[OortJacek]

@goldscott Any further thoughts? Do you think it is possible to fix this unnecessary check?

[goldscott]

Hi @OortJacek - we are looking into it. I'll reopen this ticket.

[fdesbiens]

Thanks for the report @OortJacek, and my apologies for how long this sat. You were right, and so was @goldscott's instinct in 2023: the check was a legacy leftover with no technical justification.

_tx_timer_change only writes the expiration fields of a timer that is not on an active list, and it does so with interrupts disabled, so calling it from tx_application_define is exactly as safe as calling it from a thread. Meanwhile tx_timer_create, tx_timer_activate and tx_timer_deactivate were all allowed from initialization, and tx_timer_change was even allowed from an ISR — so the API really was inconsistent.

I have removed the check in both the common and the common_smp copies, and added an initialization-time tx_timer_change call to the regression test control code so the behaviour is now covered in both test suites. I confirmed the new check fails on dev and passes with the fix, and both suites are green. The fix is in #730, and the documentation update is in eclipse-threadx/rtos-docs-asciidoc#74, since Chapter 4 listed TX_CALLER_ERROR and excluded Initialization from the Allowed From list.

You can drop your local patch once this lands.