Add APIs to Isaac Lab Mimic for supporting loco-manipulation datagen

[peterd-NV]

Description

This PR adds new APIs to Isaac Lab Mimic to expand support for loco-manipulation data generation.

It adds:

• Processing for body end effector to treat a robot's base as an eef during Mimic data generation. This enables the use of the same Mimic annotation and subtask interface to enable lower body movement.
• An optional way to cleanly add custom recorders for Mimic data generation (useful for when users want to record beyond the action/state data provided by Isaac Lab's default recorder).
• Interface for enabling a navigation p-controller during Mimic data generation for robot's with mobile bases.

Type of change

• New feature (non-breaking change which adds functionality)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds support for loco-manipulation data generation in Isaac Lab Mimic by introducing three key features:

• Body End Effector Processing: Treats the robot's base as an end effector (named "body") to enable lower-body movement control during data generation
• Navigation P-Controller Integration: Adds use_navigation_p_controller flag and get_navigation_state() API to manage mobile base navigation, with logic to repeat/skip trajectory steps based on navigation state
• Custom Recorder Configuration: Allows users to provide optional custom recorders via mimic_recorder_config for recording additional data beyond default action/state

Key Issues Found:

• Critical timing bug in data_generator.py lines 911-918: The step index is incremented at line 898 before checking if it's at the last step at line 912. This means the "repeat last action" logic triggers one step early (at len-2 instead of len-1), causing incorrect synchronization between navigation state and trajectory execution.
• Performance inefficiency where "body" validation runs inside nested loops instead of once at initialization.

Confidence Score: 2/5

• This PR has a critical logic bug that will cause incorrect behavior during navigation-controlled data generation
• The step index increment timing bug in the navigation controller logic will cause the trajectory synchronization to be off by one step, potentially leading to premature or delayed navigation transitions. While the feature additions are well-structured (clean API design, proper config additions, good documentation), the core navigation logic flaw makes this unsafe to merge without correction.
• Pay close attention to source/isaaclab_mimic/isaaclab_mimic/datagen/data_generator.py - the navigation controller logic needs the step index check timing fixed

Important Files Changed

File Analysis

Filename	Score	Overview
source/isaaclab_mimic/isaaclab_mimic/datagen/data_generator.py	2/5	Added navigation p-controller support with body end effector processing, but has critical timing bug where step index check happens after increment
source/isaaclab_mimic/isaaclab_mimic/datagen/generation.py	5/5	Added exception handling with traceback and optional custom recorder config parameter - clean implementation
source/isaaclab/isaaclab/envs/manager_based_rl_mimic_env.py	5/5	Added get_navigation_state API method stub with clear documentation - implementation left to subclasses as intended

Sequence Diagram

sequenceDiagram
    participant Env as ManagerBasedRLMimicEnv
    participant DG as DataGenerator
    participant Nav as Navigation P-Controller
    participant Body as Body End Effector
    
    Note over DG: use_navigation_p_controller enabled
    
    DG->>DG: Initialize was_navigating = False
    
    loop Each Timestep
        DG->>Env: get_navigation_state(env_id)
        Env-->>DG: {is_navigating, navigation_goal_reached}
        
        loop For Each End Effector
            DG->>DG: Increment current_eef_subtask_step_indices[eef_name]
            
            alt Body at last step AND navigating AND not goal reached
                DG->>DG: Decrement all step indices (repeat last action)
                Note over DG,Body: Keep robot moving until waypoint reached
            else Was navigating AND stopped AND not processed
                DG->>DG: Skip to end of nav subtask
                Note over DG,Body: Goal reached early, skip remaining trajectory
            end
            
            DG->>DG: Check if subtask complete
        end
        
        DG->>DG: Update was_navigating = is_navigating
    end

Loading

_{8 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[xyao-nv]

Can you make it as optional, ideally both in doc and code? It will confuse users about why navigation appears in this mimic_env.py. At least at code level, it shall specify it's not needed for tabletop manip. You can define a decorator @optional_method.

def optional_method(func):
    """
    Decorator to mark a method as optional for subclasses.
    """
    func.__optional__ = True
    return func

class Base:
    @optional_method
    def get_navigation_state(self):
        print("Base implementation")

[xyao-nv]

If possible, considering what else needed to go with any feedback controller(i.e. P vs PD vs PID). It sounds to me this Mimic eef interface shall be agnostic to what controller it is.

[xyao-nv]

Can you add assertion (navigation_state is not None && keys in the dict) before accessing the keys here?