Skip to content

Allow reusing an existing PTP session - #29

Merged
vdavid merged 3 commits into
vdavid:mainfrom
worm-emoji:reuse-existing-session
Aug 28, 2026
Merged

vdavid merged 3 commits into
vdavid:mainfrom
worm-emoji:reuse-existing-session

Conversation

@worm-emoji

@worm-emoji worm-emoji commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Teenage Engineering TP-7 firmware treats CloseSession as a request to leave MTP mode. Its companion workflow instead uses a stable session ID across host processes and continues when OpenSession reports SessionAlreadyOpen.

The current mtp-rs recovery path closes and reopens any existing session. That makes one-process-per-command TP-7 clients knock the recorder out of MTP mode after ordinary file operations. This behavior is needed by the TP-7 CLI work in totocaster/tp7#4.

Changes

  • add PtpSession::open_reusing_existing for the low-level opt-in
  • add MtpDeviceBuilder::reuse_existing_session(id) as the single high-level setting for a stable session ID plus reuse
  • keep the existing close-and-reopen behavior as the default
  • document that the setting is USB-only and that explicit MtpDevice::close() still sends CloseSession
  • emit a diagnostic when Windows selects WPD and ignores the USB session setting
  • document the TP-7 quirk, API, and hardware evidence in AGENTS.md
  • add a mock regression test proving the reuse path sends no CloseSession command
  • add an end-to-end virtual-transport test proving the builder threads the setting into the open path

No TP-7 identifiers or automatic device quirks are added to mtp-rs.

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --all-targets --all-features --quiet -- -D warnings
  • cargo test --quiet -- --skip fs_watcher (460 passed; 26 expected hardware/platform tests ignored; 6 watcher tests filtered into the separate pass)
  • cargo test --all-features --quiet -- --skip fs_watcher (471 passed; 26 expected hardware/platform tests ignored; 6 watcher tests filtered into the separate pass)
  • cargo test --all-features --quiet fs_watcher -- --test-threads=1 (6 passed outside the sandbox)
  • cargo doc --no-deps --quiet

Hardware A/B

Hardware-verified on a TP-7 running device firmware 2.5.7 and MTP firmware 1.1.11 over macOS/nusb on 2026-08-27, with the documented ptpcamerad blocker active.

The control and patched binaries were built from the same TP-7 CLI commit (503577d). The control differed only by changing reuse_existing_session(true) to reuse_existing_session(false); both used the same stable session ID (0xBAAA_AAAD).

  • Patched tp7 --json status established the recorder in mtp mode.
  • The first control tp7 --json status succeeded.
  • The second independent control process exited 3: TP-7 ... is in audio-midi mode; no MTP-compatible interface is visible.
  • Patched tp7 --json --auto-connect connect reported initial_mode: audio-midi, final_mode: mtp, mtp_ready: true.
  • Two subsequent independent patched tp7 --json status processes both succeeded and reported mode: mtp.

This A/B used only mode switching and read-only status queries. No files or recordings were transferred, changed, or deleted, and no explicit eject/CloseSession was sent after the patched run.

The Rust 1.85 MSRV check is not available locally because this machine has a single Homebrew Rust toolchain and no rustup; the prior PR revision passed the upstream Rust 1.85 CI jobs on Linux, macOS, and Windows.

@vdavid

vdavid commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Hi, @worm-emoji!

Thanks for this, and for keeping it clean. I've checked out the branch, rebased it onto v0.31.0 main, and just passes green. Allowed CI, too.

I also confirmed your diagnosis against the code. Nothing in mtp-rs closes a session on drop, so a TP-7 session really does survive a host process exiting. The CloseSession inside PtpSession::open's SessionAlreadyOpen recovery is the only one a one-process-per-command client ever sends. That's the line you're guarding.

One thing before I can merge: Hardware evidence. Your tp7 PR says the write-path hardware smoke didn't run because macOS wouldn't hand over the IOKit interface. So the central claim isn't verified on a real TP-7 yet. That's a blocker. The cause, btw, is most likely ptpcamerad, a pain in the ass process on macOS that steals PTP/MTP control for auto-mounting cameras: see docs/debugging.md for guidelines for turning it off. Run the blocker command, then do a before/after on the device. The assumption is that the old binary will knock it out of MTP mode, and yours doesn't. Every device claim in this repo has that kind of A/B behind it. I'd do this myself but I don't have a TP-7.

And four more things that either you or I can do:

1. Docs. Add a TP-7 bullet under "Known device quirks" in AGENTS.md, plus a line about the new API. Right now the feature is undiscoverable for anyone reading the docs.

2. Say what MtpDevice::close() does here. The flag only suppresses the CloseSession at open time. An explicit close() still sends one and drops the TP-7 out of MTP mode. Your CLI gets this right with tp7 eject, but the builder's rustdoc should spell it out so nobody pairs reuse with a close() by accident. (I can do this if needed, no biggie.)

3. Windows ignores the setting. Backend::Auto tries WPD first there, and WPD never touches PtpSession, so reuse_existing_session silently does nothing. A "USB backend only" sentence in the rustdoc covers it. A diag_debug! when reuse is set and WPD wins would be even better.

4. Test the wiring. builder_reuses_existing_session only checks a struct field, so nothing proves the builder threads the setting into the open path. The virtual device already answers SessionAlreadyOpen on a second open, so an end-to-end test through the builder is cheap.

Do these if you'd like, otherwise I can. Good for me either way. And don't worry about the rebase or the changelog. Your branch is two commits behind main; it rebases clean and I'll do it on merge. Changelog entries get written at release time.

@worm-emoji

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I pushed 39abdc5 with the four non-hardware follow-ups:

  1. Added the TP-7 quirk and MtpDeviceBuilder::reuse_existing_session(id) entry point to AGENTS.md.
  2. Documented that explicit MtpDevice::close() still sends CloseSession, while drop does not.
  3. Documented the USB-only behavior and added a debug diagnostic when Windows selects WPD and ignores the setting.
  4. Replaced the struct-field test with an end-to-end virtual-transport test: the first builder-configured session is dropped, a second builder-configured open gets SessionAlreadyOpen, reuses the stable ID, and successfully continues with GetDeviceInfo.

Local formatting, strict all-feature Clippy, default and all-feature tests, isolated watcher tests, and docs all pass. I am deliberately holding off on the real TP-7 A/B while the device is busy with an import. I will run the requested before/after once it is free and report the exact commands and observed mode behavior here.

@worm-emoji

Copy link
Copy Markdown
Contributor Author

Hardware A/B complete and pushed in 25ec906.

Setup:

  • TP-7 device firmware 2.5.7 / MTP firmware 1.1.11
  • macOS/nusb with the documented ptpcamerad blocker running
  • both binaries built from TP-7 CLI commit 503577d
  • control changed only reuse_existing_session(true) to reuse_existing_session(false); stable session ID remained 0xBAAA_AAAD

Result:

  1. Patched tp7 --json status succeeded in mtp mode.
  2. First control tp7 --json status succeeded.
  3. Second independent control process exited 3 with TP-7 ... is in audio-midi mode; no MTP-compatible interface is visible.
  4. Patched tp7 --json --auto-connect connect reported initial_mode: audio-midi, final_mode: mtp, and mtp_ready: true.
  5. Two subsequent independent patched tp7 --json status processes both succeeded and reported mode: mtp.

So the default SessionAlreadyOpen recovery reproduces the mode drop across CLI processes, while the reuse path preserves MTP across repeated processes. This was read-only apart from the normal MIDI mode switch: no files or recordings were transferred, changed, or deleted, and no explicit eject/CloseSession was sent after the patched run.

@vdavid
vdavid merged commit adeab07 into vdavid:main Aug 28, 2026
1 check passed
@vdavid

vdavid commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Thanks, was a clear merge now :) Happy building!

vdavid added a commit that referenced this pull request Sep 12, 2026
… every thread since July is recorded

- `README.md` tested devices: the Garmin Forerunner 955 row no longer claims a failing test under investigation. @dasJ confirmed the full read-only suite (11/11) on 2026-05-03 after the split-receive fix; uploads work with the auto-applied split-header mode.
- `docs/notes/community-threads.md`:
  - #6: Garmin marked confirmed (destructive suite never re-run), plus the two 2026-09-11 reports (Pixel 8 Pro from @max619, Galaxy S23+ via Android Bridge from @WildBenji)
  - New entries for #11 (CLI), #20/#22/#23 (Switch responders), #21/#24 (root `SendObjectInfo`), #29 (TP-7 session reuse), and open PR #32
  - #12 moved to closed with its last round and the 17/17 final run
  - Quirk table: dropped the wrong "Android rejects root uploads" row (that was our `0` vs `0xFFFFFFFF` bug) in favor of the real `SEND_ROOT` quirk; added Venu 2/2S, Switch, Sphaira, TP-7, and the Lumix 32-bit partial-read rows; replaced references to the removed `list_objects_recursive_auto` / `is_android()`
  - Contributors: added @dtretyakov, @oenderg, @worm-emoji, @max619, @WildBenji
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.

2 participants