Skip to content

Conversation

@Mr-Anyone
Copy link
Contributor

Description

Added Deoxygen support:

see this:

https://mr-anyone.github.io/Thunderbot_Software
https://mr-anyone.github.io/Thunderbot_Software/classPlay.html
https://mr-anyone.github.io/Thunderbot_Software/classTactic.html
https://mr-anyone.github.io/Thunderbot_Software/classThreadedAi.html

see here: https://github.com/Mr-Anyone/Thunderbot_Software

Length Justification and Key Files to Review

Most of it is css file and config file. N/A

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

@Mr-Anyone
Copy link
Contributor Author

image

I think someone need to set this up because I don't have repo access.

@Mr-Anyone Mr-Anyone force-pushed the doxygen branch 2 times, most recently from ec56658 to d067cb0 Compare May 9, 2025 15:53
Andrewyx
Andrewyx previously approved these changes May 17, 2025
Copy link
Contributor

@Andrewyx Andrewyx left a comment

Choose a reason for hiding this comment

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

lgtm

@Andrewyx
Copy link
Contributor

Andrewyx commented May 21, 2025

Small error here. See TOC
image

Copy link
Contributor

@itsarune itsarune left a comment

Choose a reason for hiding this comment

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

looks useful, I think you may have messed up the table of contents rendering in a bunch of markdown files. otherwise, LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

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

where is this script used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default, doxygne doesn't support mermaid file format for this page. This is a solution I stole from here to fix it.

The play here maybe to re-implement fsm_diagram_generator.py to generate xdot instead of mermaid. xdot is supported by doxygen by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see line 1039 in the doxygen file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the FSM diagrams loading properly? They look a bit strange on the link you attached

Copy link
Contributor

@itsarune itsarune left a comment

Choose a reason for hiding this comment

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

have you considered using rules_doxygen?

@Mr-Anyone
Copy link
Contributor Author

have you considered using rules_doxygen?

No, I haven't. I will give it an attempt.

@Mr-Anyone
Copy link
Contributor Author

Small error here. See TOC image

looks useful, I think you may have messed up the table of contents rendering in a bunch of markdown files. otherwise, LGTM!

I assume you're referring to how the table of contents is displayed on GitHub (e.g., this page). As far as I can tell, the TOC generated by Doxygen itself is working as intended.

This is a bit painful to fix:

Doxygen uses the [TOC] tag to generate a table of contents (reference). However, GitHub doesn't recognize [TOC]—instead, we’ve been using this tool to create GitHub-compatible TOCs.

The problem is, including both [TOC] and the GitHub-generated TOC leads to formatting issues. On Doxygen pages, you get a random-looking list of pages; on GitHub, you’re left with the [TOC] tag rendered as plain text.

My thinking was to simplify this by moving all documentation to doxygen. I am not sure how to fix the rendering on both Github and doxygen at the same time.

@itsarune
Copy link
Contributor

my sense is that right now, most people aren't looking at the locally rendered markdown files and just look online. So it's okay to ignore the markdown table of contents rendering in favour of doxygen (like you've done)

@Andrewyx
Copy link
Contributor

Andrewyx commented Jun 1, 2025

Goodness gracious, rules_doxygen uses MODULE.bazel from bazel 8, if you do use it, can you branch off from the bazel 8 migration branch?

@Mr-Anyone
Copy link
Contributor Author

Goodness gracious, rules_doxygen uses MODULE.bazel from bazel 8, if you do use it, can you branch off from the bazel 8 migration branch?

Sure. I will rebase from your branch or wait until you are done.

@Andrewyx
Copy link
Contributor

@Mr-Anyone Should be safe for rebase now off the bzl branch

@nycrat
Copy link
Member

nycrat commented Dec 8, 2025

@Mr-Anyone Is this still being worked on? Doxygen docs would be pretty useful and this looks pretty good.

I did notice some minor bugs and I'll create a PR on your fork to resolve these issues.

@Mr-Anyone
Copy link
Contributor Author

@Mr-Anyone Is this still being worked on? Doxygen docs would be pretty useful and this looks pretty good.

I did notice some minor bugs and I'll create a PR on your fork to resolve these issues.

Thank you for noticing it. I think it might go unnoticed for a long time.

This has taken a bit longer than expected because it is sort of in a limbo state since there are still a few open ended questions: (1) the table-of-contents issue Arun mentioned, (2) the bazel rules_doxygen issue, and (3) the mermaid/xdot issue. I probably have to follow up with the leads with that. What do you think?

It may be a good idea to migrate from mermaid to xdot instead because doxygen natively support xdot.

@nycrat
Copy link
Member

nycrat commented Dec 8, 2025

I think this PR is actually pretty close to being merged.

  1. Judging by this comment from Arun,

my sense is that right now, most people aren't looking at the locally rendered markdown files and just look online. So it's okay to ignore the markdown table of contents rendering in favour of doxygen (like you've done)

I think we are good to just use the Doxygen TOC format, and remove script that generates the markdown TOCs. So the TOC issue is resolved

  1. The Bazel 8 migration has already been merged in Andrewyx/Upgrade Bazel 8 #3459, and I checked by merging master into the Doxygen branch on my own fork and everything seems to work fine. If we want to use rules_doxygen, there should be no issue

  2. About the mermaid/xdot issue, I think this is the one area that might need some work. There are still a few rendering issues with mermaid:

image

But I think that these issues can be fixed and we won't need to switch everything to xdot.

Other than that everything should be good 👍

@Andrewyx Andrewyx assigned Lmh-java and unassigned Lmh-java Jan 9, 2026
@Andrewyx
Copy link
Contributor

Andrewyx commented Jan 9, 2026

@Lmh-java Would you be able to work your magic and finish off this PR? I think it is mostly done, just a few small bugs left

@nycrat nycrat mentioned this pull request Jan 9, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants