Skip to content

feat(recorder): add simple mode for direct recording without HTTP RPC#49

Merged
05F3759DF merged 6 commits intomainfrom
refactor/cli-arguments
Feb 2, 2026
Merged

feat(recorder): add simple mode for direct recording without HTTP RPC#49
05F3759DF merged 6 commits intomainfrom
refactor/cli-arguments

Conversation

@05F3759DF
Copy link
Contributor

@05F3759DF 05F3759DF commented Feb 2, 2026

Pull Request Checklist

Please ensure your PR meets the following requirements:

  • Code follows the style guidelines
  • Tests pass locally (make test or make docker-test)
  • Code is formatted (make format)
  • Documentation updated if needed (CLI usage text updated in print_usage())
  • Commit messages follow conventional commits
  • PR description is complete and clear

Summary

This PR adds a simple mode for direct recording without HTTP RPC server, allowing the recorder to start capturing data immediately via CLI instead of waiting for remote control commands.


Motivation

The HTTP RPC mode is designed for fleet management scenarios where a server controls multiple recorders remotely. However, for local development, testing, and simple recording tasks, starting an HTTP server and sending RPC commands is unnecessarily complex. Simple mode provides a streamlined workflow for these use cases.


Changes

Modified Files

Key Changes

  1. New CLI Options:

    • --simple: Enable simple mode (direct recording without HTTP RPC server)
    • --output FILE: Specify output file path (optional, defaults to auto-generated timestamp)
  2. Removed CLI Options:

    • --topic and --type options removed (topics now configured via config file only)
  3. Simple Mode Behavior:

    • Recorder starts immediately without HTTP RPC server
    • No IDLE → READY → RECORDING state transitions
    • Recording stops on Ctrl+C signal
    • Output filename options:
      • --output my_recording.mcap → Use specified filename
      • No --output → Auto-generated as YYYYMMDD_HHMMSS.mcap
  4. Configuration Changes:

    • Added dataset.output_file field to default config YAML
    • Updated subscriptions to use proper ROS2 message type format (e.g., sensor_msgs/msg/Imu instead of sensor_msgs/Imu)

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update (documentation changes only)
  • Refactoring (code improvement without functional changes)
  • Performance improvement (code changes that improve performance)
  • Test changes (adding, modifying, or removing tests)

Impact Analysis

Breaking Changes

The --topic and --type CLI options have been removed. Users must now configure subscriptions via the config file. Migration:

Before:

./axon_recorder --plugin ./ros2_plugin.so --topic /camera/image_raw --type sensor_msgs/msg/Image

After:

# Create config file with subscriptions
./axon_recorder --simple --plugin ./ros2_plugin.so --config my_config.yaml

Backward Compatibility

HTTP RPC mode remains fully backward compatible. The default behavior (without --simple) is unchanged.


Testing

Test Environment

  • ROS Distribution: ROS 2 Humble / ROS 2 Jazzy
  • OS: Ubuntu 22.04 / Ubuntu 24.04
  • Build Type: Release

Test Cases

  • Unit tests pass locally (82/82 tests passed)
  • Integration tests pass locally
  • E2E tests pass (if applicable)
  • Manual testing completed

Manual Testing Steps

  1. Simple mode with auto-generated filename:

    ./axon_recorder --simple --plugin ./ros2_plugin.so --config config/default_config_ros2.yaml

    Expected: Recording starts immediately, output file named as YYYYMMDD_HHMMSS.mcap

  2. Simple mode with custom output file:

    ./axon_recorder --simple --output /tmp/test.mcap --plugin ./ros2_plugin.so

    Expected: Output file created at /tmp/test.mcap

  3. Simple mode Ctrl+C stop:
    Expected: Recording stops gracefully, final statistics displayed

  4. HTTP RPC mode (backward compatibility):

    ./axon_recorder --config config/default_config_ros2.yaml

    Expected: HTTP server starts, waits for RPC commands (unchanged behavior)

Test Coverage

  • New tests added
  • Existing tests updated
  • N/A - No test changes for this feature

Screenshots / Recordings

N/A


Performance Impact

  • Memory usage: No change
  • CPU usage: No change
  • Recording throughput: No change
  • Lock contention: No change

Documentation

Note: Both README.md (CLI usage examples) and axon_recorder.cpp (print_usage() function) were updated.


Related Issues

  • Fixes #
  • Related to #
  • Refers to #

Additional Notes

The simple mode is primarily intended for:

  • Local development and debugging
  • Quick testing without setting up HTTP server
  • Ad-hoc recording tasks
  • Single-recorder scenarios

For production fleet deployments, HTTP RPC mode remains the recommended approach.


Reviewers

@greptile-apps


Notes for Reviewers


Checklist for Reviewers

  • Code changes are correct and well-implemented
  • Tests are adequate and pass
  • Documentation is updated and accurate
  • No unintended side effects
  • Performance impact is acceptable
  • Backward compatibility maintained (if applicable)

@greptile-apps
Copy link

greptile-apps bot commented Feb 2, 2026

Greptile Overview

Greptile Summary

This PR adds a simple mode (--simple flag) that allows direct recording without HTTP RPC server, streamlining local development and testing workflows.

Key Changes:

  • Added --simple and --output CLI flags for immediate recording
  • Removed --topic and --type CLI flags (breaking change - topics now configured via config file only)
  • Auto-generated timestamp filenames when --output is not specified (format: YYYYMMDD_HHMMSS.mcap)
  • Live statistics display during recording
  • Updated default ROS2 config with proper sensor_msgs/msg/* message types

Issues Found:

  • Critical: dataset.output_file field added to YAML config but not parsed in config_parser.cpp:parse_dataset() - this will cause the field to be silently ignored when loading from config files
  • Thread-safety: std::localtime in timestamp generation is not thread-safe; should use localtime_r (POSIX) or localtime_s (Windows)
  • Style: Magic string comparison != "output.mcap" in output file path resolution is fragile

Architecture:
Simple mode bypasses the HTTP RPC state machine by transitioning directly from IDLE → READY → RECORDING on startup, while HTTP RPC mode preserves the original task-centric workflow.

Confidence Score: 3/5

  • This PR introduces useful functionality but has a critical bug that prevents the dataset.output_file config field from being parsed.
  • Score of 3/5 reflects: (1) missing parser implementation for dataset.output_file field will cause config file settings to be silently ignored, (2) thread-safety issue with std::localtime, and (3) breaking change with removed CLI flags. The core simple mode logic is sound, but these issues need addressing before merge.
  • Pay close attention to apps/axon_recorder/config_parser.cpp (missing implementation) and apps/axon_recorder/axon_recorder.cpp:150 (thread-safety fix needed).

Important Files Changed

Filename Overview
apps/axon_recorder/axon_recorder.cpp Added simple mode with CLI flags, auto-generated timestamps, and live statistics. Thread-safety issue with std::localtime needs fixing.
apps/axon_recorder/recorder.cpp Modified output file path resolution to prioritize custom output files. Minor style issue with magic string comparison.
apps/axon_recorder/config/default_config_ros2.yaml Added dataset.output_file field and updated message types to ROS2 format. Missing parser implementation for new field.

Sequence Diagram

sequenceDiagram
    participant User
    participant Main
    participant AxonRecorder
    participant Plugin
    participant MCAP
    participant WorkerPool

    alt Simple Mode (--simple flag)
        User->>Main: ./axon_recorder --simple --output file.mcap
        Main->>Main: Parse CLI args (simple_mode=true)
        Main->>Main: generate_timestamp_filename() if no --output
        Main->>AxonRecorder: initialize(config)
        AxonRecorder->>WorkerPool: Create worker thread pool
        Main->>AxonRecorder: start()
        AxonRecorder->>AxonRecorder: IDLE → READY transition
        AxonRecorder->>Plugin: load() and init()
        AxonRecorder->>MCAP: open(output_file)
        AxonRecorder->>Plugin: setup_subscriptions()
        AxonRecorder->>AxonRecorder: READY → RECORDING transition
        AxonRecorder->>Plugin: start()
        Main->>Main: Print "Recording started. Press Ctrl+C to stop."
        loop Every 1 second
            Main->>AxonRecorder: get_statistics()
            AxonRecorder-->>Main: Statistics
            Main->>User: Print live stats
        end
        User->>Main: Ctrl+C (SIGINT)
        Main->>AxonRecorder: stop()
        AxonRecorder->>Plugin: stop()
        AxonRecorder->>MCAP: close()
        Main->>User: Print final statistics
    else HTTP RPC Mode (default)
        User->>Main: ./axon_recorder --config config.yaml
        Main->>Main: Parse CLI args (simple_mode=false)
        Main->>AxonRecorder: initialize(config)
        Main->>AxonRecorder: start_http_server()
        AxonRecorder-->>Main: Server listening on port 8080
        Main->>User: "Waiting for RPC commands"
        User->>AxonRecorder: POST /rpc/config
        AxonRecorder->>AxonRecorder: IDLE → READY transition
        User->>AxonRecorder: POST /rpc/begin
        AxonRecorder->>Plugin: load() and start()
        AxonRecorder->>MCAP: open(dataset.path/task_id.mcap)
        AxonRecorder->>AxonRecorder: READY → RECORDING transition
        User->>AxonRecorder: POST /rpc/finish
        AxonRecorder->>Plugin: stop()
        AxonRecorder->>MCAP: close()
        AxonRecorder->>AxonRecorder: RECORDING → IDLE transition
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@05F3759DF 05F3759DF merged commit c022303 into main Feb 2, 2026
23 checks passed
@05F3759DF 05F3759DF deleted the refactor/cli-arguments branch February 2, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant