Skip to content

Tolerate cameras that serve a reduced PTP property set - #68

Open
igor wants to merge 3 commits into
gosku:mainfrom
igor:tolerate-reduced-property-sets
Open

Tolerate cameras that serve a reduced PTP property set#68
igor wants to merge 3 commits into
gosku:mainfrom
igor:tolerate-reduced-property-sets

Conversation

@igor

@igor igor commented Aug 3, 2026

Copy link
Copy Markdown

What

Three changes so Filmcase works on bodies that serve a reduced PTP property set. Found on an X-E5, but the second and third are not specific to it.

Why

Camera identity failed outright. camera_info() already falls back to 0 for FirmwareVersion, which the X-S10 does not serve. The X-E5 declines PROP_BATTERY and USBMode the same way, and those two reads were not guarded, so every read of camera identity raised on that body. This extends the existing fallback to cover them.

Slot reads reported fiction. Per-property reads fall back to "" for the slot name and 0 for the film simulation. A body that cannot reach C1–Cn therefore answers a scan with a complete set of blank slots, which is indistinguishable from a camera whose slots are genuinely empty. On the X-E5, camera_info --slots printed seven slots that do not exist.

The fix asks GetDeviceInfo whether the slot cursor is advertised. When it is definitely absent, the scan is skipped and the command says so. An empty property list is treated as unknown rather than unsupported, so the older bodies that answer GetDeviceInfo with nothing keep exactly the behaviour they have today.

A protocol gap. LibFujiDevice already implements supported_properties(), but PTPDevice did not declare it, so the domain layer could not call it under mypy. Declared, and added to the test fake.

Commits

  1. Declare supported_properties on the PTPDevice protocol
  2. Tolerate cameras that do not serve battery or USB mode
  3. Skip the custom-slot scan on bodies without the slot cursor

Testing

  • tests/unit/domain/camera/test_camera_info_tolerance.py — identity reads with each status property missing, and with all three missing
  • tests/unit/domain/camera/test_supports_custom_slots.py — cursor advertised, absent, and unknown
  • tests/integration/application/camera/test_get_camera_status.py — the scan is skipped when the cursor is absent, still runs when it is advertised, and still runs when support is unknown

mypy is clean on every changed file. The camera test suites pass locally. The full suite needs the PostgreSQL install, which I do not have set up, so I am relying on CI for the rest — hence draft.

Verified against a physical X-E5. I do not have other bodies to test on; the unsupported-cursor path is exercised through the fake.

Not included

I also have local management commands (camera_probe, camera_dump_slot, a CLI push_recipe) built while working this out. They are new surface area rather than fixes, so I have left them out to keep this reviewable. Happy to open a separate PR if they would be useful.

igor added 3 commits August 3, 2026 10:03
LibFujiDevice already implements supported_properties, but the protocol
did not declare it, so callers in the domain layer could not use it
without a type error. Declare it and add it to the test fake.
camera_info() already fell back to 0 for FirmwareVersion, which the X-S10
does not serve. The X-E5 declines PROP_BATTERY and USBMode the same way,
which made every read of camera identity fail on that body. Extend the
existing fallback to those two properties.
Slot reads fall back to "" for the name and 0 for the film simulation
when a property is not served, so scanning a body that cannot reach C1-Cn
returns a full set of empty-looking slots that is indistinguishable from a
camera whose slots are genuinely empty. The X-E5 does not advertise the
cursor and reported seven fictional slots.

Expose whether the cursor is advertised, skip the scan when it is
definitely absent, and say so instead. An empty GetDeviceInfo property
list still means unknown, so bodies that report nothing keep the previous
behaviour.
@igor
igor marked this pull request as ready for review August 3, 2026 09:14
@igor

igor commented Aug 3, 2026

Copy link
Copy Markdown
Author

Marking this ready. Notes on what I was and wasn't able to verify, since I don't have the full install.

I ran the suite on main and on this branch with the same (SQLite, no PostgreSQL) setup:

passed errors
main 933 1483
this branch 946 1483

The 1483 errors are identical on both — every DB-backed test failing at ensure_connection because there's no PostgreSQL here. The +13 are exactly the tests this PR adds. So nothing that can run in this environment regressed, and the new tests pass.

Not verified: the DB-backed tests. None of them touch camera code, but I can't claim they're green.

mypy is clean on every changed file. The X-E5 behaviour was confirmed against the physical body; the unsupported-cursor path is exercised through FakePTPDevice since I only have the one camera.

Happy to adjust anything, including splitting commit 1 out if you'd rather take the protocol declaration separately.

gosku added a commit that referenced this pull request Aug 4, 2026
Restored verbatim from 4fcb6a0^, where it was removed on 2026-03-28 as
discovery tooling that had served its purpose. Its --scan-supported mode
was the only caller of PTPUSBDevice.supported_properties(), which has
been dead code since.

Brought back to investigate the X-E5 report in PR #68. That PR concludes
the body does not advertise the custom-slot cursor, so C1-Cn cannot be
read or written. The evidence available so far cannot separate a rejected
cursor write from a failed slot read, because slot_states() discards the
return code of the cursor write and both cases print identical output.

--scan-supported answers this directly. It dumps the advertised property
list, writes the cursor before reading every vendor property at C1 and
again at C2, and flags codes whose values differ between the two passes.
That reveals what the camera advertises, whether the cursor write is
accepted, and whether any property is slot-scoped under a code we do not
currently know about.

Not intended to merge. This is a branch for gathering evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@gosku gosku left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the PR, the write-up made this easy to dig into.

Before I review it properly I want to confirm the changes are needed. Reading camera info is the least of it: if the X-E5 genuinely has no slot cursor, we can't push recipes to it at all, which matters more than anything in this PR.

I suspect the camera might be in the wrong USB mode: on my X-S10, card reader and tether both refuse properties that RAW CONV serves, the slot cursor among them. Could you check it's set to MENU → CONNECTION SETTING → USB SETTING → USB RAW CONV./BACKUP RESTORE?

I also put a camera_probe command on camera-usb-mode-diagnostics:

git fetch https://github.com/gosku/Filmcase.git camera-usb-mode-diagnostics
git checkout FETCH_HEAD
python manage.py camera_probe

Run it once per mode and it'll show which one advertises the slot cursor.

@gosku

gosku commented Aug 4, 2026

Copy link
Copy Markdown
Owner

The 1483 errors are identical on both — every DB-backed test failing at ensure_connection because there's no PostgreSQL here. The +13 are exactly the tests this PR adds. So nothing that can run in this environment regressed, and the new tests pass.

Not verified: the DB-backed tests. None of them touch camera code, but I can't claim they're green.

tests/pytest_plugin_settings_isolation.py points FILMCASE_ENV_FILE at /dev/null so every setting falls back to its declared default, and DB_ENGINE defaults to PostgreSQL. So the 1483 errors are the suite looking for a PostgreSQL instance that isn't running, not SQLite refusing to serve them.

That's deliberate, so local runs and CI resolve to the same database, and it's why the README puts the full install under "for development".

Easiest fix is real environment variables to take precedence over that plugin:

DB_ENGINE=django.db.backends.sqlite3 DB_NAME=/tmp/test.sqlite3 pytest

That gets you 2409 passing. Seven will still fail regardless: the settings-conflict tests rely on a unique constraint with nulls_distinct, which SQLite doesn't support.

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