Skip to content

drivers/periph/i2c: Replace LaTeX formula with static SVG file in documentation#21876

Merged
AnnsAnns merged 1 commit intoRIOT-OS:masterfrom
crasbe:pr/periph_i2c_latex
Nov 13, 2025
Merged

drivers/periph/i2c: Replace LaTeX formula with static SVG file in documentation#21876
AnnsAnns merged 1 commit intoRIOT-OS:masterfrom
crasbe:pr/periph_i2c_latex

Conversation

@crasbe
Copy link
Contributor

@crasbe crasbe commented Nov 12, 2025

Contribution description

I started adding the MathJax support as discussed in #21490 but then I realized that it is somewhat stupid to add such a big dependency for static LaTeX formulas and instead asked ChatGPT to generate SVG files from said LaTeX formulas.

Also I changed the comments from // to /* */ to stay consistent with our Coding Convention.
This breaks things when not inside a doc.md file.

Testing procedure

Check the generated Doxygen documentation in the subpage group__drivers__periph__i2c.html.

Issues/PRs references

Fixes #21490.
Depends on (and currently includes) #21372.

@crasbe crasbe requested a review from maribu November 12, 2025 23:31
@crasbe crasbe added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Nov 12, 2025
@crasbe crasbe added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 12, 2025
@crasbe crasbe requested a review from AnnsAnns as a code owner November 12, 2025 23:31
@crasbe crasbe added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Nov 12, 2025
@github-actions github-actions bot added Area: doc Area: Documentation Area: drivers Area: Device drivers Area: tools Area: Supplementary tools labels Nov 12, 2025
@riot-ci
Copy link

riot-ci commented Nov 12, 2025

Murdock results

✔️ PASSED

1cd37aa drivers/periph/i2c: replace LaTeX eqn. with SVG in doc

Success Failures Total Runtime
1 0 1 02m:12s

Artifacts

@crasbe
Copy link
Contributor Author

crasbe commented Nov 12, 2025

This is what it looks like with Doxygen 1.15.0:
image

This is what it looks like with Doxygen 1.13.2 (which currently generates our Website):
image

This is what it looks like with Doxygen 1.9.1 (which is still used by Murdock) with enforced Dark Mode:
image

This is what it looks like with Doxygen 1.9.1 (which is still used by Murdock) without enforced Dark Mode:
image

@crasbe crasbe force-pushed the pr/periph_i2c_latex branch from 8659618 to bf3e118 Compare November 12, 2025 23:52
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) and removed Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Nov 13, 2025
@maribu
Copy link
Member

maribu commented Nov 13, 2025

image

I have seen Github showing even complex SVG images, so I would assume that the SVG source is not valid syntax.

Copy link
Member

@AnnsAnns AnnsAnns left a comment

Choose a reason for hiding this comment

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

RE: SVG problem

I've encountered that a few times during my bachelor thesis so far. The trick is to re-export the SVG using inkscape because SVG rendering is cursed weird.

@AnnsAnns
Copy link
Member

Oops, did not want to approve with this but I guess the approval still counts once the SVG is fixed 😅

Copy link
Member

@AnnsAnns AnnsAnns left a comment

Choose a reason for hiding this comment

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

Nevermind, no pre-approval for you 🤨

@AnnsAnns AnnsAnns removed the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Nov 13, 2025
@kfessel
Copy link
Contributor

kfessel commented Nov 13, 2025

Why not just get rid of the math rendering and go for somethin code style

R_min = (V_DD - V_OL) / I_OL
R_max = t_r / (0.8473 * C_b)
Where: 
    V_DD : Supply voltage
    V_OL = Low level voltage upper limit 
    I_OL =  Low level output current,
    t_r =\Signal rise time,
    C_b = Bus capacitance

this would make the formular readable in the comment (and not only in doxygen / html rendering)

the latex-math is barely readable in the c-comment and the svg even less

* \f$ I_{OL} =\f$ Low level output current,
* \f$ t_r =\f$ Signal rise time,
* \f$ C_b =\f$ Bus capacitance <br>
* @image html periph_i2c_bus_equations.svg
Copy link
Contributor

Choose a reason for hiding this comment

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

KISS

Suggested change
* @image html periph_i2c_bus_equations.svg
* R_min = (V_DD - V_OL) / I_OL
* R_max = t_r / (0.8473 * C_b)
* Where:
* V_DD: Supply voltage
* V_OL: Low level voltage upper limit
* I_OL: Low level output current,
* t_r: Signal rise time,
* C_b: Bus capacitance

Copy link
Contributor Author

@crasbe crasbe Nov 13, 2025

Choose a reason for hiding this comment

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

I like the utilitarian approach, but the SVG image is more beautifuller :)

@crasbe crasbe force-pushed the pr/periph_i2c_latex branch 2 times, most recently from adba415 to f790d71 Compare November 13, 2025 13:14
@github-actions github-actions bot removed the Area: tools Area: Supplementary tools label Nov 13, 2025
@crasbe crasbe removed the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 13, 2025
@AnnsAnns
Copy link
Member

Squashy squash pls

@crasbe crasbe force-pushed the pr/periph_i2c_latex branch from f790d71 to 1cd37aa Compare November 13, 2025 14:13
@AnnsAnns AnnsAnns added this pull request to the merge queue Nov 13, 2025
Merged via the queue into RIOT-OS:master with commit ff7a266 Nov 13, 2025
25 checks passed
@crasbe crasbe deleted the pr/periph_i2c_latex branch November 13, 2025 14:25
@crasbe
Copy link
Contributor Author

crasbe commented Nov 13, 2025

Thank you for the review and the modified SVG :)

@leandrolanzieri leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: doc Area: Documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc/doxygen: Broken LaTeX Formulas

6 participants