Skip to content

Commit 1092ec7

Browse files
committed
fix: handle URDF material name reuse and integer-default mesh scale
Tested the importer against UR5, Franka Panda, ANYmal B, and Allegro Hand URDFs from public ROS packages. Two issues fell out: - URDF allows many visuals to reference one named `<material>`. BB's DSL requires globally-unique entity names, so the generated module failed to compile. Keep the URDF name on the first visual that uses each material; strip it on later occurrences (BB's `material` entity auto-generates an identifier when `name` is absent). - `BB.Dsl.Mesh.scale` defaults to the integer `1`, and the URDF exporter calls `:erlang.float_to_binary/2` on it, which crashes. Always emit `scale` as a float so the resulting module round-trips through `mix bb.to_urdf` without hitting that bug. After these fixes, all four corpus URDFs parse, generate, compile, and round-trip through the exporter with the same link/joint counts as the inputs.
1 parent fe79d67 commit 1092ec7

4 files changed

Lines changed: 100 additions & 5 deletions

File tree

lib/bb/urdf/importer.ex

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ if Code.ensure_loaded?(Sourceror) do
5050
@spec to_quoted(Parser.robot(), module) ::
5151
{:ok, Macro.t(), [String.t()]} | {:error, term}
5252
def to_quoted(robot, module_name) when is_atom(module_name) do
53+
robot = dedupe_material_names(robot)
5354
links_by_name = Map.new(robot.links, &{&1.name, &1})
5455
joints_by_parent = Enum.group_by(robot.joints, & &1.parent)
5556
child_links = MapSet.new(robot.joints, & &1.child)
@@ -75,6 +76,31 @@ if Code.ensure_loaded?(Sourceror) do
7576
end
7677
end
7778

79+
# URDF lets many visuals reference a single named material; BB's DSL
80+
# requires globally-unique entity names. Keep the URDF name on the first
81+
# visual that uses each material and strip it on later occurrences (BB
82+
# auto-generates a unique identifier when `name` is absent).
83+
defp dedupe_material_names(robot) do
84+
{links, _seen} = Enum.map_reduce(robot.links, MapSet.new(), &dedupe_link_material/2)
85+
%{robot | links: links}
86+
end
87+
88+
defp dedupe_link_material(%{visual: %{material: %{name: name}}} = link, seen)
89+
when is_binary(name) do
90+
if MapSet.member?(seen, name) do
91+
{strip_material_name(link), seen}
92+
else
93+
{link, MapSet.put(seen, name)}
94+
end
95+
end
96+
97+
defp dedupe_link_material(link, seen), do: {link, seen}
98+
99+
defp strip_material_name(link) do
100+
visual = %{link.visual | material: %{link.visual.material | name: nil}}
101+
%{link | visual: visual}
102+
end
103+
78104
defp validate_referenced_links(joints, links_by_name) do
79105
Enum.reduce_while(joints, :ok, fn joint, _acc ->
80106
cond do
@@ -186,11 +212,10 @@ if Code.ensure_loaded?(Sourceror) do
186212
end
187213

188214
defp render_geometry({:mesh, %{filename: filename, scale: scale}}) do
189-
children =
190-
[call(:filename, [filename])] ++
191-
if scale == 1.0, do: [], else: [call(:scale, [scale])]
192-
193-
call(:mesh, [], children)
215+
call(:mesh, [], [
216+
call(:filename, [filename]),
217+
call(:scale, [scale * 1.0])
218+
])
194219
end
195220

196221
defp render_material(material) do

test/bb/urdf/importer_test.exs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,26 @@ defmodule BB.Urdf.ImporterTest do
9595
assert Enum.any?(warnings, &(&1 =~ "<transmission>"))
9696
end
9797

98+
test "dedupes named materials so multiple visuals can share a URDF material" do
99+
{source, _warnings} = import_fixture("shared_materials.urdf", MyApp.SharedGen)
100+
101+
# First visual keeps the URDF material name…
102+
assert Regex.match?(
103+
~r/link :base_link do.*?material do\s*name :grey/s,
104+
source
105+
)
106+
107+
# …and the resulting module compiles (would fail if the name leaked
108+
# into later visuals — the DSL rejects duplicate entity names).
109+
assert [{MyApp.SharedGen, _}] = Code.compile_string(source)
110+
end
111+
112+
test "emits mesh scale as a float (the default integer trips the URDF exporter)" do
113+
{source, _warnings} = import_fixture("shared_materials.urdf")
114+
115+
assert source =~ ~r/mesh do\s*filename "package:\/\/meshes\/arm\.stl"\s*scale 1\.0/
116+
end
117+
98118
test "errors out on multiple root links" do
99119
xml = """
100120
<?xml version="1.0"?>
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?xml version="1.0"?>
2+
<robot name="shared_materials_bot">
3+
<material name="grey">
4+
<color rgba="0.5 0.5 0.5 1"/>
5+
</material>
6+
7+
<link name="base_link">
8+
<visual>
9+
<geometry>
10+
<box size="0.1 0.1 0.1"/>
11+
</geometry>
12+
<material name="grey"/>
13+
</visual>
14+
</link>
15+
16+
<link name="arm1">
17+
<visual>
18+
<geometry>
19+
<mesh filename="package://meshes/arm.stl"/>
20+
</geometry>
21+
<material name="grey"/>
22+
</visual>
23+
</link>
24+
25+
<link name="arm2">
26+
<visual>
27+
<geometry>
28+
<cylinder radius="0.02" length="0.1"/>
29+
</geometry>
30+
<material name="grey"/>
31+
</visual>
32+
</link>
33+
34+
<joint name="j1" type="revolute">
35+
<parent link="base_link"/>
36+
<child link="arm1"/>
37+
<axis xyz="0 0 1"/>
38+
<limit lower="-1" upper="1" effort="5" velocity="1"/>
39+
</joint>
40+
41+
<joint name="j2" type="prismatic">
42+
<parent link="arm1"/>
43+
<child link="arm2"/>
44+
<axis xyz="1 0 0"/>
45+
<limit lower="0" upper="0.1" effort="10" velocity="0.5"/>
46+
</joint>
47+
</robot>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
SPDX-FileCopyrightText: 2026 James Harton
2+
3+
SPDX-License-Identifier: Apache-2.0

0 commit comments

Comments
 (0)