Skip to content

mm-info, mm2grid, mm2las, mm2ply, mm2txt now have a --load-plugins flag#54

Merged
jlblancoc merged 3 commits intodevelopfrom
feat/mm-apps-plugins
Apr 13, 2026
Merged

mm-info, mm2grid, mm2las, mm2ply, mm2txt now have a --load-plugins flag#54
jlblancoc merged 3 commits intodevelopfrom
feat/mm-apps-plugins

Conversation

@jlblancoc
Copy link
Copy Markdown
Member

@jlblancoc jlblancoc commented Apr 13, 2026

Summary by CodeRabbit

  • New Features

    • Added --load-plugins/-l to multiple CLI tools (mm-info, mm2grid, mm2las, mm2ply, mm2txt, rawlog-filter) to load comma-separated plugin .so files at startup; tools now report the plugin list before processing.
  • Bug Fixes

    • Added missing exception headers to several binaries and tests to ensure proper runtime error reporting when plugin loading or other errors occur.
  • Documentation

    • Updated CLI docs for all affected tools to describe the new plugin-loading option.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

Added a new optional CLI flag -l/--load-plugins to multiple tools that accepts comma-separated .so plugin paths; when set, plugins are loaded via mrpt::system::loadPluginModules() before existing metric-map/rawlog processing, and failure throws std::runtime_error. Several files also gained explicit <stdexcept> includes and a voxel-insertion consolidation.

Changes

Cohort / File(s) Summary
Plugin Loading Implementation
apps/mm-info/main.cpp, apps/mm2grid/main.cpp, apps/mm2las/main.cpp, apps/mm2ply/main.cpp, apps/mm2txt/main.cpp, apps/rawlog-filter/main.cpp
Added -l/--load-plugins CLI option; when set, prints plugin list and calls mrpt::system::loadPluginModules(plugins, errMsg) before map/rawlog processing. On failure throws std::runtime_error. Added #include <mrpt/system/os.h> where required.
Header/exception hygiene
apps/mm-filter/main.cpp, apps/mm-georef/main.cpp, mp2p_icp_filters/src/FilterAbsoluteTimestamp.cpp, mp2p_icp_filters/src/FilterDeskew.cpp, mp2p_icp_filters/src/FilterSOR.cpp, tests/test-mp2p_quality_reproject_ranges.cpp, tests/test-mp2p_quality_voxels.cpp
Added #include <stdexcept> to ensure std::runtime_error is declared where exceptions are thrown. No behavioral changes.
Voxel insertion simplification
mp2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp
Replaced separate find() + operator[] pattern with single pts_voxels.try_emplace(...) call; update existing-key path to use iterator and inserted flag.
Documentation updates
docs/source/app_mm-info.rst, docs/source/app_mm2grid.rst, docs/source/app_mm2las.rst, docs/source/app_mm2ply.rst, docs/source/app_mm2txt.rst
Documented new -l/--load-plugins <file.so> option in usage and arguments; noted plugins load before reading maps. Minor punctuation/formatting edits.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI
    participant App as App (e.g., mm2txt)
    participant Loader as Plugin Loader
    participant MRPT as mrpt::system
    participant Map as MetricMap/Rawlog loader

    CLI->>App: start with --load-plugins=<list>
    App->>Loader: parse plugin list
    Loader->>MRPT: mrpt::system::loadPluginModules(list)
    MRPT-->>Loader: success / failure (errMsg)
    alt failure
        Loader-->>App: throw std::runtime_error(errMsg)
        App-->>CLI: propagate error / exit
    else success
        App->>Map: proceed to load metric map / rawlog
        Map-->>App: loaded data
        App-->>CLI: continue existing processing/export
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰
I nibble bytes and hop on keys,
Plugins loaded on the breeze,
.so friends join the little map,
Hopping code across the gap,
Now our tools can dance with ease!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a --load-plugins flag to five specific CLI utilities (mm-info, mm2grid, mm2las, mm2ply, mm2txt).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/mm-apps-plugins

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.62%. Comparing base (d7ff03d) to head (d0847ea).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #54      +/-   ##
===========================================
- Coverage    78.63%   78.62%   -0.01%     
===========================================
  Files          191      191              
  Lines        10632    10631       -1     
  Branches       982      982              
===========================================
- Hits          8360     8359       -1     
  Misses        2272     2272              
Files with missing lines Coverage Δ
mp2p_icp_filters/src/FilterAbsoluteTimestamp.cpp 79.41% <ø> (ø)
mp2p_icp_filters/src/FilterDeskew.cpp 79.63% <ø> (ø)
mp2p_icp_filters/src/FilterSOR.cpp 94.28% <ø> (ø)
...2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp 91.11% <100.00%> (ø)
tests/test-mp2p_quality_reproject_ranges.cpp 80.85% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jlblancoc
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/mm2grid/main.cpp`:
- Around line 94-104: The new throw site uses std::runtime_error but the
translation units lack the explicit header; add an `#include` <stdexcept> to each
CLI entrypoint that throws std::runtime_error (e.g., the file containing the
throw std::runtime_error(errMsg) in main.cpp and the sibling CLI entrypoints
that also throw runtime_error) so the symbol is declared explicitly and portable
across toolchains.

In `@apps/mm2txt/main.cpp`:
- Around line 71-74: The command-line short option `-l` is duplicated:
`TCLAP::ValueArg<std::string> arg_plugins` currently registers short flag "l"
for `--load-plugins` which collides with the existing `-l` used by `--layer`;
remove or change the short flag for `arg_plugins` (e.g., use no short flag by
passing an empty string "" as the short name or pick an unused short like "p")
so the constructor call for `arg_plugins` no longer uses "l", and update the
documentation in docs/source/app_mm2txt.rst to remove the `-l` from the
`--load-plugins` entry (or reflect whichever short flag you choose).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a654e266-79f0-42bb-896b-1b70aa99e24e

📥 Commits

Reviewing files that changed from the base of the PR and between d7ff03d and c70ea5c.

📒 Files selected for processing (11)
  • apps/mm-info/main.cpp
  • apps/mm2grid/main.cpp
  • apps/mm2las/main.cpp
  • apps/mm2ply/main.cpp
  • apps/mm2txt/main.cpp
  • apps/rawlog-filter/main.cpp
  • docs/source/app_mm-info.rst
  • docs/source/app_mm2grid.rst
  • docs/source/app_mm2las.rst
  • docs/source/app_mm2ply.rst
  • docs/source/app_mm2txt.rst

Comment thread apps/mm2grid/main.cpp
Comment thread apps/mm2txt/main.cpp
@jlblancoc jlblancoc force-pushed the feat/mm-apps-plugins branch from c70ea5c to 1c7ca91 Compare April 13, 2026 19:26
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
mp2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp (1)

78-78: Remove unnecessary const_cast on mapped value.

At Line 78, it->second should already be mutable here; using const_cast adds avoidable risk and obscures intent. Prefer:
auto& vx = it->second;

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mp2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp` at line 78, The mapped
value is being forcibly cast mutable with const_cast on it->second to initialize
vx; remove the const_cast and bind directly with auto& vx = it->second to avoid
undefined behavior and clarify intent, and if the iterator variable it is a
const_iterator update its declaration to a non-const iterator so that it->second
is already mutable (adjust the loop or container access where it is created to
use iterator rather than const_iterator).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@mp2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp`:
- Line 78: The mapped value is being forcibly cast mutable with const_cast on
it->second to initialize vx; remove the const_cast and bind directly with auto&
vx = it->second to avoid undefined behavior and clarify intent, and if the
iterator variable it is a const_iterator update its declaration to a non-const
iterator so that it->second is already mutable (adjust the loop or container
access where it is created to use iterator rather than const_iterator).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a5d18ec-c136-428a-820b-d5fcc50607a8

📥 Commits

Reviewing files that changed from the base of the PR and between c70ea5c and d0847ea.

📒 Files selected for processing (19)
  • apps/mm-filter/main.cpp
  • apps/mm-georef/main.cpp
  • apps/mm-info/main.cpp
  • apps/mm2grid/main.cpp
  • apps/mm2las/main.cpp
  • apps/mm2ply/main.cpp
  • apps/mm2txt/main.cpp
  • apps/rawlog-filter/main.cpp
  • docs/source/app_mm-info.rst
  • docs/source/app_mm2grid.rst
  • docs/source/app_mm2las.rst
  • docs/source/app_mm2ply.rst
  • docs/source/app_mm2txt.rst
  • mp2p_icp_filters/src/FilterAbsoluteTimestamp.cpp
  • mp2p_icp_filters/src/FilterDeskew.cpp
  • mp2p_icp_filters/src/FilterSOR.cpp
  • mp2p_icp_filters/src/PointCloudToVoxelGridSingle.cpp
  • tests/test-mp2p_quality_reproject_ranges.cpp
  • tests/test-mp2p_quality_voxels.cpp
✅ Files skipped from review due to trivial changes (12)
  • tests/test-mp2p_quality_reproject_ranges.cpp
  • apps/mm-filter/main.cpp
  • apps/mm-georef/main.cpp
  • docs/source/app_mm2grid.rst
  • tests/test-mp2p_quality_voxels.cpp
  • mp2p_icp_filters/src/FilterSOR.cpp
  • mp2p_icp_filters/src/FilterAbsoluteTimestamp.cpp
  • docs/source/app_mm2ply.rst
  • docs/source/app_mm2las.rst
  • docs/source/app_mm2txt.rst
  • mp2p_icp_filters/src/FilterDeskew.cpp
  • apps/rawlog-filter/main.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/source/app_mm-info.rst
  • apps/mm2ply/main.cpp
  • apps/mm-info/main.cpp

@jlblancoc jlblancoc merged commit 52058dc into develop Apr 13, 2026
15 checks passed
@jlblancoc jlblancoc deleted the feat/mm-apps-plugins branch April 13, 2026 21:23
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