-
Notifications
You must be signed in to change notification settings - Fork 9
Fix(gizmo): update to new Gizmo API #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -195,8 +195,8 @@ def _create_proxy_cube( | |||||||||||||||||||
| proxy_cube.set_location(position[0], position[1], position[2]) | ||||||||||||||||||||
| proxy_cube.set_rotation_euler(euler[0], euler[1], euler[2]) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Connect gizmo to proxy cube | ||||||||||||||||||||
| self._gizmo.node.update_gizmo_follow(proxy_cube.node) | ||||||||||||||||||||
| # Connect gizmo to proxy cube. | ||||||||||||||||||||
| self._gizmo.follow(proxy_cube.node) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| logger.log_info(f"{name} gizmo proxy created at position: {position}") | ||||||||||||||||||||
| return proxy_cube | ||||||||||||||||||||
|
|
@@ -212,40 +212,55 @@ def _setup_camera_gizmo(self): | |||||||||||||||||||
| self._proxy_cube = self._create_proxy_cube( | ||||||||||||||||||||
| camera_pos, camera_rot_matrix, "Camera" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| self._gizmo.node.set_flush_transform_callback(self._proxy_gizmo_callback) | ||||||||||||||||||||
| # New API uses set_flush_localpose_callback | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| self._gizmo.set_flush_localpose_callback(self._proxy_gizmo_callback) | ||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||
| logger.log_warning(f"Failed to set gizmo callback for camera: {e}") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def _proxy_gizmo_callback(self, *args): | ||||||||||||||||||||
| """Generic callback for proxy-based gizmo. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def _proxy_gizmo_callback(self, node, translation, rotation, flag): | ||||||||||||||||||||
| """Generic callback for proxy-based gizmo: only updates proxy cube transform, defers actual updates""" | ||||||||||||||||||||
| Supports both old signature: (node, translation, rotation, flag) | ||||||||||||||||||||
| and new signature: (node, local_pose, flag) where local_pose is a 4x4 matrix. | ||||||||||||||||||||
| Updates the proxy cube transform and sets `_pending_target_transform`. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| # New API callback signature: (node, local_pose, flag) | ||||||||||||||||||||
| if len(args) != 3: | ||||||||||||||||||||
| return | ||||||||||||||||||||
| node, local_pose, flag = args | ||||||||||||||||||||
| if node is None: | ||||||||||||||||||||
| return | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Check if proxy cube still exists (not destroyed) | ||||||||||||||||||||
| # Check if proxy cube still exists | ||||||||||||||||||||
| if not hasattr(self, "_proxy_cube") or self._proxy_cube is None: | ||||||||||||||||||||
| return | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Update proxy cube transform | ||||||||||||||||||||
| if flag == (TransformMask.TRANSFORM_LOCAL | TransformMask.TRANSFORM_T): | ||||||||||||||||||||
| node.set_translation(translation) | ||||||||||||||||||||
| elif flag == (TransformMask.TRANSFORM_LOCAL | TransformMask.TRANSFORM_R): | ||||||||||||||||||||
| node.set_rotation_rpy(rotation) | ||||||||||||||||||||
| # convert to numpy 4x4 matrix | ||||||||||||||||||||
|
||||||||||||||||||||
| # convert to numpy 4x4 matrix | |
| # convert to numpy 4x4 matrix |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new callback parsing/building logic in _proxy_gizmo_callback is core to camera/robot gizmo control but there are no unit tests covering it (and the repo has an existing pytest suite under tests/). Consider adding a small test that feeds representative (node, local_pose, flag) inputs (torch tensor + numpy array) and asserts _pending_target_transform shape/value behavior, so future Gizmo API changes are caught quickly.
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detach() used to wrap detach_parent() in a try/except, but now calls self._gizmo.detach_parent() unguarded. If detach_parent() can raise during teardown (as implied by the previous code), this can crash callers. Consider restoring the try/except (or a capability check) for robustness consistent with the earlier behavior.
| # Detach gizmo using new API | |
| self._gizmo.detach_parent() | |
| # Detach gizmo using new API, but guard against failures for robustness | |
| try: | |
| detach_fn = getattr(self._gizmo, "detach_parent", None) | |
| if callable(detach_fn): | |
| detach_fn() | |
| except Exception as e: | |
| logger.log_error(f"Error detaching gizmo from parent: {e}") |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set_transform_callback’s docstring still describes a (translation, rotation) style callback, but this PR migrates to a (node, local_pose, flag) callback where local_pose is a 4x4 matrix. Update the docstring (and, if you intend to keep backward compatibility, consider adapting old-style callbacks before registering them).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_proxy_gizmo_callbackdocstring says it supports both the old (node, translation, rotation, flag) and new (node, local_pose, flag) signatures, but the implementation returns early unlesslen(args) == 3. This will silently drop callbacks from the old API. Either handle the 4-argument form (and map it into a 4x4 pose) or update the docstring/usage so the behavior matches the supported API.