-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Conversation
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>
There was a problem hiding this 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.
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. |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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)
@@ -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\" " |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
need another approach that supports rst |
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