[TRCL-658][feat] SPARC: support for LabCube after spectrograph#3339
Merged
pieleric merged 6 commits intodelmic:masterfrom Mar 17, 2026
Conversation
…fter spectrograph
pieleric
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds support for time-correlator streams when the LabCube is positioned after a spectrograph in a SPARC microscope configuration. The changes enable the time-correlator and photo-detector components to properly handle spectrograph axes (wavelength, grating, slits) in addition to the LabCube axes (density, filter).
Changes:
- Extended metadata updater to support time-correlator and photo-detector roles with spectrograph coupling
- Enhanced time-correlator stream configuration to optionally include spectrograph axes
- Added new photo-detector live display plugin for alignment purposes
- Improved LED protection mechanism in TMCM motor driver with manual override capability
- Refactored stream_bar code for consistency (self._main_data_model → main_data)
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/odemis/odemisd/mdupdater.py | Extended role checking to include time-correlator and photo-detector for spectrograph metadata updates |
| src/odemis/gui/cont/stream_bar.py | Enhanced _getAffectingSpectrograph with default parameter; added spectrograph support to addTimeCorrelator; code consistency refactoring; fixed axes ordering in addMonochromator |
| src/odemis/gui/conf/data.py | Added spectrograph axis configurations (wavelength, grating, slits, filter) for ScannedTemporalSettingsStream |
| src/odemis/driver/tmcm.py | Added protection VA for manual LED protection control; refactored LED protection logic into _switch_led_prot method; improved _expected_do_pos tracking |
| src/odemis/acq/path.py | Added slit-in-big configuration for time-correlator mode |
| plugins/photo_det_live.py | New plugin providing live stream creation for photo-detectors with spectrograph and LabCube axes |
| plugins/spectrum_raw.py | Updated _getAffectingSpectrograph call to use new signature with default parameter |
| plugins/spectrum_arbscor.py | Updated _getAffectingSpectrograph call to use new signature with default parameter |
| plugins/la_spec.py | Updated _getAffectingSpectrograph call to use new signature with default parameter |
| plugins/blanker_spectrum.py | Updated _getAffectingSpectrograph call to use new signature with default parameter |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR supplies an explicit default spectrograph into multiple stream-creation call sites, adds a PhotoDetectorLivePlugin for live SPARC photo-detector streams, extends spectrograph-related stream configuration entries, centralizes LED-protection and digital-output handling in the TMCL driver (including new protection methods and expected DO tracking), adds a 'slit-in-big' mapping to the time-correlator SPARC mode, and broadens metadata updater logic to treat time-correlator and photo-detector roles like monochromators. Sequence Diagram(s)sequenceDiagram
participant Plugin as PhotoDetectorLivePlugin
participant Microscope as Microscope (model)
participant MainApp as OdemisGUIApp
participant Detector as Detector
participant Spectro as Spectrograph resolver
participant Stream as StreamController
participant GUI as Stream View
Plugin->>Microscope: check SPARC & photo-detector presence
alt prerequisites not met
Plugin-->>MainApp: log and abort
else prerequisites met
Plugin->>Plugin: add_photo_det_stream(name, detector)
Plugin->>Detector: read detector properties
Plugin->>Microscope: read main_data, ebeam, opm, scan stage, local VAs
Plugin->>Spectro: _getAffectingSpectrograph(detector, default=main_data.spectrograph)
Spectro-->>Plugin: spectrograph (or default)
Plugin->>Plugin: build axis_map (density, filter, wavelength, grating, iris-in, slit-in, slit-monochromator, ...)
Plugin->>Stream: construct MonochromatorSettingsStream(bound to detector, axis_map)
Stream-->>GUI: add stream to view (hide "visible" button)
Stream-->>Plugin: return StreamController
Plugin-->>MainApp: registration complete
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.Add a Ruff configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/odemis/driver/tmcm.py (1)
1991-2004:⚠️ Potential issue | 🟡 MinorRename unused
lposto avoid lint warnings.🛠️ Proposed fix
- _, hpos, lpos, dur = self._do_axes[channel] + _, hpos, _lpos, dur = self._do_axes[channel]
🤖 Fix all issues with AI agents
In `@plugins/photo_det_live.py`:
- Around line 33-59: Add reST docstrings and return type hints: add a
class-level reST docstring to PhotoDetectorLivePlugin describing purpose and
authorship; add an __init__ docstring in reST and annotate __init__ with ->
None; replace the single-line docstring in add_photo_det_stream with a proper
multi-line reST docstring describing parameters (name, detector), behavior and
return value, and add a return type hint for add_photo_det_stream matching the
actual returned object (the variable stream_cont) — if the concrete type isn't
available, import typing.Any or typing.Optional and annotate accordingly; update
imports if needed.
In `@src/odemis/driver/tmcm.py`:
- Around line 1462-1510: Add a docstring to the _set_protection method
describing its purpose, parameters, and return value (it toggles protection
state, takes protected: bool, returns the new state) and remove the unused lpos
local variable from the _switch_led_prot method where _do_axes is unpacked
(change "do_an, hpos, lpos, dur = self._do_axes[channel]" to only unpack the
used names or use a throwaway name like _ for the unused element). Ensure
references to _refswitch_lock and calls to _switch_led_prot remain unchanged.
In `@src/odemis/odemisd/mdupdater.py`:
- Around line 430-433: The method observeSpectrograph is missing type
annotations and an reST docstring and should be annotated to return
Optional[bool] (it returns False in one branch and implicitly None in others);
add parameter type hints for spectrograph and comp_affected, annotate the nested
function updateOutWLRange if desired, and import Optional from typing. Add a
reST-style docstring for observeSpectrograph describing parameters and the
Optional[bool] return value, and update any signatures that reference
observeSpectrograph/updateOutWLRange to use the new annotations.
tmoerkerken
reviewed
Mar 5, 2026
nandishjpatel
approved these changes
Mar 6, 2026
pieleric
force-pushed
the
feat-sparc-support-for-time-correlator-stream-with-labcube-after-spectrograph
branch
from
3c5bae0 to
2cec4de
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/odemis/driver/tmcm.py (1)
2000-2000: Prefix unusedlposvariable with underscore.Static analysis flags
lposas unused at this location as well. Prefix it with an underscore for consistency.🔧 Suggested fix
- _, hpos, lpos, dur = self._do_axes[channel] + _, hpos, _lpos, dur = self._do_axes[channel]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/odemis/driver/tmcm.py` at line 2000, The unpacking of self._do_axes[channel] assigns an unused variable lpos; change the tuple unpack to use a prefixed underscore for that element (e.g., replace lpos with _lpos) so the line "_, hpos, lpos, dur = self._do_axes[channel]" becomes "_, hpos, _lpos, dur = self._do_axes[channel]" to satisfy static analysis while leaving behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/photo_det_live.py`:
- Around line 27-31: The return annotation uses the quoted type
"StreamController" but StreamController isn't imported, causing an undefined
name error; import StreamController from odemis.gui.cont.stream at the top of
the module and update the return annotation to use StreamController (remove the
quotes) so the annotation references the actual class; alternatively you may add
"from __future__ import annotations" if you prefer postponing evaluation, but
the primary fix is adding the import and unquoting the return type.
In `@src/odemis/driver/test/tmcm_test.py`:
- Line 532: The line assigning the device position has a minor formatting issue:
add a space around the equals operator in the expression that sets pos from
self.dev.position.value; update the statement in tmcm_test.py where pos is
assigned (referencing the symbol pos and the expression self.dev.position.value)
to use "pos = self.dev.position.value" with a space after the '=' for consistent
formatting.
In `@src/odemis/gui/cont/stream_bar.py`:
- Around line 2008-2016: The current lookup uses
self._getAffectingSpectrograph(main_data.time_correlator) only; if that returns
None you must also try resolving the spectrograph via the photo-detector path
used elsewhere (e.g. the same logic applied to photo-detector children in
mdupdater). Modify the block around _getAffectingSpectrograph and
main_data.time_correlator so that when spg is None you call
self._getAffectingSpectrograph on the photo-detector component(s) associated
with main_data (e.g. main_data.photo_detector or its children/aliases used in
mdupdater) and assign spg from that result before adding the
"wavelength"/"grating"/"iris-in"/"slit-in"/"slit-monochromator" entries to axes.
---
Nitpick comments:
In `@src/odemis/driver/tmcm.py`:
- Line 2000: The unpacking of self._do_axes[channel] assigns an unused variable
lpos; change the tuple unpack to use a prefixed underscore for that element
(e.g., replace lpos with _lpos) so the line "_, hpos, lpos, dur =
self._do_axes[channel]" becomes "_, hpos, _lpos, dur = self._do_axes[channel]"
to satisfy static analysis while leaving behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30330c0a-5d7f-4f55-abab-e38ebbfdd4cd
📒 Files selected for processing (6)
plugins/photo_det_live.pysrc/odemis/acq/path.pysrc/odemis/driver/test/tmcm_test.pysrc/odemis/driver/tmcm.pysrc/odemis/gui/cont/stream_bar.pysrc/odemis/odemisd/mdupdater.py
tmoerkerken
approved these changes
Mar 9, 2026
Allow to force the protection from a separate component. This can be useful in case another component can also cause some light in.
If a (optical fiber coupled to a) time-correlator is connected to the spectrograph output, it should also have the MD_OUT_WL metadata stored, similarly to the monochromator. Do the same with the photo-detectors, as they are children of the time-correlator.
If a (optical fiber coupled to a) time-correlator is connected to the spectrograph output, then the position of the spectograph input slit matters. Note: this actuator position will only be changed if it "affects" (directly or indirectly) the time-correlator. So, on standard SPARCs where the time-correlator is coupled before the spectrograph, this change has no effect.
Many functions have "main_data = self._main_data_model". For these function, make sure to always use directly "main_data", instead of refering to "self._main_data_model". That mostly makes the code more readable... and should be a tiny bit faster.
…photo-detector count When adjusting the alignment of the fiber when a labcube is coupled after the spectrograph, seeing the detector count over time can be useful. Otherwise, the only view possible is the immediate count (via odemis live). As this alignment is normally done only once per installation, and there is only one installation, it's currently just a plugin. If it needs to be done more frequently, we could add a new alignment mode to the alignment tab.
pieleric
force-pushed
the
feat-sparc-support-for-time-correlator-stream-with-labcube-after-spectrograph
branch
from
2cec4de to
fae9e8d
Compare
This was referenced Apr 7, 2026
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
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.
Add support for placing the optical fiber going to the LabCube after spectrograph instead of before. That means that the labcube now receives a very narrow band of the light wavelength. IOW, this allows time correlator acquisitions wavelength specific.
In practice, this means this changes: