Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/retargeters/dex_hand_retargeter.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ def __init__(self, config: DexHandRetargeterConfig, name: str) -> None:
config.hand_retargeting_config
).build()

# The optimizer has parsed the YAML; the temp file is no longer needed.
self._cleanup_temp_config()

# Cache joint names from optimizer
self._dof_names = self._dex_hand.optimizer.robot.dof_joint_names

Expand Down Expand Up @@ -177,6 +180,14 @@ def __init__(self, config: DexHandRetargeterConfig, name: str) -> None:
HandJointIndex.LITTLE_TIP,
]

def __del__(self) -> None:
# Safety net for callers that construct a retargeter and abandon it
# before the optimizer build completes (e.g. a config-load exception).
try:
self._cleanup_temp_config()
except Exception:
pass

def input_spec(self) -> RetargeterIOType:
"""Define input collections for hand tracking (Optional)."""
if self._hand_side == "left":
Expand Down Expand Up @@ -302,6 +313,24 @@ def _prepare_configs(self) -> None:

if temp_config:
self._config.hand_retargeting_config = temp_config
# Track for cleanup once the optimizer has consumed the file.
self._temp_config_path: Optional[str] = temp_config
else:
self._temp_config_path = None

def _cleanup_temp_config(self) -> None:
"""Remove the temp YAML written by ``_update_yaml`` once the optimizer
has been built from it. Safe to call repeatedly."""
path = getattr(self, "_temp_config_path", None)
if not path:
return
try:
os.unlink(path)
except FileNotFoundError:
pass
except OSError as e:
logger.warning(f"Failed to remove temp dex_retargeting config {path}: {e}")
self._temp_config_path = None

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

config.hand_retargeting_config is left pointing to a deleted file.

_prepare_configs() overwrites self._config.hand_retargeting_config with the temp YAML path, and _cleanup_temp_config() deletes that file. After construction, the config object now contains a stale path, so reusing the same DexHandRetargeterConfig can fail on the next initialization.

Suggested fix
@@
-        self._dex_hand = RetargetingConfig.load_from_file(
-            config.hand_retargeting_config
-        ).build()
+        self._dex_hand = RetargetingConfig.load_from_file(
+            self._retargeting_config_path
+        ).build()
@@
-        if temp_config:
-            self._config.hand_retargeting_config = temp_config
-            # Track for cleanup once the optimizer has consumed the file.
-            self._temp_config_path: Optional[str] = temp_config
-        else:
-            self._temp_config_path = None
+        # Keep caller-provided config immutable; use an internal load path.
+        self._retargeting_config_path = self._config.hand_retargeting_config
+        if temp_config:
+            self._retargeting_config_path = temp_config
+            # Track for cleanup once the optimizer has consumed the file.
+            self._temp_config_path: Optional[str] = temp_config
+        else:
+            self._temp_config_path = None
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/retargeters/dex_hand_retargeter.py` around lines 314 - 333,
_prepare_configs currently overwrites self._config.hand_retargeting_config with
a temp YAML path and _cleanup_temp_config deletes that file but does not restore
the original config, leaving a stale path; change _prepare_configs to save the
original value (e.g., self._original_hand_retargeting_config) before assigning
self._config.hand_retargeting_config = temp_config, and update
_cleanup_temp_config to restore self._config.hand_retargeting_config from
self._original_hand_retargeting_config (or set it to None if there was no
original), then remove the temp file and clear both self._temp_config_path and
self._original_hand_retargeting_config to avoid stale references.


def _update_yaml(self, yaml_path: str, urdf_path: str) -> Optional[str]:
"""
Expand Down
Loading