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

Documentation: use @allowed_from to document allowed contexts #21389

Closed
wants to merge 3 commits into from

Conversation

nashif
Copy link
Member

@nashif nashif commented Dec 13, 2019

This just transforms what we have right now in form of an unofficial @note to a
doxygen command the ultimitealy should be applied to all APIs to document the
context they can be called from.

A fix for a loose doxygen group in a test is also included.

Resolves #21061

@zephyrbot zephyrbot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test area: Kernel area: Documentation labels Dec 13, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 13, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

Add allowed_from doxygen command to document the context from which an
API can be called from.

Right now we have no standard way for documenting what APIs can be
called from where and if they for example are allowed to be called from
thread context or ISR context or if they can be used during
initialization of the system for example.

For example instead of now:
* @note Can be called by ISRs.

We will use:
* @allowed_from Threads, ISRs

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Instead of custom @note, use @allowed_from to document context an API
can be called from.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Doxygen groups were not balanced and had wrong name.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is active work on #18970 in draft form in #20959 to address concerns about API behavior and use across both kernel and non-kernel API, which I expect to have in reviewable form by early January. At this time "Allowed from" does not adequately cover the attributes that must be identified (in particular it fails to identify reschedule points, which affect thread behavior).

I would appreciate an opportunity to complete that work and get it through the API review process before rushing to a solution that may not cover the distinctions we need.

@nashif
Copy link
Member Author

nashif commented Dec 14, 2019

I would appreciate an opportunity to complete that work and get it through the API review process before rushing to a solution that may not cover the distinctions we need.

Sure, this can wait, however I am not sure exactly how to read #20959 and where all of this will end up, it would be great to have this in the form of a proposed change.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new doxygen comments (using @allowed_from to create an xrefitem) will only show up in the doxygen-generated HTML and not in the Sphinx/Breathe API documentation we currently publish. Breathe ignores these xrefitem entries (but does properly handle @note items), so that's something to think about. Depending on the resolution of the discussion started by @pabigot, we may want to instead use an alias to create @note directives for this kind of information so it can be handled by and appear in the breathe-processed API docs as well.

@@ -776,6 +780,8 @@ extern FUNC_NORETURN void k_thread_user_mode_enter(k_thread_entry_t entry,
* need to be.
* Note that NULL shouldn't be passed as an argument.
*
* @allowed_from Supervisor Mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is allowed from supervisor mode.
I think it's more useful to indicate which core kernel APIs are callable from user mode (although I suspect nearly all or all of them are tagged with __syscall)

@zephyrbot zephyrbot closed this Jan 2, 2020
@zephyrbot zephyrbot reopened this Jan 2, 2020
@@ -224,6 +224,7 @@ ALIASES = "rst=\verbatim embed:rst:leading-asterisk" \
"endrst=\endverbatim"

ALIASES += "req=\xrefitem req \"Requirement\" \"Requirements\" "
ALIASES += "allowed_from=\xrefitem context \"Allowed from\" \"Allowed from\" "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the random change in indentation?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a random tab in there. Could get rid of it.

@nashif
Copy link
Member Author

nashif commented Feb 5, 2020

need another approach that supports rst

@nashif nashif closed this Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Documentation area: Kernel area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document where APIs can be called from using doxygen
6 participants