fix(models): patch broken upstream RD files for Valkyrie and Fetch#543
Merged
Conversation
Both models already load via robot_descriptions, but the upstream files themselves are broken: Valkyrie's valkyrie_sim.urdf still contains an unexpanded xacro macro call with no definition anywhere in its repo, and Fetch's fetch.urdf uses an undeclared XML namespace prefix (unbound prefix). Neither is fixable upstream (roboschool is archived; the nasa-urdf-robots macro would have to be invented from scratch). Add an optional patch hook to URDF_file/URDF_read/URDFRobot.__init__ that runs on the raw text before xacro/XML processing, and use it in each model to surgically strip the one broken, Gazebo-simulation-only element responsible — neither block has any bearing on kinematics, dynamics, or geometry. Each patch documents exactly which robot_descriptions version and upstream commit it targets, so it can be removed if upstream ever fixes the underlying file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #543 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 137 137
Lines 13690 13446 -244
======================================
+ Misses 13690 13446 -244 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
petercorke
added a commit
that referenced
this pull request
Jul 5, 2026
Valkyrie, Fetch, and KinovaGen3 are now fixed on main; FetchCamera is removed entirely (so its entry would be a harmless no-op, but delete it for hygiene). Only LBR remains genuinely broken.
petercorke
added a commit
that referenced
this pull request
Jul 5, 2026
…547) * test(models): add smoke test that every DH/URDF/ETS model constructs Existing tests are hand-written per model, so a model exported in __all__ can go completely untested -- Valkyrie, Fetch, KinovaGen3, FetchCamera, and LBR all had zero coverage this way, three of them silently broken with nothing to catch it (see tech-debt.md). Iterates each category's own __all__ directly (rather than scanning module __dict__/inspect.getmembers, which would also pick up re-exported base classes like Robot/URDFRobot) and asserts every listed class constructs with no arguments. Known-currently-broken models are tracked in an explicit EXPECTED_FAILURES set, which fails loudly in *both* directions: a newly-broken model fails the test, and a since-fixed "expected failure" that starts passing also fails the test, forcing its entry to be cleaned up rather than silently going stale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(models): trim EXPECTED_FAILURES now that #543/#545/#546 are merged Valkyrie, Fetch, and KinovaGen3 are now fixed on main; FetchCamera is removed entirely (so its entry would be a harmless no-op, but delete it for hygiene). Only LBR remains genuinely broken. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Valkyrie()andFetch()both already load viarobot_descriptions(bare-name path), but the upstream files served by RD are themselves broken: Valkyrie'svalkyrie_sim.urdfcontains an unexpanded xacro macro with no definition anywhere in its own repo, and Fetch'sfetch.urdfuses an undeclared XML namespace prefix (unbound prefix). Neither is fixable upstream —openai/roboschoolis archived (no PRs possible), and the missing Valkyrie macro would have to be invented from scratch with no source of truth.patch: Callable[[str], str]hook toURDF_file/URDF_read/URDFRobot.__init__that runs on the raw file text before xacro/XML processing. Each model defines a small local patch function that surgically strips the one broken, Gazebo-simulation-only element responsible — neither has any bearing on kinematics/dynamics/geometry.robot_descriptionsversion and upstream commit it targets, so it can be confirmed safe to delete if upstream ever fixes the file (the regex would simply stop matching).Test plan
rtb.models.URDF.Valkyrie()and.Fetch()both construct andprint()correctly, matching their docstring runblock examples exactly🤖 Generated with Claude Code