Fix broken / update ign links in tutorials (Harmonic) #2107

mabelzhang · 2023-09-01T04:30:27Z

🦟 Bug fix

Fix broken links in tutorial rendered here https://gazebosim.org/api/sim/8/terminology.html
This is targeting Harmonic directly instead of waiting for forward-port because the tutorial party is happening right now.

To test
Compile, then in a browser, navigate to build/gz-sim8/doxygen/html/tutorials.html

Checklist

Signed all commits for DCO
Added tests
Updated documentation (as needed)
Updated migration guide (as needed)
Consider updating Python bindings (if the library has them)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Mabel Zhang <[email protected]>

codecov · 2023-09-01T06:02:14Z

Codecov Report

Merging #2107 (8216769) into gz-sim8 (8e7f612) will increase coverage by 0.12%.
Report is 23 commits behind head on gz-sim8.
The diff coverage is 90.44%.

❗ Current head 8216769 differs from pull request most recent head 8c97df5. Consider uploading reports for the commit 8c97df5 to get more accurate results

@@             Coverage Diff             @@
##           gz-sim8    #2107      +/-   ##
===========================================
+ Coverage    65.34%   65.47%   +0.12%     
===========================================
  Files          322      323       +1     
  Lines        30554    30704     +150     
===========================================
+ Hits         19967    20102     +135     
- Misses       10587    10602      +15

Files Changed	Coverage Δ
src/SimulationRunner.cc	`91.73% <50.00%> (-0.40%)`	⬇️
src/Server.cc	`87.07% <80.00%> (-0.06%)`	⬇️
src/MeshInertiaCalculator.cc	`92.75% <92.75%> (ø)`
src/systems/physics/Physics.cc	`71.90% <100.00%> (-0.07%)`	⬇️
src/systems/user_commands/UserCommands.cc	`60.80% <100.00%> (ø)`

azeey · 2023-09-01T17:04:54Z

tutorials/terminology.md

@@ -10,19 +10,19 @@ to developers touching the source code.

 * **Entity**: Every "object" in the world, such as models, links,
    collisions, visuals, lights, joints, etc.
-    An entity [is just a numeric ID](namespace gz_1_1gazebo.html#ad83694d867b0e3a9446b535b5dfd208d),
+    An entity [is just a numeric ID](namespacegz_1_1sim.html#ad83694d867b0e3a9446b535b5dfd208d),


I think there's a better way to do this than linking to the generated files. Can you try \ref gz::sim::Entity "is just a numeric ID"?

Cool, the links look the same for me.
I did a grep and actually there are no other places using this kind of \ref syntax. Is that preferred?

Another pattern that might need fixing is, I did a grep on classignition and we actually have quite a few hard URL links that are going to the ignition::gazebo namespace and have "Ignition Gazebo" in the title.

I'm talking about like this page
https://gazebosim.org/api/sim/8/resources.html
the PluginInfo links to this hard URL https://gazebosim.org/api/gazebo/7/classignition_1_1gazebo_1_1ServerConfig_1_1PluginInfo.html
and should instead go to
https://gazebosim.org/api/sim/8/classgz_1_1sim_1_1ServerConfig_1_1PluginInfo.html

They should be changed to at least relative links, if not \ref. But not all of them. Some of them, like classignition_1_1msgs need to stay, as the gz version doesn't seem to exist?
Like gz::msgs::EntityFactory in the entity creation tutorial links to
https://gazebosim.org/api/msgs/6.0/classignition_1_1msgs_1_1EntityFactory__V.html linked from on
But the gz version of the URL doesn't exist:
https://gazebosim.org/api/msgs/6.0/classgz_1_1msgs_1_1EntityFactory__V.html

fdfe8b4 changes the links in this tutorial to \ref.

One downside is that \ref doesn't play well with text formatting. The bullet heading "Event manager" boldface is lost and I tried a few ways but can't get it back. It may be a known behavior doxygen/doxygen#9023

5a0c4a0 makes the hard URLs mentioned above into local links.
I didn't change those to \ref, because they are mostly in the migration tutorials, mostly point to the header files as opposed to the class definition. The \ref way goes to the class definition.

142c447 is along the same vein - updates more URLs from ignition to gz, and change some \ref links

It also updates to the latest library release version.
I believe I've tested all the new URLs.
@quarkytale maybe you could help review this PR, since this is now related to the docs PR updating Harmonic tutorials... a lot of link version updates.

The usage looks good to me but need to test it once!

Okay, I see why I thought other libraries don't work.

gz-msgs, as mentioned above, seems to not follow the pattern of other libraries. A line like this already exists for gz-msgs, but \ref gz::msgs::EntityFactory,\ref gz::msgs::Model, and \ref gz::msgs::Light still don't work. I'm not sure why. I compared the CMakeLists.txt of gz-msgs with others, and it doesn't seem to be missing anything obvious.

Could it be because gz-msgs headers are generated from protobuf?

Could it be because gz-msgs headers are generated from protobuf?

Yes. We used to generate doxygen for the messages (see https://gazebosim.org/api/msgs/8/namespaceignition_1_1msgs.html), but I think we removed them from gz-msgs10. @mjcarroll, do we plan to add them back?

Signed-off-by: Mabel Zhang <[email protected]>

tutorials/terminology.md

tutorials/migration_actor_api.md

tutorials/migration_joint_api.md

tutorials/migration_light_api.md

tutorials/migration_link_api.md

tutorials/migration_model_api.md

tutorials/migration_sensor_api.md

tutorials/migration_world_api.md

tutorials/rendering_plugins.md

tutorials/terminology.md

azeey · 2023-09-14T04:44:19Z

This needs to target gz-sim8.

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang · 2023-09-14T05:09:43Z

tutorials/resources.md

@@ -62,7 +62,7 @@ Gazebo will look for GUI plugins on the following paths, in order:
 2. [GUI plugins that are installed with Gazebo](https://github.com/gazebosim/gz-sim/tree/main/src/gui/plugins)
 3. Other paths added by calling `gz::gui::App()->AddPluginPath`
 4. `~/.gz/gui/plugins`
-5. [Plugins which are installed with Gazebo GUI](https://gazebosim.org/api/gui/4/namespace gz_1_1gui_1_1plugins.html)
+5. [Plugins which are installed with Gazebo GUI](https://gazebosim.org/api/gui/8/namespacegz_1_1gui_1_1plugins.html)


This one doesn't work with \ref links.

Maybe the files defining plugins in gz-gui/src/plugins/need something like this in the source code to mark the namespace:

gz-sim/src/systems/ackermann_steering/AckermannSteering.hh

Lines 29 to 30 in 9d79b62

// Inline bracket to help doxygen filtering.

inline namespace GZ_SIM_VERSION_NAMESPACE {

Probably won't change that now.

Looking at the gz-gui8.tag.xml file, I expected to see

<compound kind="namespace"> <name>gz::gui::plugins</name> ...

but I don't. Adding

/// \brief Namespace for all plugins namespace plugins { }

to gz-gui/include/gz/gui/Plugin.hh does create the namespace in gz-gui8.tag.xml, so that's probably the necessary fix. I don't think the inline namespace is the issue.

Okay. I'll put that single fix in another PR if that's okay, so that the fixes in this PR don't have to wait for the gz-gui fix.

Welp, I just opened a PR gazebosim/gz-gui#571

gz::gui::plugins now works in 8c97df5
But this PR now depends on gz-gui#571 above. So don't merge this before that. Or I can break out this one change into a new PR if the gz-gui one takes too long.

quarkytale

All the links work for me! But in all the migration guide pages (migration_*.md) the title isn't coming up right, it just shows Gazebo Sim: $title, see Garden's version https://gazebosim.org/api/sim/7/migrationworldapi.html

tutorials/terminology.md

tutorials/resources.md

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang · 2023-09-14T20:37:53Z

Re title of migration tutorials - good catch!
The migration tutorials did the titles a bit differently from other tutorials - I had wondered about that, the migration titles were always much bigger than other tutorials.
Fixed in bfe664b

it just shows Gazebo Sim: $title

Mine looks a bit different. Before the fix:

After the fix, it now looks consistent with the other tutorials:

Are you using Doxygen 1.9.2 (see gazebosim/docs#388 (comment))?

Either way, it should look right now.

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang · 2023-09-14T21:05:33Z

This PR now dependent on gazebosim/gz-gui#571 , which is just the one change per #2107 (comment)
It's just for the ref link of gz::gui::plugins in 8c97df5
I can separate it into a new PR if we want to do that.

quarkytale · 2023-09-14T22:16:55Z

The migration tutorials did the titles a bit differently from other tutorials - I had wondered about that, the migration titles were always much bigger than other tutorials.

Oh I meant in the browser's title bar, but its working now after the fix!

fix broken link to API

dce7acf

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang added the beta Targeting beta release of upcoming collection label Sep 1, 2023

a few more

79c51cd

Signed-off-by: Mabel Zhang <[email protected]>

azeey reviewed Sep 1, 2023

View reviewed changes

mabelzhang added 3 commits September 2, 2023 01:07

use \ref for links. Revert common::Event link

fdfe8b4

Signed-off-by: Mabel Zhang <[email protected]>

make links local so that version updates automatically

5a0c4a0

Signed-off-by: Mabel Zhang <[email protected]>

update URLs to latest release versions, and ignition->gz namespace

142c447

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang changed the title ~~Fix broken links in terminology tutorial (Harmonic)~~ Fix broken / update ign links in tutorials (Harmonic) Sep 2, 2023

mabelzhang requested a review from quarkytale September 2, 2023 06:26

Bi0T1N reviewed Sep 2, 2023

View reviewed changes

tutorials/terminology.md Outdated Show resolved Hide resolved

This was referenced Sep 2, 2023

gz-sim: Terminology gazebosim/gazebo_test_cases#378

Closed

gz-sim: Terminology gazebosim/gazebo_test_cases#379

Closed

azeey reviewed Sep 13, 2023

View reviewed changes

azeey added the documentation Improvements or additions to documentation label Sep 13, 2023

change to \ref links

f16de9f

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang requested a review from mjcarroll as a code owner September 14, 2023 04:45

mabelzhang changed the base branch from main to gz-sim8 September 14, 2023 04:46

azeey approved these changes Sep 14, 2023

View reviewed changes

change to \ref link

4cb5d38

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang commented Sep 14, 2023

View reviewed changes

quarkytale reviewed Sep 14, 2023

View reviewed changes

tutorials/terminology.md Show resolved Hide resolved

tutorials/resources.md Outdated Show resolved Hide resolved

fix migration tutorial titles

bfe664b

Signed-off-by: Mabel Zhang <[email protected]>

mabelzhang mentioned this pull request Sep 14, 2023

Add plugins namespace gazebosim/gz-gui#571

Merged

8 tasks

ref link for gz::gui::plugins, dependent on gz-gui PR#571

8c97df5

Signed-off-by: Mabel Zhang <[email protected]>

iche033 mentioned this pull request Sep 14, 2023

gz-sim: Rendering plugins gazebosim/gazebo_test_cases#358

Closed

2 tasks

quarkytale approved these changes Sep 14, 2023

View reviewed changes

mabelzhang merged commit 294fddc into gz-sim8 Sep 14, 2023
4 of 7 checks passed

mabelzhang deleted the mabelzhang/tutorial_fix_links_h branch September 14, 2023 23:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix broken / update ign links in tutorials (Harmonic) #2107

Fix broken / update ign links in tutorials (Harmonic) #2107

mabelzhang commented Sep 1, 2023 •

edited

Loading

codecov bot commented Sep 1, 2023 •

edited

Loading

azeey Sep 1, 2023

mabelzhang Sep 1, 2023

mabelzhang Sep 2, 2023 •

edited

Loading

mabelzhang Sep 2, 2023

mabelzhang Sep 2, 2023 •

edited

Loading

quarkytale Sep 11, 2023

mabelzhang Sep 14, 2023

azeey Sep 14, 2023

azeey commented Sep 14, 2023

mabelzhang Sep 14, 2023

azeey Sep 14, 2023

mabelzhang Sep 14, 2023

mabelzhang Sep 14, 2023

mabelzhang Sep 14, 2023

quarkytale left a comment

mabelzhang commented Sep 14, 2023

mabelzhang commented Sep 14, 2023 •

edited

Loading

quarkytale commented Sep 14, 2023

	// Inline bracket to help doxygen filtering.
	inline namespace GZ_SIM_VERSION_NAMESPACE {

Fix broken / update ign links in tutorials (Harmonic) #2107

Fix broken / update ign links in tutorials (Harmonic) #2107

Conversation

mabelzhang commented Sep 1, 2023 • edited Loading

🦟 Bug fix

Checklist

codecov bot commented Sep 1, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mabelzhang Sep 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mabelzhang Sep 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azeey commented Sep 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quarkytale left a comment

Choose a reason for hiding this comment

mabelzhang commented Sep 14, 2023

mabelzhang commented Sep 14, 2023 • edited Loading

quarkytale commented Sep 14, 2023

mabelzhang commented Sep 1, 2023 •

edited

Loading

codecov bot commented Sep 1, 2023 •

edited

Loading

mabelzhang Sep 2, 2023 •

edited

Loading

mabelzhang Sep 2, 2023 •

edited

Loading

mabelzhang commented Sep 14, 2023 •

edited

Loading