Skip to content

Commit d1e1ca2

Browse files
committed
fix(vendored-trackers): resolve code-review nits
- live-trackers: rename stale user-facing 'FullBodyTrackerPico' strings (log/limp-mode/exception) to the renamed public 'FullBodyTracker'. - schema-tests: add compile-time coverage for the opt-in full_body_compat.hpp aliases so a mistyped ...Pico mapping can no longer ship silently. - tensor-types: route NUM_BODY_JOINTS_PICO through the lazy deprecation __getattr__ (warns, dropped from __all__) like every sibling alias, and migrate its lone consumer (teleop_ros2 test_messages) to NUM_BODY_JOINTS. - helpers: discover pipeline leaf nodes once in get_required_oxr_extensions_from_pipeline instead of traversing twice. - teleop-session tests: correct the misleading LIVE-mode vendor test docstring to describe what it actually verifies (replay-only guard, deferred to enter).
1 parent ad3aeb0 commit d1e1ca2

7 files changed

Lines changed: 75 additions & 16 deletions

File tree

examples/teleop_ros2/python/tests/test_messages.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from isaacteleop.retargeting_engine.tensor_types import (
1818
DLDataType,
1919
NDArrayType,
20-
NUM_BODY_JOINTS_PICO,
20+
NUM_BODY_JOINTS,
2121
NUM_HAND_JOINTS,
2222
ControllerInput,
2323
ControllerInputIndex,
@@ -73,11 +73,11 @@ def _active_controller() -> TensorGroup:
7373

7474
def _active_full_body() -> TensorGroup:
7575
full_body = TensorGroup(FullBodyInput())
76-
positions = np.zeros((NUM_BODY_JOINTS_PICO, 3), dtype=np.float32)
76+
positions = np.zeros((NUM_BODY_JOINTS, 3), dtype=np.float32)
7777
positions[0] = [1.0, 2.0, 3.0]
78-
orientations = np.zeros((NUM_BODY_JOINTS_PICO, 4), dtype=np.float32)
78+
orientations = np.zeros((NUM_BODY_JOINTS, 4), dtype=np.float32)
7979
orientations[:, 3] = 1.0
80-
valid = np.zeros(NUM_BODY_JOINTS_PICO, dtype=np.uint8)
80+
valid = np.zeros(NUM_BODY_JOINTS, dtype=np.uint8)
8181
valid[0] = 1
8282

8383
full_body[FullBodyInputIndex.JOINT_POSITIONS] = positions

src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ LiveFullBodyTrackerPicoImpl::LiveFullBodyTrackerPicoImpl(const OpenXRSessionHand
5858
}
5959
if (!body_tracking_props.supportsBodyTracking)
6060
{
61-
std::cerr << "[FullBodyTrackerPico] Body tracking not supported by this system, running in limp mode"
61+
std::cerr << "[FullBodyTracker] Body tracking not supported by this system, running in limp mode"
6262
<< std::endl;
6363
return;
6464
}
@@ -85,7 +85,7 @@ LiveFullBodyTrackerPicoImpl::LiveFullBodyTrackerPicoImpl(const OpenXRSessionHand
8585
throw std::runtime_error("Failed to create body tracker: " + std::to_string(result));
8686
}
8787

88-
std::cout << "FullBodyTrackerPico initialized (24 joints)" << std::endl;
88+
std::cout << "FullBodyTracker initialized (24 joints)" << std::endl;
8989
}
9090

9191
LiveFullBodyTrackerPicoImpl::~LiveFullBodyTrackerPicoImpl()
@@ -127,7 +127,7 @@ void LiveFullBodyTrackerPicoImpl::update(int64_t monotonic_time_ns)
127127
if (XR_FAILED(result))
128128
{
129129
tracked_.data.reset();
130-
throw std::runtime_error("[FullBodyTrackerPico] xrLocateBodyJointsBD failed: " + std::to_string(result));
130+
throw std::runtime_error("[FullBodyTracker] xrLocateBodyJointsBD failed: " + std::to_string(result));
131131
}
132132

133133
if (!tracked_.data)

src/core/retargeting_engine/python/tensor_types/__init__.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
Generic3AxisPedalInput,
1717
NUM_HAND_JOINTS,
1818
NUM_BODY_JOINTS,
19-
NUM_BODY_JOINTS_PICO,
2019
RobotHandJoints,
2120
)
2221
from .tactile_types import (
@@ -58,7 +57,6 @@
5857
"Generic3AxisPedalInput",
5958
"NUM_HAND_JOINTS",
6059
"NUM_BODY_JOINTS",
61-
"NUM_BODY_JOINTS_PICO",
6260
"RobotHandJoints",
6361
# Tactile / haptic types
6462
"TactileVector",
@@ -84,7 +82,10 @@
8482

8583
# Deprecated re-exports resolved lazily so access emits a DeprecationWarning; kept out
8684
# of __all__ and the eager imports above so importing this module stays quiet.
87-
_DEPRECATED_ALIASES = {"BodyJointPicoIndex": "BodyJointIndex"}
85+
_DEPRECATED_ALIASES = {
86+
"BodyJointPicoIndex": "BodyJointIndex",
87+
"NUM_BODY_JOINTS_PICO": "NUM_BODY_JOINTS",
88+
}
8889

8990

9091
def __getattr__(name: str):

src/core/retargeting_engine/python/tensor_types/standard_types.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
NUM_HAND_JOINTS = 26 # XR_HAND_JOINT_COUNT_EXT from OpenXR
1818
NUM_BODY_JOINTS = 24 # XR_BODY_JOINT_COUNT_BD from XR_BD_body_tracking
1919

20-
# Deprecated alias for NUM_BODY_JOINTS.
21-
NUM_BODY_JOINTS_PICO = NUM_BODY_JOINTS
22-
2320
# ============================================================================
2421
# Hand Tracking Types
2522
# ============================================================================

src/core/schema_tests/cpp/test_full_body.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,62 @@ static_assert(std::is_trivially_copyable_v<core::BodyJoints>, "BodyJoints should
4545
static_assert(sizeof(core::BodyJoints) == 24 * sizeof(core::BodyJointPose),
4646
"BodyJoints should contain exactly 24 BodyJointPose entries");
4747

48+
// =============================================================================
49+
// Compile-time verification of the deprecated "...Pico" back-compat aliases.
50+
//
51+
// full_body_compat.hpp is an opt-in header: nothing else in-tree includes it and
52+
// the generated header does not pull it in, so without this block a mistyped alias
53+
// (e.g. BodyJointPico_HEAD mapped to BodyJoint_NECK) would compile and ship
54+
// silently. Lock every alias to its renamed target here, mirroring the Python
55+
// TestDeprecatedPicoAliases coverage. The references are intentionally to
56+
// [[deprecated]] names, so suppress that diagnostic for this block only.
57+
// =============================================================================
58+
#if defined(__GNUC__) || defined(__clang__)
59+
#pragma GCC diagnostic push
60+
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
61+
#endif
62+
#include <schema/full_body_compat.hpp>
63+
64+
// Type aliases resolve to the renamed generated types.
65+
static_assert(std::is_same_v<core::FullBodyPosePico, core::FullBodyPose>);
66+
static_assert(std::is_same_v<core::FullBodyPosePicoT, core::FullBodyPoseT>);
67+
static_assert(std::is_same_v<core::FullBodyPosePicoTracked, core::FullBodyPoseTracked>);
68+
static_assert(std::is_same_v<core::FullBodyPosePicoTrackedT, core::FullBodyPoseTrackedT>);
69+
static_assert(std::is_same_v<core::FullBodyPosePicoRecord, core::FullBodyPoseRecord>);
70+
static_assert(std::is_same_v<core::FullBodyPosePicoRecordT, core::FullBodyPoseRecordT>);
71+
static_assert(std::is_same_v<core::BodyJointsPico, core::BodyJoints>);
72+
static_assert(std::is_same_v<core::BodyJointPico, core::BodyJoint>);
73+
74+
// Every enumerator alias keeps its renamed target's numeric value.
75+
static_assert(core::BodyJointPico_PELVIS == core::BodyJoint_PELVIS);
76+
static_assert(core::BodyJointPico_LEFT_HIP == core::BodyJoint_LEFT_HIP);
77+
static_assert(core::BodyJointPico_RIGHT_HIP == core::BodyJoint_RIGHT_HIP);
78+
static_assert(core::BodyJointPico_SPINE1 == core::BodyJoint_SPINE1);
79+
static_assert(core::BodyJointPico_LEFT_KNEE == core::BodyJoint_LEFT_KNEE);
80+
static_assert(core::BodyJointPico_RIGHT_KNEE == core::BodyJoint_RIGHT_KNEE);
81+
static_assert(core::BodyJointPico_SPINE2 == core::BodyJoint_SPINE2);
82+
static_assert(core::BodyJointPico_LEFT_ANKLE == core::BodyJoint_LEFT_ANKLE);
83+
static_assert(core::BodyJointPico_RIGHT_ANKLE == core::BodyJoint_RIGHT_ANKLE);
84+
static_assert(core::BodyJointPico_SPINE3 == core::BodyJoint_SPINE3);
85+
static_assert(core::BodyJointPico_LEFT_FOOT == core::BodyJoint_LEFT_FOOT);
86+
static_assert(core::BodyJointPico_RIGHT_FOOT == core::BodyJoint_RIGHT_FOOT);
87+
static_assert(core::BodyJointPico_NECK == core::BodyJoint_NECK);
88+
static_assert(core::BodyJointPico_LEFT_COLLAR == core::BodyJoint_LEFT_COLLAR);
89+
static_assert(core::BodyJointPico_RIGHT_COLLAR == core::BodyJoint_RIGHT_COLLAR);
90+
static_assert(core::BodyJointPico_HEAD == core::BodyJoint_HEAD);
91+
static_assert(core::BodyJointPico_LEFT_SHOULDER == core::BodyJoint_LEFT_SHOULDER);
92+
static_assert(core::BodyJointPico_RIGHT_SHOULDER == core::BodyJoint_RIGHT_SHOULDER);
93+
static_assert(core::BodyJointPico_LEFT_ELBOW == core::BodyJoint_LEFT_ELBOW);
94+
static_assert(core::BodyJointPico_RIGHT_ELBOW == core::BodyJoint_RIGHT_ELBOW);
95+
static_assert(core::BodyJointPico_LEFT_WRIST == core::BodyJoint_LEFT_WRIST);
96+
static_assert(core::BodyJointPico_RIGHT_WRIST == core::BodyJoint_RIGHT_WRIST);
97+
static_assert(core::BodyJointPico_LEFT_HAND == core::BodyJoint_LEFT_HAND);
98+
static_assert(core::BodyJointPico_RIGHT_HAND == core::BodyJoint_RIGHT_HAND);
99+
static_assert(core::BodyJointPico_NUM_JOINTS == core::BodyJoint_NUM_JOINTS);
100+
#if defined(__GNUC__) || defined(__clang__)
101+
#pragma GCC diagnostic pop
102+
#endif
103+
48104
// =============================================================================
49105
// Compile-time verification of BodyJoint enum.
50106
// =============================================================================

src/core/teleop_session_manager/python/helpers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ def get_required_oxr_extensions_from_pipeline(pipeline: Any) -> List[str]:
104104
"""
105105
import isaacteleop.deviceio as deviceio
106106

107-
trackers = _get_trackers_from_pipeline(pipeline)
108107
sources = _get_sources_from_pipeline(pipeline)
108+
trackers = [source.get_tracker() for source in sources]
109109
vendor_config = build_vendor_config_from_sources(sources)
110110
extensions = deviceio.DeviceIOSession.get_required_extensions(
111111
trackers, vendor_config

src/core/teleop_session_manager_tests/python/test_teleop_session.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2696,13 +2696,18 @@ def test_unvendored_source_in_replay_is_allowed(self):
26962696
TeleopSession(config) # get_vendor() is None -> no raise
26972697

26982698
def test_vendored_source_in_live_is_allowed(self):
2699-
"""LIVE mode honors the vendor, so construction must not raise."""
2699+
"""A vendored source does not trip the replay-only guard in LIVE mode.
2700+
2701+
Construction must not raise: vendor validation is a live-session concern
2702+
deferred to session entry (``__enter__`` -> ``VendorConfig``), so unlike
2703+
REPLAY there is no construction-time rejection here.
2704+
"""
27002705
config = TeleopSessionConfig(
27012706
app_name="test",
27022707
pipeline=MockPipeline(leaf_nodes=[self._vendored_head_source()]),
27032708
mode=SessionMode.LIVE,
27042709
)
2705-
TeleopSession(config) # live mode honors the vendor -> no raise
2710+
TeleopSession(config) # replay-only guard does not fire in LIVE -> no raise
27062711

27072712

27082713
class TestReplayModeSessionEnter:

0 commit comments

Comments
 (0)