feat: dual-camera support (one active at a time, per-channel binding) - #607
Open
hongquanli wants to merge 52 commits into
Open
feat: dual-camera support (one active at a time, per-channel binding)#607hongquanli wants to merge 52 commits into
hongquanli wants to merge 52 commits into
Conversation
…r-camera overrides Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…el.camera Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mera Add AbstractCamera.supports_hardware_trigger() (true iff the camera was constructed with both hw trigger functions wired), and squid/camera/facade.py with ActiveCameraFacade: an AbstractCamera-shaped object that delegates every call to whichever concrete camera is currently active. The facade keeps its own frame-callback registry, so registrations survive a camera switch and frames from the inactive camera are dropped. close() closes every concrete camera. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_camera with per-camera trigger memory Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…write set_active_camera now restores the facade target, active id, and (best effort) the outgoing camera's trigger mode when applying the incoming camera's mode or starting its stream raises, so microscope.camera and GUI listeners never see a half-switched state. remember_trigger_mode_for_active_camera takes _camera_switch_lock so a set_trigger_mode racing a switch cannot record the mode against a mid-transition camera id (RLock keeps the in-switch call safe). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nding Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… testing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…transforms set_pixel_format kept the cached _current_raw_frame, so switching format after the first trigger (reachable from the live pixel-format dropdown) re-served the stale wrongly-shaped array via the np.roll path -- frame.shape disagreed with frame_pixel_format/is_color(). Mirror the invalidation set_binning already does. Also add committed regression coverage for the _process_raw_frame transforms (rotate_and_flip_image, crop_image), which previously had no tests for any format, asserting the (H, W, 3) channel axis and dtype survive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ettings A camera whose get_binning/get_pixel_format raised anything other than AttributeError or RuntimeError (e.g. OSError from a yanked USB camera) took down the whole save: no camera's settings were written and the exception escaped save_all_camera_settings. On a real shutdown that propagates through _cleanup_common and aborts every later step - camera close, Z retract, turret and microcontroller close. Catch Exception in _settings_dict_for so a broken camera is skipped and the healthy ones are still saved, matching the module's fail-safe contract and the per-camera isolation the restore path already has. Also pin two previously untested branches: the single-entry v2 fallback, and 'no camera readable' leaving the existing cache file untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… via UserRole Display decoration for multi-camera systems: live-view dropdowns and multipoint channel lists show a per-camera dot icon and a '<name> — <camera name>' suffix for channels bound to a non-primary camera. Identity stays the bare channel name everywhere: every entry stores it in Qt.UserRole and all readers use data(UserRole) with a text fallback, so saved YAMLs, set-mode-by-name and MCP APIs never see a decorated label. Channels whose camera is declared but unavailable are greyed out (disabled + tooltip) and excluded from the acquisition sequence. Single-camera systems render exactly as before (bare names, no icons). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dropdown reader Review follow-ups for the channel display labeling task: - Disabled live-dropdown entries (camera declared but unavailable) now carry the same tooltip as the greyed list rows. The string is promoted to a public UNAVAILABLE_CAMERA_TOOLTIP in control.channel_sequence and shared, not duplicated. - Unify both live widgets on the 'itemData(i) or itemText(i)' reader idiom and add tests that wire the production activated-lambda, fire it on a decorated entry, and assert the bare channel name (not the decorated label) is what reaches channel lookup — pinning the exact lines where a decorated label could otherwise leak into get_channel_by_name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o named method Round-2 review fix: the previous reader tests exercised a hand-written copy of the reader expression, so reverting either production reader passed the suite. LiveControlWidget's activated lambda is now a named method (_on_mode_dropdown_activated) and the tests borrow the REAL methods — LiveControlWidget._on_mode_dropdown_activated and NapariLiveWidget.select_new_microscope_mode_by_name — onto stubs, fire them on a decorated entry, and assert the bare channel name reaches selection / channel lookup. Verified by temporarily reverting each production reader to the old itemText idiom: the corresponding test fails with ['BF Color — Side Camera'] != ['BF Color']. Also parametrizes the disabled-entry tooltip test over both widgets' copies of _add_mode_item (previously only LiveControlWidget's copy was asserted). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ge refresh bridge The trigger dropdown is now rebuilt from the active camera's capabilities: Hardware is only offered when that camera's trigger line is wired (cameras.yaml hardware_trigger), and the selection resyncs to the mode the LiveController actually holds - so a dropdown call that lost a race with a camera switch can no longer leave the UI claiming a mode the hardware is not in. The repopulation is guarded (blockSignals + is_switching_mode, with the matching early-return added to update_trigger_mode) so it never turns into an MCU trigger-mode command on top of the one set_active_camera already sent. Microscope's camera-change listener is bridged to the GUI thread with a queued QTimer.singleShot; the handler refreshes the live control widget (and the napari live widget when present) and redraws the nav-viewer FOV, since sensor geometry differs per camera. Multi-camera builds get one CameraSettingsWidget per concrete camera - never the facade, because settings are per-camera identity state - each on its own tab named from cameras.yaml, with the primary tab renamed to match. Single camera systems are unchanged: one plain "Camera" tab, Software + Hardware. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…her camera LiveControlWidget builds its trigger dropdown and exposure range from the active camera, then set_microscope_mode on the startup channel can switch to a different one - and the GUI's camera-change listener is not wired until make_connections, so nothing corrected the widget. A startup channel bound to a camera with no trigger line therefore came up offering Hardware. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fferable trigger modes Two defects from review. The camera-change bridge was a silent no-op off the GUI thread. QTimer.singleShot posts to the CALLING thread's event loop, and the switches that matter come from threads that have none: the acquisition worker (MultiPointWorker._select_config -> set_microscope_mode) and the TCP server thread. After a server- or worker-driven switch the dropdown kept offering Hardware for an unwired camera, exposure limits stayed stale and the nav-viewer FOV never redrew. Replaced with a Qt signal on the GUI class: emission is thread-safe and, with the receiver in the GUI thread, Qt queues delivery onto the GUI event loop. The new test drives set_active_camera from a plain threading.Thread and fails (waitUntil timeout) against the old bridge. refresh_trigger_options could silently desync: setCurrentText with an entry the combo does not contain is a no-op, so a held Hardware mode left the dropdown reading Software with no way back from a one-item list. It now clamps to Software, warns, and pulls the LiveController to the same mode so UI and hardware agree. The napari variant clamps and warns for display only - LiveControlWidget always exists and runs first on a camera change, so syncing there too would double-program the MCU. Also fixed the napari test stub, whose own is_switching_mode filter made the "repopulation is not a user choice" assertion vacuous. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p) and unavailable-camera validation Two pure checkers in control/core/multi_point_utils.py: - get_unavailable_camera_channels: selected channels bound to a camera that never opened (LiveController would silently image them on the active camera). - get_camera_geometry_mismatch: cameras whose frames are not interchangeable (size after crop/binning, color-ness, binned pixel size). Zarr stores one uniform array per region, so a mixed selection cannot be written. MultiPointController.run_acquisition raises ValueError on either condition before any acquisition setup — the backstop for entry points with no Start button (TCP control server, scripts). Both multipoint widgets show a warning label under the channel list and disable Start while the conflict exists, so the GUI never reaches the backstop. Single-camera systems never see either. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eck the camera guard at Start Review follow-ups on the multi-camera acquisition guards. 1. Storage bit depth is now part of the geometry check. Two mono cameras of the same size and pixel size still differ as uint8 vs uint16, and a Zarr array's dtype is fixed by the first frame — the second camera's frames would have been silently cast instead of refused. New CameraPixelFormat.storage_bit_depth() buckets the formats (RGB24/RGB32/BAYER_RG8/MONO8 -> uint8, rest -> uint16) and the mismatch message names the dtype. 2. Start now has two explicit vetoes instead of one flag. The guard and the stage's loading-position lock each record their own, and the button is enabled only when neither vetoes; previously clearing a camera conflict re-enabled Start even with the stage at the loading position. Both owners go through the mixin's disable/enable_the_start_aquisition_button. 3. Two silent-conflict paths closed. toggle_acquisition and on_snap_images re-run the guard and show error_dialog, so a live switch to Zarr in Preferences no longer produces a click that does nothing (the controller backstop's ValueError inside a Qt slot is only logged); Preferences apply also re-runs the guard so the label updates live. The warning label now also names channels the channel list drops on its own because their camera never opened (a dropped YAML or a cached sequence), which nothing else reported — those do not veto Start. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…acquisition parameters The warm-up grab that today happens once (inside the disk-space estimate) now runs once per camera the run will use, ending on the first channel's camera, so no camera pays for its slow first frame inside the acquisition. Single-camera runs are unchanged: the only used id is already the active one, so it is still one grab and no switch. "The" sensor pixel size of a run is only the active camera's once channels can sit on different cameras, so acquisition parameters.json and acquisition.yaml now also carry channel_pixel_sizes_um (channel name -> objective factor x that channel's camera's binned pixel size). MultiPointWorker keeps _pixel_size_um and computes the same map alongside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…isit the starting camera last set_active_camera assumes triggering is quiesced (it reprograms the MCU trigger mode), but the live trigger timer is a threading.Timer on its own thread and can send_trigger() through the facade mid-switch — the one-pending-command race from PR #461. The warm-up now stops live first, but only when it is actually going to switch cameras, so the single-camera path is untouched. run_acquisition would otherwise read the stopped live view as "the user was not live" and never resume it, so the stop is handed off to it via _live_stopped_for_warm_up. Also: visit the run's starting camera last (it ends where the run begins, one switch fewer) and catch per camera, so a secondary camera that will not warm up no longer costs the estimate the frame it came for. Documents the warm-up's coverage — GUI Start with saving; not skip-saving, Snap Images, fluidics or headless — on the wrapper and on get_estimated_acquisition_disk_storage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…recording primary-camera guard End-to-end test: a two-channel selection spanning the mono primary and the RGB secondary camera runs a full 1-FOV simulated acquisition to completion under INDIVIDUAL_IMAGES (the one non-Zarr saver that can persist an RGB frame today - the OME-TIFF writer is 2D-grayscale-only), asserting the per-channel camera switch sequence and one correctly-shaped file per channel on disk. Headless test: set_microscope_mode + acquire_image on a camera-2 channel routes the trigger and frame read to camera 2 (the MCP snap path). GUI guard: opening the Tracking or Simple Recording tab on a multi-camera system forces the primary camera, stopping live first (new LiveControlWidget.stop_live keeps the Live button in sync). redraw_fov now no-ops before the first stage-position event instead of raising a logged TypeError when a camera switch happens right after startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite machine_configs/cameras.yaml.example around the dual-camera (one-active-at-a-time) fields — type, hardware_trigger and the per-camera overrides — replacing the old "simultaneous imaging" framing, and note that `type` is now required whenever more than one camera is declared. Add docs/dual-camera.md covering configuration, channel-to-camera binding in the channel editor, the dot + suffix labeling, per-camera trigger behavior and settings tabs, the acquisition path (per-camera warm-up, per-channel pixel sizes), the Zarr mixed-geometry guard, failure handling and the v1 limits. Corrects the design doc's claim that OME-TIFF handles heterogeneous shapes: the writer is 2D-grayscale-only, so mixed mono+color runs must save individual images. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three corrections from the accuracy audit, all documentation wording: - The primary-camera `hardware_trigger: false` + INI Hardware Trigger default combination does not warn and clamp. Startup applies the default mode to the active camera and a camera built without a hw_trigger_fn raises ValueError, aborting the app before the window appears. The widget-level clamp is real but only reachable on a runtime camera switch. - OME-TIFF does not tolerate heterogeneous mono shapes: one stack file is opened per region+FOV with shape and dtype fixed by the first plane, so differently sized mono frames raise "Image dimensions do not match existing OME memmap stack" and differing bit depths are silently astype-cast. RGB remains unsupported outright. Any camera mismatch should use individual images. - A legacy flat camera-settings cache applies to every camera that asks for it, not only the primary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ak, sim auto-WB signature Three findings from the whole-branch final review. 1. The mixed-geometry guard steered users to OME-TIFF, which cannot hold any of the mismatches it rejects: RGB raises NotImplementedError in the OME writer, differing mono Y*X raises mid-run, and differing mono bit depth is silently .astype()'d. Point at individual images instead, and drop the dual-camera doc note that existed only to warn the suggestion was wrong. 2. MultiPointController._live_stopped_for_warm_up was set by the per-camera warm-up and consumed only in run_acquisition, so five abort paths (disk and RAM dialogs on both multipoint widgets, a failed validate, the multi-camera backstop raise) stranded it True. A later run that skips the warm-up (skip-saving, snap, fluidics, TCP) then resumed live it never stopped — illumination on the sample with nobody at the scope. Clear it in start_new_experiment, which runs before the estimate on every Start path, so the warm-up still re-arms it when it legitimately applies. 3. SimulatedCamera.set_auto_white_balance_gains was missing the abstract's `on: bool`, so the per-camera settings tab's Auto WB button raised TypeError inside a Qt slot on the documented RGB simulation path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… disc color The per-camera palette dot only distinguished camera identity; with a mono + color pair the user couldn't tell which channel images in color. The icon now derives from the camera's pixel format (live camera state, registry default_pixel_format fallback for unopened cameras); identity remains carried by the '— <camera name>' suffix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ignature Two ToupTek cameras on one microscope could not be told apart: the driver always opened the first enumerated device, and the existing _open(sn=...) path never actually worked (it computed sn_matches, then fell through to devices[None] -> TypeError). - ToupcamCamera.__init__ now goes through _open_for_config(), which opens by config.serial_number when one is set and keeps the old _open(index=0) behavior when it is not. - _open(sn=...) resolves an identifier in two passes: first the opaque enumeration id (ToupcamDeviceV2.id, what Toupcam_Open takes), then, since the SDK only reports the true serial number for an open device, by opening each remaining device, reading Toupcam.SerialNumber(), and closing it again unless it matches. A device that cannot be opened (already in use) is skipped rather than fatal. Failing to match now lists every device's id and serial so the right string can be copied into the config. - Probe handles are closed on every path, including when capability building fails after a match. - set_auto_white_balance_gains() gained the abstract `on: bool` parameter; without it the settings tab's Auto WB button raised TypeError inside a Qt slot on a color ToupTek. on=True triggers AwbInit() as before; on=False is a logged no-op because the SDK's AWB is one-push, not continuous. Tests mock the vendored SDK module (no hardware needed) and cover index opening, id matching, serial probing, probe cleanup, error messages, and both auto white balance paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A colour ToupTek could not produce a colour frame. The dual-camera feature documents `default_pixel_format: "RGB24"` for the colour camera, but building one raised `ValueError: Unsupported pixel format` and every other colour route was dead too. The RGB work on this branch went into SimulatedCamera, so simulation passed and hardware did not. Verified on an ITR3CMOS26000KPA. - _configure_camera picks the frame format from the configured pixel format instead of hard coding RAW. The SDK only debayers in RGB frame format, so an RGB pixel format could never be configured. - _calculate_strobe_info keys the line length table off the sensor's readout depth rather than the size of the host side pixel. RGB24/RGB32 are 8 bit readouts and RGB48 is a 16 bit one; deriving the depth from the byte size left line_length at 0 and divided by zero. - The frame callback handles RGB frames. It used to reject anything that was not RAW outright, dropping every colour frame. _rgb_image_from_read_buffer strips the SDK's row padding and returns (h, w, 3) - uint8 for RGB24/RGB32, uint16 for RGB48 - and keeps Grey8/Grey16 in RGB mode 2D, which would otherwise have silently produced an (h, w, 2) array. - The byte order is pinned to RGB. The SDK defaults to BGR on Windows, so without this the red and blue channels swap on one platform only. - Buffer sizing uses _row_pitch_bytes, which matches the SDK's documented row pitch. The old call passed bits to _tdib_width_bytes, which multiplies by 24 itself, over-allocating roughly 24x. That helper is now unused and removed. get_available_pixel_formats() is implemented off the sensor's TOUPCAM_FLAG_MONO bit (new is_mono capability) rather than raising. It raised before, so the camera settings tab fell back to a hard coded mono list for every ToupTek: on a colour camera that hid RGB24 and displayed MONO8 while the camera was actually in RGB24 (setCurrentText on an absent entry is a silent no-op), and it offered BAYER_RG8/RG12, which no ToupTek supports - selecting one raised inside a Qt slot. set_pixel_format validates before touching the camera. Applying the frame format switch and then failing on the pixel format left a pair that _get_pixel_size_in_bytes cannot map, which broke every later frame and every exposure change rather than just that call. Tests mock the vendored SDK (no hardware needed) and cover frame format selection, row pitch padding, buffer unpacking for all four formats plus the grey-in-RGB-mode cases, strobe timing for the colour pixel sizes, the sensor-type format lists, and that a rejected pixel format leaves the camera untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The live control panel's exposure and gain edits were connected straight to the primary camera's CameraSettingsWidget, and each of those widgets drives one concrete camera. Editing exposure while a secondary camera was imaging therefore retuned the primary camera and left the active one untouched: the number changed in the UI and was saved to the channel config, but the sensor kept its old exposure until the channel was re-selected (set_microscope_mode applies it through the facade, which does target the active camera). Dispatch both edits through the active camera's settings widget instead. Single-camera systems declare no extra widgets, so they resolve to the same widget as before. Verified against two Toupcams: with the colour camera active, a 123 ms edit now reaches that camera's driver and its SDK handle, and the mono camera keeps 15 ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NapariMultiChannelWidget.updateLayers compared each incoming frame's dtype against a single widget-wide self.dtype. A run that mixes cameras interleaves a mono camera's MONO16 (uint16, HxW) with a colour camera's RGB24 (uint8, HxWx3), so that test was true on every camera switch: initLayers then cleared the whole LayerList and each layer was re-added as its next frame arrived. That churn runs on the GUI thread. At 2084x2084 with five channels it cost 749 ms per FOV (11 ms after this change), which saturates the Qt event loop for long enough that Windows reports the window as "Not Responding"; closing it during a stall kills the process with no traceback. It also meant the display never held more than four of the five channels, because each flip destroyed the others. Compare against the channel's own layer and rebuild only that one, and size each canvas from the frame that feeds it rather than from whichever camera sent the acquisition's first frame. Single-camera runs see one dtype throughout, so the rebuild branch never fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
user_profiles/ is gitignored, so CI starts without one and the app generates a default profile on first use, while a development machine carries local channel state. That state changes test outcomes: a channel bound to a secondary camera decides which camera is active at startup, which decides what the trigger dropdown offers and whether an acquisition needs a camera switch. Four tests failed on a dual-camera machine for that reason alone. The configs are written during tests too - the live-control spinboxes persist through ConfigRepository on every edit - so a run could rewrite a developer's channel configs. Generate a default profile into a temp dir using the app's own ensure_default_configs and point default-path repositories at it, mirroring isolate_ambient_camera_registry. Repositories constructed with an explicit base_path are left alone. Generation failures raise a UsageError naming the fix rather than leaving every test with an empty profile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y path Review follow-up to 8954e96 and 0040970, which each dropped one piece of widget-wide state but left its neighbours in place. Contrast limits (a regression introduced by 8954e96): ContrastManager tracks a single run-wide acquisition_dtype, and keeping layers alive across a dtype switch removed the accidental rescale the old teardown performed - initLayers called scale_contrast_limits() before re-adding every layer. A uint8 RGB layer was left with uint16 limits, which napari renders as essentially black, so the colour channel was invisible for a whole mixed acquisition. Default limits now come from each layer's own dtype (get_limits_for_dtype); a limit the user set still wins. Layer scale: pixel_size_um was computed once at acquisition start from the active camera, so cameras differing in pixel pitch or binning did not overlay. Each frame now carries the pixel size of its own channel's camera, resolved by the emitter from the channel's `camera` binding - not read from the facade in the widget, where the queued connection means the worker may already have switched. Also drops state the rewrite orphaned: image_width/image_height had no readers left, and a discard/add pair on self.channels cancelled out. Documents that layers_initialized is now a latch. Live exposure/gain dispatch: keyed on the channel's own `camera` binding, the key set_microscope_mode treats as authoritative, instead of active_camera_id - which after an acquisition still points at whatever ran last, so editing a channel on camera 1 retuned camera 2. The edit also no longer relies on QDoubleSpinBox.setValue emitting: it emits nothing when the value is unchanged, and that spinbox is not resynced when set_microscope_mode writes the camera directly, so re-entering a displayed value silently left the old exposure on the sensor. The spinbox is synced with signals blocked and the camera driven with its clamped value. Per-camera settings widgets now live in one map keyed by camera id, primary included, so no caller re-derives "extras plus the primary". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-up to d94b203. The generation guards raised pytest.UsageError, and isolate_ambient_user_profiles is autouse, so on a checkout without the gitignored machine_configs/illumination_channel_config.yaml *every* test in the repo errored instead of only the config-dependent ones - 49 errors where the same run now reports 48 passed and one warning. The application merely logs a warning for that condition (ConfigRepository.load_profile swallows the FileNotFoundError), so the harness was strictly more brittle than the app. Generation failure now warns and falls back to the ambient profile, i.e. to how those runs behaved before the fixture existed. The profile is also copied per test rather than shared for the session. The app persists channel settings through ConfigRepository as spinboxes are edited, so a single directory let one widget-driven test decide what every later test read, making outcomes order-dependent; the previous GUI test had to stub update_channel_setting to work around exactly that. The copy goes in a directory of the fixture's own rather than tmp_path, because the repository tests use tmp_path as a ConfigRepository base_path and a profile planted there collides with the ones they build themselves. tests/test_profile_isolation.py covers the leak: one test writes a channel setting, the next asserts it did not carry over. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…type Found while verifying the mixed-camera display on hardware: a contrast setting made on one camera's channel was silently rewritten into the other camera's range. Two causes, both from treating the acquisition dtype as run-wide when it is really a property of each camera - every channel on one camera shares its dtype, and it changes only when that camera's pixel format does. ContrastManager kept one acquisition_dtype and rescaled EVERY stored channel on each dtype change, which on a mixed run fires at every camera switch. Limits are now stored with the dtype they were chosen in and converted only when that channel's own dtype changes, lazily on read; scale_contrast_limits just records the latest dtype, so its four existing callers stay correct without sweeping other cameras' channels. get_scaled_limits converts without re-anchoring the record, for views that render at a different depth. The mosaic then fed the same corruption back in. mosaic_dtype is latched from whichever tile arrives first (the colour camera's uint8 here), and the limits it derives for that view were assigned to the layer, whose contrast event wrote them back into the shared per-channel store unlabelled - so a mono channel's (800, 1600) came back as (3.1, 6.2). That assignment is now made with the event blocked, since the value came from the manager in the first place, and a genuine user drag in the mosaic is recorded with mosaic_dtype. Verified on two Toupcams over two consecutive mixed acquisitions: colour keeps (10, 200) and mono keeps (800, 1600) exactly, where mono previously came back rescaled. tests/control/core/test_contrast_manager.py covers the rule directly, including that a channel's own dtype change still converts its limits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-back
The v1-limits section still said serial-number camera opening was unimplemented
("not yet plumbed through the Toupcam/Hamamatsu/Tucsen drivers"). The Toupcam
driver has resolved serials since 5c419a0/be66789a, and that is exactly what
makes a same-model Toupcam pair - the configuration this document describes -
work at all. Corrected to name the two drivers that do support it (Toupcam, FLIR)
and the ones that still open the first device found, and fixed the same stale
claim in the single-camera section, where the file is ignored *including* its
serial_number.
Also documents what a Toupcam serial_number may contain, since neither form is
obvious: the SDK serial or the opaque enumeration id, why the serial is the one to
prefer (the enumeration id encodes the USB port), why probing logs a benign
"SerialNumber ... Not implemented" for a device the other camera holds open, and
that a failed match lists every camera found with its id and serial - the easiest
way to discover the value for a machine.
Tests for the mosaic half of e92b314, which had none: a channel's stored limits
must survive being displayed in a mosaic latched to another camera's dtype, and a
genuine contrast drag in that view must be recorded with mosaic_dtype so it is
converted rather than read literally later. Each fails with its half of the fix
removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The SDK picks the white balance mode when the camera is opened, and the two
modes are mutually exclusive. Opened plain, ITR3CMOS26000KPA serves Temp/Tint
and answers "not implemented" to the entire RGB gain API this driver is written
against, so every Auto White Balance press raised HRESULTException. Appending
";wb=rgb" to the camId selects RGB gain mode, after which AwbInit and
get/put_WhiteBalanceGain work. Mono cameras have no white balance in either
mode and are still opened plain.
Both open sites go through the new helper. The serial-number probe opens each
device to read its serial and hands the matching handle back to _open for
reuse, so changing only the obvious site would have left a camera matched by
serial - which is how this machine's colour camera is matched - holding a plain
handle and no better off.
Two related driver bugs, both of which only became reachable once white balance
worked at all:
* put_InitWBGain built a c_short array while Toupcam_put_InitWBGain is
declared as taking c_ushort * 3, so ctypes rejected every call with a
TypeError before it reached the DLL.
* set_white_balance_gains forwarded its arguments to an SDK that declares
c_int * 3. AbstractCamera types them as float, so any caller holding
non-integer gains failed.
The fake SDK handle now rejects floats the way ctypes does; without that, a
driver that forwards them keeps passing here and fails on hardware.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The handler called straight into the driver, so any failure escaped to the global excepthook and looked like a crash to the user. Two failures are routine rather than exceptional: the SDK computes the gains from live frames and errors when the camera is stopped, and not every camera model implements white balance at all. Refuse up front when the camera is not streaming, with a message that says so, and report driver errors rather than letting them propagate. The off path needs the same treatment as the on path - it reads the gains back, which fails just as readily on a camera that does not support them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gains are per-camera identity state, so they join binning and pixel format in the existing per-serial cache instead of getting a mechanism of their own. They are read inside their own try: a mono camera raises on white balance, and that must not cost us the binning and pixel format already read successfully. Cache entries written before this simply load as None. The restore reports each value only when it actually applied. Printing the cached value regardless read as success even when the camera rejected it, which is exactly how a failing restore went unnoticed - and a failed restore of gains we did cache is a warning rather than a debug line, because the save on close then overwrites them with whatever the camera happens to hold. Tests also stop writing to the machine's cache/camera_settings.yaml. The GUI close path saves through it, so any test that built and closed a GUI replaced the developer's real per-camera settings with the test's simulated cameras - the same class of leak as isolate_ambient_user_profiles, for the one piece of ambient state that had not been covered. The fixture wraps the module functions rather than patching _DEFAULT_CACHE_PATH: that constant is bound as a default argument value at import time, so rebinding it would not change where any existing call goes. Calls passing an explicit cache_path are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tube_lens_f_mm describes the objectives actually fitted, not a property of the software: Nikon objectives are designed for a 200mm tube lens and Olympus for 180mm, and the effective magnification is the nominal one scaled by installed/design. objective_and_sample_formats/objectives.csv is checked in and shared by every machine, so a system running Nikon objectives on an Olympus tube lens had nowhere to say so - editing the shared file would push one machine's optics onto all of them. Sample formats already resolve cache first and fall back to the checked-in default; objectives now do the same, in the same function. cache/ is gitignored, so machine-specific optics stay local. Getting this wrong is quiet rather than loud: everything derived from pixel size - scan grid spacing, stitching, scale bars, click-to-move distances, recorded metadata - is off by the ratio, 11% for Nikon on an Olympus tube lens, with nothing to indicate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A click arrives as a displacement from the centre of the *displayed* image, which rotate_and_flip_image has already rotated and flipped, while the stage moves along sensor axes. The handler passed those display coordinates straight to move_x/move_y, so on a rotated view the stage set off along the wrong axis entirely - a 90 degree view turns a horizontal click into vertical travel. display_to_sensor_displacement() inverts that transform and lives beside the forward one, so the two stay together. It takes a displacement rather than a point: both operations are about the image centre, so a centre-relative vector needs only the linear part and no translation, which is why it needs no image size. Exposed as a camera method and delegated by the facade rather than read off a config in the GUI. The facade deliberately holds no _config, so reaching through it would raise AttributeError on a multi-camera system - and the rotation and flip are per-camera anyway, since two cameras can be mounted differently, so the correction has to come from whichever one produced the image. Verified by round-tripping against the real transform - mark a pixel, run the frame through the display path, measure where it landed, invert - for all four rotations against all four flips, rather than re-deriving the algebra in the test and hoping both derivations agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mosaic overview is one OME-TIFF with a plane per channel, (C, Y, X). A colour layer is (H, W, 3) and cannot be a member of that stack, so colour was skipped from the save with a warning - once per timepoint during an acquisition, since save_for_timepoint goes through the same writer. On a machine whose second camera runs RGB24 that meant the colour channel was simply absent from every overview. Colour layers are now written one PNG each, alongside the mono TIFF, for both the whole view and the per-well crops. PNG cannot carry a pixel size, so theirs is recorded only in the YAML sidecar next to the TIFF's embedded OME metadata; a consumer needs the sidecar to scale them. Also fixes a colour-only acquisition saving nothing whatsoever: with no mono layers to stack, the save bailed out early and no overview was produced at all. squid holds colour frames as RGB and cv2 writes BGR, so the write converts, as the tracking and streaming save paths already do. A test pins an exact pixel round-trip - getting this wrong swaps red and blue and reports no error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he stage" This reverts commit 1471576. The premise was wrong: it assumed the stage axes line up with the raw sensor, so a click read off a rotated display had to be un-rotated before driving the stage. On a machine that needs rotate_image_angle at all, the opposite is true. The rotation is there precisely because the sensor is mounted rotated, so the setting is what brings sensor and stage into agreement - the displayed image is already in the stage frame and a click on it needs no correction. Applying the inverse rotated it back out of alignment, and the stage travelled 90 degrees off. Removing it again centres the clicked feature, confirmed on the dual-camera rig with rotate=90 and flip=Both. The transform itself was correct - a genuine inverse of rotate_and_flip_image, pinned by a round-trip test over all four rotations and four flips. That is the point worth remembering: the maths was verified and the tests passed, and it was still the wrong thing to do, because no test of the transform can check which frame the stage actually moves in. Before reinstating anything like this, measure it: move the stage a known distance and see which way image content shifts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts the four PyQt6 compatibility commits in a single commit, restoring the PyQt5 pin at the entry points: - 7cd5701 fix(qt6): replace removed QDesktopWidget with QScreen API - b9634c8 fix(qt6): use binding-agnostic matplotlib backend_qtagg - eaca176 feat(qt): entry points select Qt binding via squid.qt_binding - c6130cc feat(qt): add binding selector preferring PyQt6 when installed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l cameras PVCAM has no open-by-serial, so cameras.yaml gains an optional per-camera device_index (0-based PVCAM enumeration index) that PhotometricsCamera now honors, making a dual-Photometrics pair distinguishable. init_pvcam/ uninit_pvcam are process-global, so they are now refcounted: the library stays initialized until the last open camera closes, and a failed open releases its hold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
get_camera used to catch the vendor-SDK ImportError and silently substitute the Daheng DefaultCamera, so a machine without (say) PVCAM installed reported confusing Daheng open errors — or silently imaged on the wrong camera. Raise a CameraError naming the camera type and the missing module instead. In multi-camera builds a secondary then gets cleanly marked unavailable and a failed primary stops startup with the real cause. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er type The primary camera's settings widget was gated on the global INI CAMERA_TYPE with the dead string "Kinetix" (real Photometrics INIs say camera_type = Photometrics), so Kinetix machines never saw the temperature controls, and the dual-camera secondary tabs hardcoded them off. Each tab now derives its options from its own camera's driver type: temperature controls for Toupcam/ Tucsen/Photometrics, the historical auto-WB pairing preserved (secondaries keep the WB button; the widget already shows it only for color formats). Photometrics raises NotImplementedError for the temperature reading callback (setpoint works, no live readout), so the widget's guard now catches that alongside AttributeError and leaves the measured label blank. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drivers apply config.default_roi at init, but every camera inherited the single INI [CAMERA_CONFIG] ROI, which is tuned for the primary sensor. A CameraDefinition can now carry its own default_roi ([offset_x, offset_y, width, height]), so e.g. a Kinetix secondary gets its 25mm-FOV crop instead of the primary's ROI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Dual-camera support where only one camera is active at a time (e.g. a monochrome fluorescence camera + a color brightfield camera behind a beam splitter). Channels bind to cameras; live view and acquisitions switch automatically. The second camera runs software-trigger-only (no hardware trigger wiring required).
Distinct from the simultaneous multi-camera efforts — this is camera switching, built so single-camera systems are completely unaffected.
How it works
machine_configs/cameras.yaml— each entry gainstype(driver),hardware_trigger: true/false, and optional per-camera overrides (default_pixel_format, crop, rotate/flip, binning). Cameraid: 1is the primary. Nocameras.yaml(or a single-camera one) → exactly today's behavior, no facade installed.microscope.camerabecomes anActiveCameraFacadedelegating to the active concrete camera, so the ~15 components that cache a camera reference work unchanged.Microscope.set_active_camera()centralizes switching: per-camera trigger-mode memory, MCU trigger reprogramming, streaming handover, rollback on failure, and listener notification (delivered cross-thread to the GUI via a Qt signal).AcquisitionChannel.camera(int id,null= primary) is now honored at runtime —LiveController.set_microscope_modeswitches cameras before applying exposure/gain, which gives live view,MultiPointWorker, contrast AF, and the TCP/MCP server the behavior through one seam. (Also fixes the channel editor storing the camera name string into the int field.)docs/dual-camera.mddocuments all format constraints).Docs
software/docs/dual-camera.md(setup, channel binding, trigger behavior, format constraints, migration notes) + updatedcameras.yaml.example.Test plan
tests/squid(177), deterministic multipoint subset, firmware-sim whitelist, GUI construction incl. a cross-thread camera-change bridge testFollow-up (non-blocking) items are triaged in
AI-docs/Squid/to-do/2026-08-09-dual-camera-followups.md.🤖 Generated with Claude Code