Skip to content

Conversation

@gmarull
Copy link
Member

@gmarull gmarull commented Oct 25, 2021

This PR adds a new extension that converts Sphinx C domain roles (e.g., :c:func:`zephyr_api_call` ) to links to Doxygen pages.

The end goal is to use Doxygen and not breathe to render APIs. There are a few reasons to do that:

  • Doxygen is faster to render API docs (hard to measure “how faster” as not all API are part of Sphinx today).
  • As of today, not all APIs are rendered using breathe, creating a sort of incomplete documentation
  • Doxygen renders everything automatically, there is no need to explicitly enable groups
  • Doxygen docs look & feel is now more in line with Zephyr docs

Sphinx should still be the central place for documentation: GSG, usage guidelines, examples, tutorials, etc. These are the type of docs where rich content is important and where Sphinx is clearly better than Doxygen.

Note: The extension is a proof of concept, so it may have issues (e.g., PDF not supported yet, does not support incremental builds properly, etc.).

Live PoC: https://teslabs.github.io/zephyr-doctest/

@gmarull gmarull added area: Documentation RFC Request For Comments: want input from the community dev-review To be discussed in dev-review meeting labels Oct 25, 2021
@nashif
Copy link
Member

nashif commented Oct 25, 2021

the million dollar question is how long does it take to build the documentation with this change? :)

@gmarull
Copy link
Member Author

gmarull commented Oct 25, 2021

the million dollar question is how long does it take to build the documentation with this change? :)

Some quick numbers from CI, I've taken a couple of random PRs:

image
image

image
image

This PR:
image
image

So, the gain seems to be about ~25% for HTML, ~50% for PDF. PDF goes down to 879 pages from 1905.

@henrikbrixandersen
Copy link
Member

I like the idea of separating the two, but I don't think we should do it based on the time it takes to build the documentation. We should change it if this is a better way of presenting the Zephyr documentation.

Personally I've always been a bit puzzled as to why we provide both "inline API" documentation and the stand-alone Doxygen documentation.

In my opinion, this really comes down to UX. Which is the better user experience?

@gmarull
Copy link
Member Author

gmarull commented Oct 26, 2021

I like the idea of separating the two, but I don't think we should do it based on the time it takes to build the documentation. We should change it if this is a better way of presenting the Zephyr documentation.

Personally I've always been a bit puzzled as to why we provide both "inline API" documentation and the stand-alone Doxygen documentation.

In my opinion, this really comes down to UX. Which is the better user experience?

I agree, even though build time is a UX parameter IMO. Kconfig is probably a better example where build time leads to a bad UX.

Remove all usages of breathe.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Deactivate breathe extension from docs.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add an initial version of doxybridge, an extension that allows to use
Sphinx C domain to automatically reference to Doxygen pages. This
extension is a proof of concept and so it is not expected to be fully
functional.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Enable the doxybridge extension.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@nashif
Copy link
Member

nashif commented Oct 28, 2021

We need a link for each subsystem under the API Reference section:

image

@nashif
Copy link
Member

nashif commented Oct 28, 2021

is there a way to have the same left sidebar colors and layout on both API and RSR documentation?

@nashif
Copy link
Member

nashif commented Oct 28, 2021

image

vs

image

@nashif
Copy link
Member

nashif commented Oct 28, 2021

dev-meeting: stick with current approach and look into optimizing kconfig rendering.

@mbolivar-nordic
Copy link
Contributor

dev-meeting: stick with current approach and look into optimizing kconfig rendering.

So this will be closed, then? It's not handling the DT API reference correctly, so I want to know if I need to leave detailed comments or if this is no longer relevant.

@gmarull
Copy link
Member Author

gmarull commented Nov 10, 2021

dev-meeting: stick with current approach and look into optimizing kconfig rendering.

So this will be closed, then? It's not handling the DT API reference correctly, so I want to know if I need to leave detailed comments or if this is no longer relevant.

I imagine DT does not work because it uses :c:func: for macros instead of :c:macro:. I think for now we should focus on improving Doxygen structure first (and Kconfig performance problems). My feeling is that we'll likely need to revisit this in the future, I don't think breathe will improve anytime soon (I wish I was wrong, though). Not only that, but I see Doxygen as a superior option to render API docs, specially now that they look better with the theme.

@carlescufi carlescufi removed the dev-review To be discussed in dev-review meeting label Nov 11, 2021
@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Nov 11, 2021 via email

@gmarull
Copy link
Member Author

gmarull commented Nov 11, 2021

No, I am referring to doc/reference/devicetree/api.rst. This uses doxygengroup extensively and the macros are no longer rendering in the output.

This was the idea, to stop using breathe to render API docs. I just dropped all breathe directives for testing.

That's fine, but we need to make sure the existing API docs are still OK and don't silently drop expected content, right?

Doxygen renders everything breathe can render. In case of Zephyr, Doxygen contains more stuff, since all APIs are rendered without the need to manually specify which groups should appear.

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Nov 11, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Documentation RFC Request For Comments: want input from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants