Fix Compass training doc#5786
Conversation
There was a problem hiding this comment.
Documentation Review
Thanks for this documentation improvement, @sameerchavan0027! 📝
Summary
This PR simplifies the USD asset installation instructions in the COMPASS navigation policy tutorial by consolidating multiple code blocks and steps into a more streamlined workflow.
Changes Reviewed
| Aspect | Assessment |
|---|---|
| Clarity | ✅ The new instructions are clearer and more concise |
| Accuracy | ✅ Commands are syntactically correct |
| Completeness | ✅ All necessary steps are preserved |
Specific Feedback
👍 What works well:
- Consolidating the extraction and move commands into a single code block reduces cognitive overhead
- The explicit
cd <compass-nurec>/COMPASSmakes it clear where the user should be working - Using a straightforward
mvcommand instead of describing source/destination paths separately is more intuitive
💡 Minor suggestion (non-blocking):
Consider adding a note about removing the extracted groot_mobility_rl_es_usds/ directory after moving the usd folder, or alternatively using mv with the -v flag for verbose output so users can confirm the operation succeeded.
Checklist Items
- The PR description template is not fully filled out (the type of change and issue number are missing). For future documentation PRs, marking "Documentation update" and linking to the relevant bug (6199408 mentioned in the branch name) would help with traceability.
Overall this is a helpful cleanup that will make the setup process easier to follow. No blocking concerns.
Update (f6c7e4e): Reviewed new commits after rebase to develop branch. The documentation changes remain the same - the incremental diff shows no file modifications, indicating this was a clean rebase to the new target branch. No new concerns introduced. ✅
Update (226d3c5): Reviewed additional commits. Nice improvements! 🎉
New additions:
- Added a helpful tip box with
--includeand--excludeglob filter examples for selective dataset downloads - this is excellent for users who only need specific environments and want to save bandwidth/time - Added explicit
mvcommand example showing how to move the downloaded Galileo environment into the COMPASS extension directory - Improved clarity by separating the download step from the directory layout explanation
Assessment: These additions significantly improve the documentation by providing practical examples for common use cases (partial downloads, specific file patterns) and making the workflow more explicit. The code examples are well-structured and use proper shell escaping. ✅ No concerns with these changes.
|
can you update the PR title and description to describe the changes? |
Description
Issue with the folder structure when unzipping and copying compass_usdz.zip:
cp: cannot stat '.../compass_usds/groot_mobility_rl_es_usds/usd': No such file or directory
Actual extracted structure:
COMPASS_assets/
└── groot_mobility_rl_es_usds/
└── usd/
├── office/
└── ...
Solution:
Doc should show the correct path:
groot_mobility_rl_es_usds/usd
And should include the actual shell commands (unzip + mv/cp) rather than just describing what to do.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there