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

doc: fix multiple Doxygen and breathe problems #33972

Merged
merged 5 commits into from
Apr 7, 2021

Conversation

gmarull
Copy link
Member

@gmarull gmarull commented Apr 2, 2021

Introduction

This PR is an attempt to fix multiple documentation problems we have when using Doxygen + Breathe.

Summary of the changes

Remaining issues

The remaining issues should be fixed by sphinx-doc/sphinx#8313. TLDR; as of today you can't have something like:

 /* demo considered a duplicate because the name is used by both struct and function */
struct demo {
...
};

void demo(...);

/* id considered duplicate */
union {
	uint16_t id;
	struct {
		uint16_t id;
	} vnd;
};

After Sphinx fixes the problem above we should be able to (hopefully) get rid of the current "known issues".

zephyr.warnings_filter extension

Instead of post-processing Sphinx output, I have created a small extension that natively filters out Sphinx warnings. This Sphinx plugin can be used to filter out warnings that are known to be false positives. The warnings are filtered out based on a set of regular expressions given via a configuration file, the same principle as before.

At the same time I have simplified current regular expressions and moved them to doc/known-warnings.txt with the idea to make docs more self-contained.

include/bluetooth/gatt.h Outdated Show resolved Hide resolved
@gmarull gmarull requested a review from jhedberg April 5, 2021 22:14
@gmarull gmarull added the dev-review To be discussed in dev-review meeting label Apr 5, 2021
@gmarull gmarull requested review from mbolivar-nordic and jhedberg and removed request for jhedberg and mbolivar-nordic April 6, 2021 08:31
@gmarull gmarull force-pushed the fix/breathe-warnings branch 3 times, most recently from 2bf59f9 to 46fcb51 Compare April 6, 2021 13:41
Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

LGTM

@carlescufi
Copy link
Member

carlescufi commented Apr 6, 2021

API meeting 2021.04.06

  • Remove the commit net: net_ip: hide some external and forward declarations from Doxygen. Instead of ifdefing out the forward declarations, the warnings will be suppressed by the new plugin
  • Otherwise agreed to the changes

@carlescufi carlescufi mentioned this pull request Apr 6, 2021
gmarull added 5 commits April 6, 2021 19:00
This option prevents some structures to have missing pages on the
Doxygen HTML output.

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
`IEEE802154_MAX_ADDR_LENGTH` was defined in 2 headers, keep one only.

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
Predefine ATOMIC_DEFINE in the Doxyfile so that documentation output is
generated correctly. In order to simplify the predefinition
ATOMIC_BITMAP_SIZE has been introduced.

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
warnings_filter is an extension that allows to filter out Sphinx
warnings.

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
Replace current filter setup with the warnings_filter Sphinx extension.
Note that current regexes have been simplified to make them more
readable while keeping the most relevant information.

Sphinx warnings are now treated as errors (-W).

Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
@gmarull gmarull force-pushed the fix/breathe-warnings branch from 46fcb51 to 36494d9 Compare April 6, 2021 17:48
@carlescufi carlescufi merged commit 96c7d10 into zephyrproject-rtos:master Apr 7, 2021
@gmarull gmarull deleted the fix/breathe-warnings branch April 7, 2021 14:24
@marc-hb
Copy link
Collaborator

marc-hb commented Apr 11, 2021

While testing #33448 I noticed that stderr output from Sphinx (and more commonly) from the very many subprocesses it forks) escaped the defunct filter-known-issues.py and tended to get drowned in the rest of the output and missed. @gmarull I'm curious whether what is the current status of stderr after this PR, unchanged? Just wondering whether you're aware of that since you're on top of everything documentation-related now.

@gmarull
Copy link
Member Author

gmarull commented Apr 13, 2021

While testing #33448 I noticed that stderr output from Sphinx (and more commonly) from the very many subprocesses it forks) escaped the defunct filter-known-issues.py and tended to get drowned in the rest of the output and missed. @gmarull I'm curious whether what is the current status of stderr after this PR, unchanged? Just wondering whether you're aware of that since you're on top of everything documentation-related now.

This PR introduced a way of filtering warnings that does not deal anymore with the output of Sphinx. What it does is to directly intercept Sphinx warning logs and filters them out (or converts them to INFO in non silent mode). So the stderr will be what Sphinx originally outputs there.

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: Base OS Base OS Library (lib/os) area: Bluetooth area: Documentation area: Networking dev-review To be discussed in dev-review meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants