[TRCL-645][TRCL-694][feat] SPARC streakcam: improve settings control by pieleric · Pull Request #3454 · delmic/odemis

pieleric · 2026-04-24T14:58:18Z

Several small improvements to the GUI when using SPARC with streakcam.

Can control the phase lock (on/off) of the Hamamatsu delay generator from Odemis
User can control the trigger delay directly from the acquisition tab
time range -> trigger delay connection was not set at init
Add entries to write down the gun parameters in the metadata

Now the Temporal spectrum stream looks like that:

The gun exciter settings look like that:

Useful to record the e-beam column settings. Also introduce a small plugin to automatically load the values based on the selected operation.

Not useful per-say, but if the triggerDelay VA range is changed, to simulate different hardware (such as for the C12270), then it's easier.

There was a bit of support for this delay generator already in, but not enough to be able to control anything. Based on the (very little) documentation, and some testing, we now have basic phase lock control.

… TemporalSpectrum It's handy for the user to see the actual triggerDelay (and in some cases, change it) when playing the stream. For the cases where the hardware support .phaseLock, also allow to control it from Odemis (as a checkbox).

So far, changing the timeRange would update the triggerDelay, based on the metadata. However, if the timeRange value at init is the one the user intends to use, then the triggerDelay is not set. It's not possible to use init=True on the VA, because at init the metadata is not set yet. So instead, on the first time the metadata is loaded (typically, by the backend, just after init), set the value. When the metadata is updated, we let the caller decide whether the value is to be directly set or not (typically, this is done by the GUI, which does update the value directly too).

coderabbitai · 2026-04-24T14:58:25Z

Warning

Rate limit exceeded

@pieleric has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 40 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 40 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e3652d9f-ddc0-4129-af48-3e29cf32a55e

📥 Commits

Reviewing files that changed from the base of the PR and between c27fd50 and 7c85a3d.

📒 Files selected for processing (8)

install/linux/usr/share/odemis/sim/sparc2-streakcam-sim.odm.yaml
plugins/gun_exciter_operation.py
src/odemis/acq/stream/_helper.py
src/odemis/driver/hamamatsurx.py
src/odemis/driver/simstreakcam.py
src/odemis/driver/streak.py
src/odemis/gui/conf/data.py
src/odemis/gui/cont/stream_bar.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR improves SPARC streakcam temporal spectrum usability by exposing additional streak-delay settings in the stream UI, adding phase-lock support, and introducing an operation-mode loader for the e-beam gun exciter.

Changes:

Extend Temporal Spectrum stream creation to include streak-delay “hw VAs” (e.g. trigger delay / phase lock) in the stream panel.
Add/extend delay-generator behavior so that when MD_TIME_RANGE_TO_DELAY is first set, triggerDelay is immediately updated from the current timeRange; add phase-lock VA support for sim and Hamamatsu RemoteEx delay generator.
Add a new plugin that applies gun-exciter VA presets from MD_CALIB when the operation VA changes; update GUI config and sim YAML accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/odemis/gui/cont/stream_bar.py`	Passes streak-delay VA names into Temporal Spectrum stream creation.
`src/odemis/gui/conf/data.py`	Updates HW settings UI config for gun exciter ordering/tooltips; adds streak-delay-related controls to streak stream UI config.
`src/odemis/driver/streak.py`	Updates metadata handling to immediately apply trigger delay when calibration is first provided.
`src/odemis/driver/simstreakcam.py`	Simulates `phaseLock` and aligns trigger-delay range validation with actual VA range; applies immediate update on first calibration set.
`src/odemis/driver/hamamatsurx.py`	Adds phase-lock control via RemoteEx “Lock Mode”, adjusts delay VA units for C12270, and applies immediate update on first calibration set.
`src/odemis/acq/stream/_helper.py`	Extends `TemporalSpectrumSettingsStream` to surface streak-delay VAs as hardware VAs on the stream.
`plugins/gun_exciter_operation.py`	New plugin to auto-apply VA presets based on `operation` and `MD_CALIB`.
`install/linux/usr/share/odemis/sim/sparc2-streakcam-sim.odm.yaml`	Updates sim gun-exciter laser VA naming and adds operation/calibration metadata.

Copilot · 2026-04-24T15:05:50Z

            detvas=detvas,
-            streak_unit_vas=get_local_vas(main_data.streak_unit, main_data.hw_settings_config))
+            streak_unit_vas=get_local_vas(main_data.streak_unit, main_data.hw_settings_config),
+            streak_delay_hwvas=get_local_vas(main_data.streak_delay, main_data.hw_settings_config),


streak_delay_hwvas is currently populated with get_local_vas(main_data.streak_delay, ...), which will include all non-readonly delay generator VAs (e.g. delayB, delayC, …) since there’s no streak-delay role config to hide them. This will likely flood the Temporal Spectrum stream panel with many controls, even though the GUI config change only adds entries for triggerDelay/phaseLock.

Consider passing an explicit small set (e.g. just {"triggerDelay", "phaseLock"}) or adding a streak-delay HW settings config that hides unwanted delay VAs so get_local_vas() returns only the intended ones.

Suggested change

streak_delay_hwvas=get_local_vas(main_data.streak_delay, main_data.hw_settings_config),

streak_delay_hwvas=[

va_name for va_name in get_local_vas(main_data.streak_delay, main_data.hw_settings_config)

if va_name in {"triggerDelay", "phaseLock"}

],

Copilot · 2026-04-24T15:05:51Z

+    def __init__(self, name, detector, dataflow, emitter,
+                 streak_unit: model.HwComponent, streak_delay: model.HwComponent,
+                 streak_unit_vas: set, streak_delay_hwvas: set = None, **kwargs):


The new parameters on TemporalSpectrumSettingsStream.__init__ use very broad/ambiguous typing (streak_unit_vas: set, streak_delay_hwvas: set = None) even though these are sets of VA names (set[str]) and streak_delay_hwvas is optional. Also, the earlier parameters (name, detector, dataflow, emitter) remain untyped while this constructor is being modified.

Tighten the type hints (e.g. set[str] / Optional[set[str]]) and add parameter types for the remaining args for consistency within this method.

Suggested change

def __init__(self, name, detector, dataflow, emitter,

streak_unit: model.HwComponent, streak_delay: model.HwComponent,

streak_unit_vas: set, streak_delay_hwvas: set = None, **kwargs):

def __init__(self, name: str, detector: Any, dataflow: Any, emitter: Any,

streak_unit: model.HwComponent, streak_delay: model.HwComponent,

streak_unit_vas: set[str], streak_delay_hwvas: Optional[set[str]] = None,

**kwargs: Any) -> None:

Copilot · 2026-04-24T15:05:51Z

+        :param streak_unit: the streak-unit (ie, fast scanner) which is used to control the time range
+        :param streak_delay: the streak-delay (ie, time offset) which is used to control the signal timing
+        :param streak_unit_vas: set of VAs of the streak unit that should be duplicated in the stream (see detvas)
+        :param streak_delay_hwvas: set of VAs of the streak delay that should *copied* on the stream (see hwdetvas)


Docstring typo: “should copied on the stream” is missing “be”.

Suggested change

:param streak_delay_hwvas: set of VAs of the streak delay that should *copied* on the stream (see hwdetvas)

:param streak_delay_hwvas: set of VAs of the streak delay that should be copied on the stream (see hwdetvas)

Copilot · 2026-04-24T15:05:51Z

+            return
+
+        self._gun_exciter = gun_exciter
+        gun_exciter.operation.subscribe(self._on_operation)


The plugin subscribes to gun_exciter.operation without init=True (and doesn’t otherwise apply the current operation at startup). This means the calibration values won’t be applied until the user changes the operation, which can leave filamentCurrent/extractorVoltage out of sync with the selected operation after launch.

Subscribe with init=True or explicitly call the handler once using the current operation.value after subscribing.

Suggested change

gun_exciter.operation.subscribe(self._on_operation)

gun_exciter.operation.subscribe(self._on_operation, init=True)

Copilot · 2026-04-24T15:05:52Z

        min_time = 0
        max_time = float(self.parent.DevParamInfoEx(self.location, delay_name)[-1])
-        max_time = min(max_time, 10)  # don't report too high range
        range_time = (min_time, max_time)


GetDelayRangeByName() no longer caps max_time (previously limited to 10). For DG645-style delays this can expose very large ranges returned by RemoteEx, making the GUI control hard to use and increasing the risk of setting extreme values by accident.

If the cap was removed to support the C12270 “Delay Time” case, consider applying the cap conditionally (e.g. keep a reasonable cap for time-based delays with unit "s", but not for the tick-based “Delay Time”).

Copilot · 2026-04-24T15:05:52Z

+        if first_time and model.MD_TIME_RANGE_TO_DELAY in md and self._streak_unit:
+            self._on_time_range(self._streak_unit.timeRange.value)


DelayConnector.updateMetadata() references self._streak_unit, but _streak_unit is only set when a streak-unit dependency is provided; otherwise it’s never defined in __init__, so this can raise AttributeError when metadata is updated on connectors without a streak-unit.

Define self._streak_unit = None in the no-dependency branch (or use getattr(self, "_streak_unit", None) in the condition) before calling _on_time_range.

Suggested change

if first_time and model.MD_TIME_RANGE_TO_DELAY in md and self._streak_unit:

self._on_time_range(self._streak_unit.timeRange.value)

streak_unit = getattr(self, "_streak_unit", None)

if first_time and model.MD_TIME_RANGE_TO_DELAY in md and streak_unit:

self._on_time_range(streak_unit.timeRange.value)

pieleric added 5 commits April 24, 2026 16:58

[feat] SPARC GUI: add extra data to be displayed about the gun-exciter

cb69ce1

Useful to record the e-beam column settings. Also introduce a small plugin to automatically load the values based on the selected operation.

[fix] driver simstreakcam: generalize code for checking trigger delay

65eab3f

Not useful per-say, but if the triggerDelay VA range is changed, to simulate different hardware (such as for the C12270), then it's easier.

[feat] driver hamamatsurx: C12270 delay generator support

95bb436

There was a bit of support for this delay generator already in, but not enough to be able to control anything. Based on the (very little) documentation, and some testing, we now have basic phase lock control.

Copilot AI review requested due to automatic review settings April 24, 2026 14:58

github-actions Bot added the size/M label Apr 24, 2026

Copilot started reviewing on behalf of pieleric April 24, 2026 14:58 View session

pieleric changed the title ~~[feat] SPARC streakcam: improve settings control~~ [TRCL-645][TRCL-694][feat] SPARC streakcam: improve settings control Apr 24, 2026

pieleric requested review from nandishjpatel and tepals April 24, 2026 15:03

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TRCL-645][TRCL-694][feat] SPARC streakcam: improve settings control#3454

[TRCL-645][TRCL-694][feat] SPARC streakcam: improve settings control#3454
pieleric wants to merge 5 commits into
delmic:masterfrom
pieleric:feat-sparc-streakcam-improve-settings-control

pieleric commented Apr 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 24, 2026

Rate limit exceeded

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            streak_delay_hwvas=get_local_vas(main_data.streak_delay, main_data.hw_settings_config),
+            streak_delay_hwvas=[
+                va_name for va_name in get_local_vas(main_data.streak_delay, main_data.hw_settings_config)
+                if va_name in {"triggerDelay", "phaseLock"}
+            ],

	:param streak_delay_hwvas: set of VAs of the streak delay that should copied on the stream (see hwdetvas)
	:param streak_delay_hwvas: set of VAs of the streak delay that should be copied on the stream (see hwdetvas)

	gun_exciter.operation.subscribe(self._on_operation)
	gun_exciter.operation.subscribe(self._on_operation, init=True)

		if first_time and model.MD_TIME_RANGE_TO_DELAY in md and self._streak_unit:
		self._on_time_range(self._streak_unit.timeRange.value)

-        if first_time and model.MD_TIME_RANGE_TO_DELAY in md and self._streak_unit:
-            self._on_time_range(self._streak_unit.timeRange.value)
+        streak_unit = getattr(self, "_streak_unit", None)
+        if first_time and model.MD_TIME_RANGE_TO_DELAY in md and streak_unit:
+            self._on_time_range(streak_unit.timeRange.value)

Conversation

pieleric commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 24, 2026

Rate limit exceeded

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pieleric commented Apr 24, 2026 •

edited

Loading