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}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds comprehensive support for loco-manipulation data generation to Isaac Lab Mimic by enabling navigation controller integration. The changes introduce three key capabilities:

• Body end effector processing: Treats the robot's base as an end effector, enabling lower body movement during data generation
• Custom recorder interface: Provides optional mimic_recorder_config field allowing users to add custom recorders beyond default action/state data
• Navigation controller integration: Adds use_navigation_controller flag and get_navigation_state() API to manage mobile base navigation during data generation

The implementation handles navigation state transitions by repeating the last navigation subtask action when the robot is still navigating to a waypoint, and skipping forward when the goal is reached early. Error handling was improved in run_data_generator with traceback logging.

Minor issues found:

• Docstring missing new recorder_cfg parameter in setup_env_config
• Validation check for "body" end effector repeated on every loop iteration (inefficient)
• copy.deepcopy used unnecessarily for boolean assignment
• No validation that get_navigation_state is implemented before calling it when flag is enabled

Confidence Score: 4/5

• This PR is safe to merge with minor documentation and performance improvements recommended
• Score of 4 reflects well-structured implementation with clear separation of concerns. The navigation controller logic is properly guarded by flag checks, preventing unintended execution. Issues identified are non-critical: missing docstring parameter, inefficient validation placement, and minor style improvements. No logic errors or security vulnerabilities found - previous review comments about NameError were incorrect as variable scoping is correct.
• Pay attention to source/isaaclab_mimic/isaaclab_mimic/datagen/data_generator.py for the validation and efficiency improvements

Important Files Changed

File Analysis

Filename	Score	Overview
source/isaaclab_mimic/isaaclab_mimic/datagen/data_generator.py	3/5	Added navigation controller support for loco-manipulation data generation - processes body end effector, handles navigation state, and manages trajectory skipping/repeating based on waypoint goals
source/isaaclab_mimic/isaaclab_mimic/datagen/generation.py	4/5	Added optional custom recorder configuration parameter and error handling with traceback to run_data_generator and setup_env_config
source/isaaclab/isaaclab/envs/manager_based_rl_mimic_env.py	4/5	Added optional get_navigation_state method decorated with @optional_method to provide navigation state for loco-manipulation data generation
source/isaaclab/isaaclab/envs/mimic_env_cfg.py	5/5	Added use_navigation_controller flag to DataGenConfig and mimic_recorder_config field to MimicEnvCfg for custom recorder configuration

Sequence Diagram

sequenceDiagram
    participant User
    participant DataGenerator
    participant Env as ManagerBasedRLMimicEnv
    participant NavController as Navigation Controller
    
    User->>DataGenerator: generate() with use_navigation_controller=True
    
    loop For each timestep
        DataGenerator->>DataGenerator: Execute waypoint for all end effectors
        
        alt Navigation controller enabled
            DataGenerator->>Env: get_navigation_state(env_id)
            Env-->>DataGenerator: {is_navigating, navigation_goal_reached}
            
            alt Body at last subtask step & navigating & not reached goal
                DataGenerator->>DataGenerator: Repeat last nav subtask action (decrement indices)
            else Was navigating & now stopped navigating
                DataGenerator->>DataGenerator: Skip to end of nav subtask (advance indices)
            end
        end
        
        DataGenerator->>DataGenerator: Update was_navigating state
        
        alt All subtasks done
            DataGenerator-->>User: Return generated trajectory
        end
    end

Loading

[greptile-apps]

_{8 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: potential KeyError if get_navigation_state doesn't return the expected keys

Suggested change

	navigation_goal_reached = navigation_state["navigation_goal_reached"]
	is_navigating = navigation_state.get("is_navigating", False)

[greptile-apps]

logic: potential KeyError if get_navigation_state doesn't return the expected keys

Suggested change

	navigation_goal_reached = navigation_state.get("navigation_goal_reached", False)

[greptile-apps]

logic: was_navigating is referenced on line 922 but only initialized when use_navigation_p_controller=True, will cause NameError on line 922 branch if the flag changes at runtime

[greptile-apps]

syntax: docstring parameter name mismatch: documented as env_id (singular) but parameter is env_ids (plural)

Suggested change

	env_id: The environment index to get the navigation state for. If None, all envs are considered.
	env_ids: The environment index to get the navigation state for. If None, all envs are considered.

[greptile-apps]

_{8 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: inefficient to validate "body" existence on every iteration of the eef_name loop - move this check outside the loop (after line 897) to run once per timestep instead of once per end effector