eclipse-threadx / threadx

Eclipse ThreadX is an advanced real-time operating system (RTOS) designed specifically for deeply embedded applications.
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/threadx/index.md
MIT License
2.87k stars 782 forks source link

Inconsistent Timer API #224

Open OortJacek opened 1 year ago

OortJacek commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

goldscott commented 1 year ago

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