You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add standalone Manus glove calibration worker and browser example - #1406
Adds Stage 1 of Manus glove calibration: a separate calibration worker executable in the existing Manus plugin package, and a standalone browser page that drives it. The worker writes the resulting MCAL files into a plain folder on the host the gloves are attached to, and the page lets the operator download them in the browser. Nothing here touches CaptureServer, the capture protocol, or the application frontend; those are later stages.
manus_glove_calibrator (src/plugins/manus/calibration/): a C++ executable that owns the Manus SDK connection for one attempt, reads the step list from the SDK, runs each step on operator confirmation, exports each glove's calibration straight into the folder it is given, shuts the SDK down, and only then reports a single terminal result. It is not a registered tracking plugin and publishes no device data. The wire format and step engine live in an SDK-free static library so they can be tested without hardware.
Calibration page (examples/manus_calibration/): a plain HTML page served by a small standard-library HTTP + server-sent-events server (no UI dependency). One browser owns an attempt; the others are read-only. Every confirmation names the prompt it is for, so a click on a superseded prompt is refused. The server checks Host and Origin and requires a per-tab client id on every action, because the page can drive hardware and has no login. When a calibration completes, the page shows a download link for each generated file; GET /download/<name> serves only the files the last completed calibration reported, never anything else in the folder.
Supervision (Python): the supervisor bounds the answers only the worker can give (startup through the first prompt, a confirmed step's acknowledgement, the time from the last step to the result) and terminates a silent worker with an uncertain outcome. Operator think time is never bounded. A worker that cannot be reaped keeps new attempts refused.
Files.--out-dir is a plain folder. The supervisor hands it to the worker, which writes each file atomically (<name>.part, then a link to <attempt>_<side>.mcal) and never overwrites an existing one. There is no staging area, manifest or re-verification step. A cancelled, failed or uncertain attempt removes whatever it wrote, so only a completed calibration leaves files.
Behavior to know about
Continuous steps: on the tested gloves every SDK step reports a negative time, which the SDK header describes as a continuous step with an estimated duration. The SDK documents no completion signal for such steps, so the worker does not claim one: confirming starts the SDK step and the next prompt follows, and the estimate is shown to the operator as guidance. Non-finite times are rejected before any step starts.
Shutdown results are checked: a failed SDK release downgrades a completed or cancelled outcome to failed.
The worker's start message names its output directory staging_dir. The name is historical: it is the real output folder, and the README says so.
The page binds to loopback by default. Use SSH port forwarding for remote use.
Known issue
With the real SDK and gloves attached (no working Manus license on the test machine), glove discovery and the step list returned normally, and cancelling at the first prompt ended cleanly. The SDK call that starts a step then did not return within the supervisor's 30 second bound, and the cause was not established (no stack trace was obtainable). So a full calibration from the first step to saved files has not been demonstrated, and neither has loading an exported file through the tracking plugin. This PR does not claim a license failure as the root cause.
Validation
Python tests for the example (protocol, supervisor, controller, HTTP server including downloads, watchdog) against a scripted stand-in for the worker: 381 passed.
SKIP=check-copyright-year pre-commit run --all-files: all hooks passed; the copyright-year hook is skipped as in CI.
C++ unit tests with a fake SDK, the repo CMake wiring and the real-SDK discovery, step list and cancel-at-first-prompt checks passed at an earlier revision; the latest commit changes no C++.
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
Walkthrough
Adds a standalone Manus glove calibration workflow. A C++ worker communicates with a Python supervisor through a versioned JSON protocol. The supervisor manages attempts, verifies and publishes MCAL exports, and serves a browser interface for operator actions and live status. The changes also add build and packaging configuration, usage documentation, and C++ and Python test suites.
sequenceDiagram
participant Browser
participant PageServer
participant CalibrationController
participant CalibrationWorker
participant manus_glove_calibrator
participant CalibrationEngine
participant ManusGloveSdk
participant McalStore
Browser->>PageServer: Send calibration action
PageServer->>CalibrationController: Validate action and update attempt
CalibrationController->>CalibrationWorker: Start or control attempt
CalibrationWorker->>manus_glove_calibrator: Send command over IPC
manus_glove_calibrator->>CalibrationEngine: Handle command and advance attempt
CalibrationEngine->>ManusGloveSdk: Discover gloves and perform calibration
CalibrationEngine-->>CalibrationWorker: Send worker events
CalibrationWorker->>McalStore: Verify and publish completed exports
PageServer-->>Browser: Stream controller view over SSE
Loading
Merge Risk: Low · previous review 6d611
The calibration workflow is isolated in a new example and worker. The remaining issues are narrow edge cases that would end an attempt as uncertain or fail it, plus test-selection hygiene. Fix them before merge or accept them as follow-ups. Full real-glove calibration remains unverified, as the PR states.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 29.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: a standalone Manus glove calibration worker and browser example.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 29.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 294 functions across 37 files. (11 skipped: 11 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
Stage 1 of Manus glove calibration.
- src/plugins/manus/calibration: manus_glove_calibrator, a separate
executable in the Manus plugin package (not a registered tracking
plugin). It owns the SDK connection for one attempt, runs the SDK's
calibration steps on operator confirmation, exports each glove's
calibration, shuts the SDK down, then sends one terminal result over a
line-delimited JSON socket. The wire format and step engine are an
SDK-free static library with a fake-SDK test suite.
- examples/manus_calibration: a plain HTML page served by a standard
library HTTP and server-sent-events server. One browser owns an attempt,
confirmations name the prompt they answer, and Host, Origin and a
per-tab client id are checked. A supervisor bounds answers only the
worker can give, terminates a silent worker with an uncertain outcome,
and refuses new attempts while an unreaped worker may hold the gloves.
Exported files are verified and published through a hard link and an
atomic manifest only after a completed result and clean exit.
- CMake: build and install the worker with the manus component; add its
tests when the SDK is present.
Negative SDK step times (continuous steps with an estimated duration) are
operator-paced and carry the estimate as guidance; the worker claims no
completion signal for them. A failed SDK release downgrades a completed or
cancelled outcome to failed.
Known issue: with the real SDK and no working license, the SDK call that
starts a step did not return within the supervisor's bound; the cause is
not established. An end-to-end calibration, export and tracking load are
not yet demonstrated, and the final revision was not retested.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
… lint
CI rejects unformatted C++; six worker sources were not clang-format 14
clean. Also explain the two intentionally empty exception handlers in the
page server, and make the discarded values in the example's tests explicit.
No behavior change.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Silences the remaining code-quality finding; no behavior change.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
- Parser: check the type of a worker line before the lookup so an unhashable
JSON value is a protocol error, not a TypeError.
- Worker: ignore the rest of a read once the attempt has finished, so no
rejection can follow the terminal result.
- Engine: report a tiny nonzero SDK time as one millisecond on the wire
instead of 0.000, which the supervisor rejects.
- Tests: prefix the Catch2 cases and map their tags to CTest labels, and tag
them [unit] so `ctest -L unit` selects them.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
A failed SDK calibration stop or shutdown can follow any outcome, and the
process then exits while the gloves may still be in use. The terminal result
now always carries `released`; the supervisor treats a result with
released=false as uncertain, publishes nothing, and refuses new attempts
until the program restarts. Process exit alone is not evidence of release.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Drop the store (private staging, size/SHA-256 re-verification, hard-links
to unique names, JSON manifests, list_published). The worker already
writes each export atomically under its final <attempt>_<side>.mcal name
and never overwrites, so the supervisor now hands it the real output
folder and reports the files it wrote. A cancelled, failed or uncertain
attempt removes whatever it wrote.
The example page gets a download link for each generated file; the
server serves only the files the last completed calibration reported.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The reason will be displayed to describe this comment to others. Learn more.
🤖 Agent-generated (Codex · gpt-6)
Summary
Reviewed b9f3a087 against the stated Stage 1 objective, repository guidance, and separate readability, conciseness, and correctness assessments. The SDK-free engine, process supervisor, and presentation/controller boundaries are readable and well scoped. Two reproducible Python behavior defects and a test-integration gap need attention. Full calibration and loading the resulting MCAL files remain unverified, as the PR already discloses.
Legend: 🚫 Blocker · 💡 Suggestion · 🔍 Nit
Finding
🚫
Resolve the validated executable path before launch.examples/manus_calibration/python/isaaccapture_examples/manus_calibration/worker.py:102 — --worker ./manus_glove_calibrator passes the CLI's file/executable checks, but Path strips ./, and create_subprocess_exec then searches PATH for the bare name. I reproduced failed / spawn_failed / No such file or directory with a valid executable in the current directory. If PATH contains a different binary with that name, it can launch that binary instead. Resolve the file to an absolute path before storing/launching it.
💡
Preserve cancellation before the start command.examples/manus_calibration/python/isaaccapture_examples/manus_calibration/worker.py:439 — Cancelling while waiting for ready, before start is sent, terminates the worker. The C++ worker handles SIGTERM in its idle state by emitting result(attempt=null, outcome=cancelled, released=true). This guard rejects that legitimate reply; _decide then prioritizes the violation over the user cancellation. A scripted worker reproducing the C++ response yielded FAILED: worker broke the protocol (Result before the attempt was started). Accept this narrowly defined terminal response during pre-start cancellation while retaining rejection of unexpected pre-start events.
🚫
Register the Python suite in the repository test tree.examples/manus_calibration/pyproject.toml:31 — The new suite lives under examples/manus_calibration/tests/ and has no CTest registration. tests/AGENTS.md explicitly requires new suites under tests/python/examples/<example>/ and wiring through CMake. The standard build jobs run CTest, so these protocol, ownership, watchdog, and HTTP regressions are currently outside that CI path.
💡
Consider dropping checksum metadata without a consumer.src/plugins/manus/calibration/core/engine.cpp:428 — The custom SHA-256 implementation computes a value repeated in exported and result; the supervisor compares those reported values but does not hash the saved file, and the page/download representation omits the checksum. Removing this currently unused metadata would reduce implementation and protocol surface. Optional simplification only; the documented design deliberately has no file re-verification step.
Validation
Ran the Python suite on Python 3.13: 265 passed, 116 failed. The HTTP tests were blocked by this environment's PermissionError binding 127.0.0.1; this does not establish HTTP regressions. Separately reproduced both behavior defects above. C++ builds/tests and real glove calibration were not run here. Checked existing PR discussion; the findings above do not repeat the already addressed parser, timing-precision, terminal-message, or CTest-label comments.
Actionables (for bots — copy-paste-ready for AI)
Agent-generated suggestions, not human-vetted obligations. Skip anything wrong, already addressed, or not worth the churn.
examples/manus_calibration/python/isaaccapture_examples/manus_calibration/worker.py:102 — Store an absolute executable path; add coverage for a valid ./worker absent from PATH and for a same-named PATH executable.
examples/manus_calibration/python/isaaccapture_examples/manus_calibration/worker.py:439 — Recognize the idle worker's clean terminal cancellation response when cancellation preceded start; add a regression exercising the SIGTERM/result exchange.
examples/manus_calibration/pyproject.toml:31 — Move the suite to tests/python/examples/manus_calibration/, use the repository path helper for imports, and register it with CTest without introducing SDK/GPU requirements.
src/plugins/manus/calibration/core/engine.cpp:428 — Optionally remove unused SHA-256 computation and matching protocol fields/validation/tests if no checksum consumer is intended.
Spawn the worker by absolute path so a bare ./name is not re-read as a PATH
lookup, accept an idle worker's clean cancelled result when the user
cancelled before start, and move the Python suite into the repository test
tree with CTest registration.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The reason will be displayed to describe this comment to others. Learn more.
🤖 Automated first-pass review (Claude, for Dejun) — I'll follow up personally where needed.
Summary — Adds manus_glove_calibrator (an SDK-free step engine and wire format plus a thin Manus SDK binding) and a stdlib-only browser page that supervises it and serves the resulting MCAL files. Supervision, ownership and file cleanup are carefully built and widely tested against fakes. One blocker from the earlier review is still open, and the continuous-step pacing needs a hardware check before anyone relies on the files. Neither the scan nor my own read found anything internal (hosts, paths, names), which matters because the repo is public.
Must fix
worker.py:102 — Still open from the earlier review: a relative --worker such as ./manus_glove_calibrator passes _validate, but it is then launched through a PATH search. Make the path absolute once, either here or at __main__.py:74, and add a relative-path test. The fixture only ever passes absolute paths.
Suggestions
engine.cpp:344, app.js:81 — Please verify on real gloves. For a continuous step the page tells the operator to move first and then confirm. So StartStep runs after the movement, and the next prompt replaces the instruction at once. On a glove's last step, finish() (engine.cpp:392) follows start_step() immediately. If StartStep is asynchronous, that step gets no sampling time; the test modelled on the real gloves ends with a 3 s step. If StartStep instead blocks until it sees movement, that could explain the 30 s hang in the Known issue. Before blaming the SDK or the licence, try "confirm, then move for about the estimate", i.e. pace it like a motion step. test_engine.cpp:273 pins the current order.
worker.py:504 — _on_rejected only handles confirm. Suppose --out-dir exists but isn't writable: mkdir(exist_ok=True) passes and the worker rejects start (engine.cpp:108). The page then shows "Starting…" until the startup bound expires (60 s by default) and ends with "Outcome uncertain", never showing the worker's reason. Fail the attempt with the worker's message when start is rejected, and check os.access(out_dir, W_OK | X_OK) before launching. The fake worker never rejects start, so no test covers this.
src/plugins/manus/CMakeLists.txt:109 — calibration/ is added after the SDK gate's return() (line 82). So manus_calibration_core and its 119 Catch2 cases (tests/cpp/plugins/CMakeLists.txt:21) never build in CI, which has no Manus SDK. Add calibration/core before the gate and keep app after it. Make the core Linux-only, because it uses POSIX calls and there is a Windows workflow. No workflow runs the pytest suite either; this is probably the test-integration gap the earlier review mentioned.
worker.py:439 — The earlier review's pre-start-cancel finding is also still open, and the tests can't catch it. On SIGTERM before start, the real worker sends a result with attempt: null (app/main.cpp:243 → engine.cpp:475), but the fake just dies (fake_worker.py:97). Make the fake behave like the worker.
__main__.py:87 — The page handles only SIGINT and SIGTERM, and the worker has no SIGHUP handler either (app/main.cpp:214). If the SSH session the README recommends drops, both processes likely die on SIGHUP. A first glove's <attempt>_left.mcal then stays behind, contrary to example README:92. Handle SIGHUP like SIGTERM on both sides. Better still, have terminate() unlink m_exported when the outcome is not completed, so the guarantee doesn't depend on the page process surviving.
protocol.py:384 — The parser treats an empty instruction as a protocol violation: the worker is killed and the outcome is uncertain. But the worker forwards the SDK description verbatim (manus_sdk.cpp:225), and the contract doesn't forbid an empty one. Either reject it up front in the step check (engine.cpp:258) or accept it, and document which.
server.py:356 — tab.changed.clear() runs after await writer.drain(). A change that arrives while drain() yields (e.g. over a slow tunnel) is lost until the next change or the 15 s keepalive. Clear the event before taking the snapshot.
Stale after the last commit:
export_writer.hpp:29 says the file is renamed into place, but the code uses link().
staging_dir itself (worker README:46) is new in this PR. Renaming it to out_dir costs nothing now, but becomes a compatibility constraint once this PR and #1408 merge.
link() (export_writer.cpp:84) fails on vfat/exFAT. An --out-dir on a USB stick therefore loses the calibration only at export time. Probe for it in directory_usable or note it in the README.
Conflicts
With main: none in the local 3-way merge. The branch is 2 commits behind; no rebase needed.
With open PRs:
#1408 (same author, stacked on this branch) shares 12 example files with no textual conflict. Carry the fixes above (and any staging_dir rename) into it, and rebase it once this merges.
#1153 (huadongzhang97), #1250 (jmex-sw), #1252 (obzhimin, draft) and #1407 (HAOTianGa03) each append a block to the end of tests/cpp/plugins/CMakeLists.txt. That is a one-hunk textual conflict with no semantic overlap; whoever merges second keeps both blocks.
#998, #1321, #1324 and #1340 touch the same file but target other bases, so conflicts were not computed.
Adds SDK-free manus_calibration_core and the manus_glove_calibrator executable (a stub target when the SDK is missing), installed beside manus_hand_plugin
⚠️ the core is only defined when the SDK is found
calibration/core/protocol.*, json.*
Strict NDJSON command parser (flat objects, integers only, no duplicate keys, 64 KiB cap) and event serializer with UTF-8 repair
✅
calibration/core/engine.*
Single-threaded step machine: discovery and glove-ID pinning, per-step prompts and tokens, sampling window for motion steps, per-glove export, one terminal result with released
⚠️ continuous and last-step pacing needs verifying
calibration/core/export_writer.*
.part plus fsync, link() to the final name (never overwrites), directory fsync
⚠️ header comment says rename; needs a filesystem with hard links
calibration/core/sha256.*
Self-contained SHA-256 for the reported hashes
✅ (comment is stale)
calibration/app/main.cpp
CLI; SIGTERM, SIGINT and parent death all trigger cancel; socket line framing; 50 ms poll loop
Executable path — the worker path is now made absolute, so ./manus_glove_calibrator is spawned by path and never re-read as a PATH lookup. Test: a relative worker runs with an empty PATH.
Python suite registration — moved to tests/python/examples/manus_calibration/, imports go through repo_paths, registered with CTest as manus_calibration_* (label manus_calibration, no SDK/GPU needed).
Cancel before start — an idle worker's result(attempt=null, outcome=cancelled) is accepted when the user cancelled before the start command was sent; any other result in that window is still a protocol violation. Test covers both.
Checksum metadata — not changed. It is the optional simplification; the reported checksum stays so the supervisor can compare the exported and result values.
The earlier timing-precision thread is unchanged: wire_seconds() already floors small step times at 1 ms.
End an attempt with the worker's reason when it rejects start, and refuse an
unwritable output folder before launching. Clear the SSE wake event before
the snapshot so a concurrent change is not lost. Correct stale comments about
renaming and hash verification, and note the hard-link and sdk_version
behavior. The fake worker now rejects start and reports an idle cancel like
the real worker.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Addressed the second review's low-risk findings in 8573854 and 31cfb3d:
Rejected start — the supervisor now ends the attempt as FAILED with the worker's reason (start_rejected) instead of waiting out the startup bound, and refuses an existing but unwritable --out-dir before launching. The fake worker can now reject start; tests cover both.
Pre-start cancel — the fake worker now reports a cancelled result with no attempt id on SIGTERM, like the real worker.
SSE wake race — tab.changed is cleared before the snapshot is taken, so a change that lands during drain() wakes the next wait.
Stale text — export_writer.hpp (hard link, not rename; needs a filesystem with hard links), sha256.hpp and protocol.py (no supervisor verification), and "staging directory" in protocol.py.
Nits — sdk_version being always null and the hard-link filesystem requirement are now documented in src/plugins/manus/calibration/README.md.
Still open and being discussed separately: the calibration/core CMake gating, SIGHUP handling, empty instruction, the staging_dir rename, and continuous-step pacing on real gloves.
Add calibration/core before the SDK gate so its Catch2 suite is built in CI,
which has no Manus SDK, on Linux. The SDK-linked worker stays after the gate.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…ename staging_dir to out_dir
A dropped terminal session now cancels the worker and the page instead of
killing them mid-export. The worker forwards the SDK step description
verbatim, so an empty instruction is a valid prompt. The start command's
directory field is named for what it is, the output folder, before any
consumer depends on the old name.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…e carries
The engine reports whole milliseconds as a double, so 0.001 is the expected
value, not the float 0.001f widened. Cover sub-millisecond step times, which
must not round to zero.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
CMake gating — calibration/core is now added before the Manus SDK gate in src/plugins/manus/CMakeLists.txt (Linux only); the SDK-linked worker stays after it. Verified in a clean configure with no Manus SDK present: manus_calibration_core and manus_calibration_tests build, and the Catch2 suite runs under CTest. Doing so exposed two tests that compared a step duration against a widened 0.001f instead of the double 0.001 the wire carries; fixed, and added a sub-millisecond step-time case.
SIGHUP — handled like SIGTERM in both the worker (app/main.cpp) and the page (__main__.py), so a dropped terminal cancels instead of killing either process mid-export. I did not add worker-side unlinking of partial exports; the signal handlers cover the case reported.
Empty instruction — accepted: the worker forwards the SDK text verbatim, so the Python parser no longer rejects an empty one, and the README documents it.
staging_dir → out_dir — renamed on the wire, in the worker, the Python mirror, the README and the tests, before any consumer depends on the old name.
Not changed: continuous-step pacing (engine.cpp / static/app.js). Whether to confirm before or after the movement needs to be checked on real gloves, so I have not reordered it.
…steps
With --continuous-pacing=window, confirming a continuous step starts the SDK
step and the worker then holds it for the SDK's estimate, including the last
step before export, reporting bounded progress. The default keeps the existing
operator-paced order. The supervisor, page and worker take the same policy;
the JSON schema is unchanged. The estimate is a time window, not completion
detection, and the order is to be compared on real gloves before it becomes
the default.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Continuous-step pacing is now implemented as an opt-in policy in c838d2f, so the two orders can be compared on real gloves without a protocol change:
--continuous-pacing=operator (default, unchanged): move, then confirm; the step ends at once.
--continuous-pacing=window: confirm and move; the worker calls StartStep once, then holds the step for the SDK's estimate (including the last step before finish and export) and reports step_progress bounded by estimated_s. The window is a time estimate, never completion detection. A cancel that arrives during the SDK call, or a glove loss during the window, ends the attempt without publishing files.
The supervisor accepts progress up to estimated_s for a continuous step, and the page words the instruction, button and progress bar to match (it shows a neutral "waiting for the SDK" message while the start is pending, so the operator does not have to wait before moving). The JSON schema is unchanged, but the worker, supervisor and page must run with the same policy, so window is selected by one flag on the launcher (--continuous-pacing) that is passed through to the worker.
Tests: window-policy engine cases (hold before next prompt, last step held before export, all-continuous routine with bounded progress, cancel and glove loss inside the window, duplicate confirm), Python cases for both policies, and a fake worker that models the window.
This is still unverified on hardware: whether StartStep returns immediately or blocks, and whether the exported files load in tracking, need real gloves. The default stays as it was until that comparison is done.
On terminate() unlinking partial exports: not added. A dropped terminal is now handled by SIGHUP in both processes, which cancels and cleans up normally; a second place that deletes files in the worker risks removing a good file from a completed attempt, so I left a hard kill of both processes (SIGKILL or a crash) as a known gap.
…mple
The worker supervisor, wire protocol and MCAL storage are now a
standard-library-only package beside the native worker
(src/plugins/manus/calibration/python), reusable by any UI or server. The
browser example keeps only its page, server and view state and depends on the
package. Document that calibration is its own process with its own SDK
adapter and shares no SDK code with the tracking plugin.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The calibration worker is a separate process with its own SDK adapter and no
code shared with the tracking plugin, so it now lives beside it in
src/plugins/manus_calibration rather than inside src/plugins/manus. It still
reuses the vendor SDK's build setup: the imported ManusSDK target is global
and the plugin exports its rpath for the worker.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The must-fix item (a relative `--worker` path launched through a `PATH` search) is fixed on `main` as merged in f0f5fe389: `CalibrationWorker.init` makes the executable absolute (`os.path.abspath`, `worker.py`), and `test_a_relative_worker_path_is_spawned_by_path_not_looked_up_on_PATH` covers a relative path from a changed working directory. I checked the code and the test names; I did not re-run the suite for this comment. The suggestions in that review are not addressed here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
AndreusNvidius
left a comment
NVdejung
left a comment
Summary
Adds Stage 1 of Manus glove calibration: a separate calibration worker executable in the existing Manus plugin package, and a standalone browser page that drives it. The worker writes the resulting MCAL files into a plain folder on the host the gloves are attached to, and the page lets the operator download them in the browser. Nothing here touches CaptureServer, the capture protocol, or the application frontend; those are later stages.
manus_glove_calibrator(src/plugins/manus/calibration/): a C++ executable that owns the Manus SDK connection for one attempt, reads the step list from the SDK, runs each step on operator confirmation, exports each glove's calibration straight into the folder it is given, shuts the SDK down, and only then reports a single terminal result. It is not a registered tracking plugin and publishes no device data. The wire format and step engine live in an SDK-free static library so they can be tested without hardware.examples/manus_calibration/): a plain HTML page served by a small standard-library HTTP + server-sent-events server (no UI dependency). One browser owns an attempt; the others are read-only. Every confirmation names the prompt it is for, so a click on a superseded prompt is refused. The server checks Host and Origin and requires a per-tab client id on every action, because the page can drive hardware and has no login. When a calibration completes, the page shows a download link for each generated file;GET /download/<name>serves only the files the last completed calibration reported, never anything else in the folder.--out-diris a plain folder. The supervisor hands it to the worker, which writes each file atomically (<name>.part, then a link to<attempt>_<side>.mcal) and never overwrites an existing one. There is no staging area, manifest or re-verification step. A cancelled, failed or uncertain attempt removes whatever it wrote, so only a completed calibration leaves files.Behavior to know about
startmessage names its output directorystaging_dir. The name is historical: it is the real output folder, and the README says so.Known issue
With the real SDK and gloves attached (no working Manus license on the test machine), glove discovery and the step list returned normally, and cancelling at the first prompt ended cleanly. The SDK call that starts a step then did not return within the supervisor's 30 second bound, and the cause was not established (no stack trace was obtainable). So a full calibration from the first step to saved files has not been demonstrated, and neither has loading an exported file through the tracking plugin. This PR does not claim a license failure as the root cause.
Validation
SKIP=check-copyright-year pre-commit run --all-files: all hooks passed; the copyright-year hook is skipped as in CI.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests