Software requirements: Semaphore: collected all requirements so far #58

stanislaw · 2024-04-09T11:27:44Z

As discussed on the meeting last week, I would like to open this combined PR for review.

The contents:

Requirements from req: add semaphore requirements #30 by @nashif. Note that all requirements have been preserved.
Requirements from (shelved) gregshue/zep semaphore reqs #31 by @legrand-gregshue.
Requirements from @legrand-gregshue and @stanislaw brainstorming sessions.
Some requirements were duplicates. I have merged them where appropriate, and these requirements now contain more than one alternatives separated by a horizontal line ----. Let's pick the wording that works best.

Some notes:

Note that due to the recent merges I could not preserve the original commits of both @nashif and @legrand-gregshue. Instead, it is just one commit by @stanislaw. Hopefully it is not too big of an issue.
To prevent merge conflicts with the fragments PRs that Greg has been creating, I am adding this fragment to be the last one in the software requirements index.sdoc.

Signed-off-by: Stanislav Pankevich <s.pankevich@gmail.com>

gregshue

I have only reviewed the first Semaphore requirement and the structure. I'll get to the others once we resolve the issues with this one.

gregshue · 2024-04-09T13:45:11Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


INCOSE GtWR R4 - Defined Terms:

"Compile time" and "build time" are terms that need to be defined in the glossary. "Build time" is more encompassing and includes the following in addition to "compile": configuration evaluation, code generation, document extraction, linking, and sub-builds.

"Zephyr RTOS" is a term that needs to be defined in the glossary.

Yes, these terms need to be added to the glossary, but this does not block merging.

gregshue · 2024-04-09T13:50:10Z

docs/software_requirements/semaphore.sdoc

+COMPONENT: Counting Semaphore
+TITLE: Build-Time Allocation and Initialization
+STATEMENT: >>>
+The Zephyr RTOS shall provide a mechanism to define and initialize a semaphore at compile time.


INCOSE GtWR R4 - Defined Terms: This statement needs to use the value of the COMPONENT: attribute.

gregshue · 2024-04-09T13:52:48Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


INCOSE GtWR R5 - Definite Articles: Both statements use "a"/"an". The GtWR rule is to use "the" rather than "a".

The requirement asks for an interface - "the" would be misleading here, as there is no interface existing that can be addressed as "the interface". So it is ok as it is here with the "an interface".

Agreed, since both must happen at build time in this case. Run-time is a different matter.

gregshue · 2024-04-09T13:57:52Z

docs/software_requirements/semaphore.sdoc

+The Zephyr RTOS shall expose an interface for a Counting Semaphore to accomplish all of the following at build-time:
+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.


INCOSE GtWR R11 - Separate Clauses: "allocating" and "initialize" are separate actions. They should be in separate clauses.

INCOSE GtWR R19 - Combinators: "count limit and initial count". Please either justify the high coupling of these parameters or state them in different clauses.

I do not agree with Greg's comment, as this requirement focuses on the need to have an interfacing functionality and what this interface functionality must look like - even if it is not atomic at the first glance, it does not make sense to me to split it up - So for me this requirement is ok as is.

gregshue · 2024-04-09T16:13:29Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


INCOSE GtWR R33 - Tolerance: "initial count", "count limit", and the "count" implicit in "Counting Semaphore" are quantities. Per R33 " Define each quantity with a range of values appropriate to the entity to which the quantity applies and against which the entity will be verified or validated."

Yes, but for me that are additional requirements.
It does not block merging this requirement, it is a need for at least one more requirement here - and the additional requirements are following already below anyway.

This is a good point for discussion.

gregshue · 2024-04-09T16:16:44Z

docs/software_requirements/semaphore.sdoc

+TITLE: Interface requirements
+
+[SECTION]
+TITLE: Initialize semaphore


This is a good example of following INCOSE GtWR R41 - Related Needs and Requirements: "Group related needs and requirements together."

gregshue · 2024-04-09T16:17:25Z

docs/software_requirements/semaphore.sdoc

+IMPORT_FROM_FILE: software_requirements.sgra
+
+[SECTION]
+TITLE: Interface requirements


This is a good example of following INCOSE GtWR R42 - Structured Sets: "Conform to a defined structure or template for organizing sets of needs and requirements."

nicpappler

I only have minor comments, for me this can be merged from a Functional Safety perspective.

nicpappler · 2024-04-18T17:14:42Z

docs/software_requirements/semaphore.sdoc

+The Zephyr RTOS shall expose an interface for a Counting Semaphore to accomplish all of the following at build-time:
+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.


I do not agree with Greg's comment, as this requirement focuses on the need to have an interfacing functionality and what this interface functionality must look like - even if it is not atomic at the first glance, it does not make sense to me to split it up - So for me this requirement is ok as is.

nicpappler · 2024-04-18T17:15:52Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


Yes, these terms need to be added to the glossary, but this does not block merging.

nicpappler · 2024-04-18T17:16:59Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


The requirement asks for an interface - "the" would be misleading here, as there is no interface existing that can be addressed as "the interface". So it is ok as it is here with the "an interface".

nicpappler · 2024-04-18T17:19:12Z

docs/software_requirements/semaphore.sdoc

+
+- statically allocating the Counting Semaphore;
+- statically initialize the Counting Semaphore with the provided count limit and initial count.
+<<<


Yes, but for me that are additional requirements.
It does not block merging this requirement, it is a need for at least one more requirement here - and the additional requirements are following already below anyway.

nicpappler · 2024-04-18T17:21:27Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-003
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


The COMPONENT changes between Counting Semaphore and Semaphore - is this intentionally? Otherwise I'd like to stick to one of them.

tobiaskaestner

This already is a very much improved synthesis of the previous two proposals. And it's only because of that, that the things I noted became visible. Overall these are my main observations

by using the technical term thread (which has a very specific meaning in the context of Zephyr) everywhere the text excludes other running contexts where it shouldn't. E.g. Semaphores can be taken/given from ISRs as well, yet ISRs mustn't block. We could deal with this in different ways, i.e. introduce similar RQTs for the ISR context, introduce a more generic term such as running context, control flow unit etc.
Timeouts are tricky and I am not sure this covers completely their implications. E.g. just because a timeout runs out, doesn't mean the waiting thread starts to run to evaluate the error - which by the time the thread runs indeed might be obsolete
I didn't find anything about what happens when giving a semaphore that has already reached its max count. I believe, nothing happens, ie. the calling thread stills see SUCCESS, just the counter isn't incremented any further.

tobiaskaestner · 2024-04-22T14:58:34Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-005
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


nitpick: Should probably read Counting Semaphore

tobiaskaestner · 2024-04-22T14:59:08Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-007
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


nitpick: Counting Semaphore

tobiaskaestner · 2024-04-22T14:59:32Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-010
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


nitpick: Counting Semaphore

tobiaskaestner · 2024-04-22T15:02:32Z

docs/software_requirements/semaphore.sdoc

+COMPONENT: Semaphore
+TITLE: Semaphore Timeout
+STATEMENT: >>>
+When attempting to take (acquire) a semaphore, the Zephyr RTOS shall accept options that specify timeout periods, allowing threads to set a maximum wait time for semaphore acquisition.


suggestion: strictly speaking semaphores could also be taken from ISRs when they K_NO_WAIT, so it's not solely about threads. That's why I was proposing a more generic term such as task or control flow entity to cover such corner cases. For now it's probably fine to leave as is and we can revisit later.

tobiaskaestner · 2024-04-22T15:06:47Z

docs/software_requirements/semaphore.sdoc

+COMPONENT: Semaphore
+TITLE: Maximum limit of a semaphore
+STATEMENT: >>>
+When a semaphore is used for counting purposes and when the semaphore does not have an explicit


issue: What other purposes are Counting Semaphores use for than counting? Also I am pretty sure that the maximum limit cannot be avoided as per our current implementation.

tobiaskaestner · 2024-04-22T15:33:31Z

docs/software_requirements/semaphore.sdoc

+- resume the waiting thread;
+- indicate TIMEDOUT to the waiting thread;
+- reschedule the waiting thread.
+


thought: there is an interesting question here: what happens if by means of a timeout a waiting thread becomes runnable again but can't run for some more time in which the semaphore now is given ... shall it then return TIMEOUT or SUCCESS?

tobiaskaestner · 2024-04-22T15:34:19Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-009
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


nitpick: Counting Semaphore

tobiaskaestner · 2024-04-22T15:36:00Z

docs/software_requirements/semaphore.sdoc

+STATUS: Draft
+TYPE: Functional
+COMPONENT: Counting Semaphore
+TITLE: Semaphore Give while Thread Waiting


suggestion: while threads are waiting

tobiaskaestner · 2024-04-22T15:36:24Z

docs/software_requirements/semaphore.sdoc

+COMPONENT: Counting Semaphore
+TITLE: Semaphore Give while Thread Waiting
+STATEMENT: >>>
+While a thread is waiting for a Counting Semaphore,


suggestion: there can be one more more threads waiting for the semaphore

tobiaskaestner · 2024-04-22T15:37:59Z

docs/software_requirements/semaphore.sdoc

+UID: ZEP-SEMAPHORE-006
+STATUS: Draft
+TYPE: Functional
+COMPONENT: Semaphore


nitpick: Counting Semaphore

stanislaw · 2024-05-02T16:52:18Z

As discussed at the SWG meeting 02.05.2024, this PR is being closed. The only source of truth for the new semaphore requirements is: #30.

Software requirements: Semaphore: collected all requirements so far

8badbe5

Signed-off-by: Stanislav Pankevich <s.pankevich@gmail.com>

gregshue suggested changes Apr 9, 2024

View reviewed changes

nicpappler approved these changes Apr 18, 2024

View reviewed changes

tobiaskaestner reviewed Apr 22, 2024

View reviewed changes

stanislaw closed this May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Software requirements: Semaphore: collected all requirements so far #58

Software requirements: Semaphore: collected all requirements so far #58

stanislaw commented Apr 9, 2024

gregshue left a comment

gregshue Apr 9, 2024

nicpappler Apr 18, 2024

gregshue Apr 9, 2024

gregshue Apr 9, 2024

nicpappler Apr 18, 2024

gregshue Apr 25, 2024

gregshue Apr 9, 2024

nicpappler Apr 18, 2024

gregshue Apr 9, 2024

nicpappler Apr 18, 2024

gregshue Apr 25, 2024

gregshue Apr 9, 2024

gregshue Apr 9, 2024

nicpappler left a comment

nicpappler Apr 18, 2024

nicpappler Apr 18, 2024

nicpappler Apr 18, 2024

nicpappler Apr 18, 2024

nicpappler Apr 18, 2024

tobiaskaestner left a comment

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

tobiaskaestner Apr 22, 2024

stanislaw commented May 2, 2024

Software requirements: Semaphore: collected all requirements so far #58

Software requirements: Semaphore: collected all requirements so far #58

Conversation

stanislaw commented Apr 9, 2024

gregshue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicpappler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiaskaestner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stanislaw commented May 2, 2024