Skip to content

Add function to create equality constraints between MJCF sites#20

Open
vpunithreddy wants to merge 1 commit intomainfrom
add_equality
Open

Add function to create equality constraints between MJCF sites#20
vpunithreddy wants to merge 1 commit intomainfrom
add_equality

Conversation

@vpunithreddy
Copy link
Copy Markdown
Collaborator

added equality contraint for closed chains

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new helper to generate MJCF <equality> constraints between named site pairs (intended for closed-chain modeling), alongside minor formatting cleanup in the existing joint equality helper.

Changes:

  • Added add_equality_constraints_for_sites() to create connect/weld equality constraints from site name pairs.
  • Normalized add_joint_eq() function signature formatting (trailing comma / alignment).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +436 to +440
for site1, site2 in site_pairs:
# Verify both sites exist
site1_elem = mjcf.find(f".//site[@name='{site1}']")
site2_elem = mjcf.find(f".//site[@name='{site2}']")

Comment on lines +452 to +462
elif constraint_type == "weld":
# Weld constraint references bodies
# Find parent bodies of the sites
body1 = None
body2 = None
for body in mjcf.findall(".//body"):
if body.find(f".//site[@name='{site1}']") is not None:
body1 = body.attrib.get("name")
if body.find(f".//site[@name='{site2}']") is not None:
body2 = body.attrib.get("name")

Comment on lines +474 to +476
print(
f"Created {constraint_type} equality constraint between {site1} and {site2}"
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree

Comment on lines +420 to +421
"""
Add equality constraints between pairs of sites in MJCF.
Comment on lines +417 to +419
def add_equality_constraints_for_sites(
mjcf: ET.Element, site_pairs: List[tuple], constraint_type: str = "connect"
) -> ET.Element:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, can we add a small test that adds an equality between sites in a minimal model (like a four bar linkage or something similar made by spherical joints) and check if mujoco parse it correctly?

ET.Element: The modified MJCF file.
"""
# Find or create the equality element
equality = mjcf.find("equality")
joint2: str,
name: str = None,
multiplier: float = 1.0,
offset: float = 0.0,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove changes unrelated to the PR, thanks!

Comment on lines +417 to +419
def add_equality_constraints_for_sites(
mjcf: ET.Element, site_pairs: List[tuple], constraint_type: str = "connect"
) -> ET.Element:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, can we add a small test that adds an equality between sites in a minimal model (like a four bar linkage or something similar made by spherical joints) and check if mujoco parse it correctly?

# Find or create the equality element
equality = mjcf.find("equality")
if equality is None:
equality = ET.SubElement(mjcf, "equality")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use the same style used in add_sites_eq?

@GiulioRomualdi
Copy link
Copy Markdown

Discussing with @traversaro we decided to keep this PR open but at the same time we created a new branch called https://github.com/gbionics/mujoco-urdf-loader/tree/devel-team-hermes where we will do integration test

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.

4 participants