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

CMSIS RTOSv2 API: Redundant thread states like osThreadWaiting, osThreadError #67

Closed
ilg-ul opened this issue Jul 8, 2016 · 11 comments
Closed
Labels

Comments

@ilg-ul
Copy link
Contributor

ilg-ul commented Jul 8, 2016

I did not yet see the full documentation pages for these definitions, so my understanding of these functions might not be accurate, but I wanted to share my experience with CMSIS++.

I noticed that in enum osThreadState_t, you have separate osThreadWaiting and osThreadSuspended.

From a scheduler point of view, apart from running, the thread can be either ready or suspended. This reflect a clear fact, if the thread is in the READY list or not.

Thread states like osThreadWaiting are artificial. If a thread is not ready, i.e. it is suspended, it is clearly waiting for some resource, or synchronisation event, or timeout, etc to occur.

Initially I also had multiple states in my schedulers, but after implementing the CMSIS++ portable scheduler, I was pleased to discover that ready and suspended are the only essential states.

As it is now structured, CMSIS++ separates the implementation of the portable synchronisation objects from the implementation of the portable scheduler.

This has some interesting applications. For example I can configure a functional system running on top of a third party scheduler (like FreeRTOS), and select some of the synchronisation objects to be implemented by the CMSIS++ portable objects and some to be forwarded to the third party RTOS (this refers to the actual CMSIS++ port on top of FreeRTOS, where each objects can be configured at compile time to use either FreeRTOS or the portable CMSIS++ objects)

For this to work, there should be an API to a portable scheduler. The main functions of this scheduler API are simple, suspend() and resume(). What is doing a suspended thread is not relevant for the scheduler, it is the responsibility of the object that suspended the thread to keep track of it, possibly link it in a waiting list, etc.

This also implies that from the scheduler point of view, there are no other states to further describe suspended threads.

Somehow similarly for osThreadError. The thread is either ready or suspended. If an recoverable error occurred, it should be the responsibility of each function to return the error code. If a catastrophic error occurred (no idea if this is realistic), the thread is probably terminated (again, I never encountered such cases, my threads either run or not).

Suggestions:

  • remove osThreadWaiting (osThreadSuspended should be enough)
  • remove osThreadError
@RobertRostohar
Copy link
Collaborator

States osThreadWaiting and osThreadSuspended have been joined.

osThreadError is not a state but rather a special return code for osThreadGetState to indicate an error.

@ilg-ul
Copy link
Contributor Author

ilg-ul commented Oct 12, 2016

osThreadError is not a state but rather a special return code for osThreadGetState to indicate an error.

aha. can you exemplify with a situation when osThreadGetState() returns osThreadError?

@RobertRostohar
Copy link
Collaborator

When the parameter thread_id is invalid.

@ilg-ul
Copy link
Contributor Author

ilg-ul commented Oct 12, 2016

ok.

I still think that the definition of the thread state in itself should be standalone, and should not include error codes specific to a function.

in addition, you might have a consistency problem. what error code do you return from other functions when the thread_id is invalid?

@RobertRostohar
Copy link
Collaborator

Functions which have thread_id as parameter and return osStatus_t will return osErrorParameter when thread_id is invalid.

@ilg-ul
Copy link
Contributor Author

ilg-ul commented Oct 12, 2016

then make this function return osErrorParameter when thread_id is invalid and a thread state normally.

@JonatanAntoni
Copy link
Member

The return values (i.e. using in-band error codes) are intentionally API design.

@JonatanAntoni JonatanAntoni added DONE and removed review labels Mar 2, 2017
@ilg-ul
Copy link
Contributor Author

ilg-ul commented Mar 2, 2017

intentionally API design

what is the advantage of such a design?

@gerhil
Copy link

gerhil commented Mar 16, 2017

Where can I find osThreadSuspend and osThreadResume in mbed-os 5?

@JonatanAntoni
Copy link
Member

@Wazalogic Your questions is slightly of topic here. Please refer to the mbed documentation.

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Mar 24, 2017

@ilg-ul let me give some additional rational from our side. The advantage we aim to gain is a slightly less complex interface. I appreciate that others claim in-band error signaling to be error prone. Unfortunately the current API is already widely used. Thus changing the error signaling strategy would break the API . We may rethink this on some time in the future having a major API update on the timeline.

Please close this issue if your concerns regarding redundant thread states is solved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants