refactor(mcs-card): adjust mcs card to only have mca channels. #114
Merged
appel_c
merged 9 commits from 2026-01-26 07:44:43 +01:00
refactor/mcs_card_refactoring into main
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
issue::not confirmed
issue::not reproducible
issue::reproducible
priority::critical
priority::needed
priority::nice to have
scope::backend
scope::frontend
stage::in development
stage::on hold
stage::pre_release
stage::ready for review
stage::selected for development
stage::weekly backlog
type::bug
type::discussion
type::doc
type::feature
type::improvement
type::test
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Used if an issue has not yet been confirmed by a developer
Used if an issue is not reproducible
Used if a developer was able to reproduce the issue
Should be done as soon as possible
An important update to the code base
Not really needed but a good improvement
An issue regarding the backend services
An issue regarding the user-facing services
This issue is currently being worked on
Something is blocking the development
For issues that have been merged into a pre-release branch and are waiting for their release.
For issues that have been resolved and are ready for review
This issue has been selected for development but has not been scheduled yet
This issue has been selected for development for the next development cycle
It's a bug!
A discussion is wanted/needed
Improvements to the documentation
A new feature
Neither a feature nor a bug but an improvement to the code base
Missing tests or improvements to the test suite
Something is not working
This issue or pull request already exists
New feature
Need some help
Something is wrong
More information is needed
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
acerbo_a (acerbo_a)
bec-gitea
celcer_t (celcer_t)
debenjak_l (debenjak_l)
diaz (diaz)
divall_e (divall_e)
guizar_m (guizar_m)
haghta_s (haghta_s)
holler (holler)
lombosi_c (lombosi_c)
menzel (menzel)
perl_d (perl_d)
usov_i (usov_i)
wakonig_k (wakonig_k)
wang_x1 (wang_x1)
wyzula_j (wyzula_j)
zamofing_t (zamofing_t)
Clear assignees
No Assignees
appel_c
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bec/csaxs_bec#114
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
This PR revisited and partially rewrote the integration of the MCS card and the DigitalDelayGenerator (DDG) devices at the beamline. Hardware components were extensively tested to navigate and compensate any HW (or IOC) limitations of the individual devices, and integrated them for any software triggered scans running from BEC. The MCS card can now be operated in burst mode (frames_per_trigger >1) and we currently limit acquisitions to <5000Hz.
This limits is currently established on the DelayGenerator as the lowest deadtime, but can be adjusted if needed. The IOC of the MCS card was adjusted to allow 1e5 (or 1e6, to be checked) points. This limits the total number of points within a scan. The card is programmed to buffer all data before it is being read out. It should be checked if continuous readouts is going to be faster (and stable). As discussed, the MCS card now simply exposes the 32 input channels. We will discuss to create virtual devices who will use the channels to calculate new signals, e.g. a 'bpm' device. These virtual devices are not part of this PR.
Documentation
As both device integration are quite complex and are already heavily involved with beamline logic, I have spent additional time to write a documentation for the ophyd classes. This includes comments about HW (or IOC) limitations and obstacles that need to be considered as some operations have unexpected side effects that have to be considered in the integration.
In addition, due to the central timing source (master trigger), different devices share dependencies if certain procedures have to be executed in order. This creates additional complexity in the integration, in particular with the imposed limitations of the HW. All of this is documented as good as possible in README.md files in both modules, as well as directly in the source code of the device integrations.
To Do's
be5c0d870bto077359d63e077359d63etoe7f18db24ae046412eb8to47ff1c6699b9a9b5302ftob5369682deb5369682deto351e7a0e3dlooks good! - Only some minor comments.
@@ -0,0 +28,4 @@bec: BECClient # type: ignore[no-redef]gui: BECGuiClient # type: ignore[no-redef]as discussed offline, gui is not a builtin. It is better to access it through bec.gui
@@ -0,0 +49,4 @@name = re.sub(r"[^a-zA-Z0-9_]", "", name)if not name:return "_"I guess if there is no valid name, it should raise, no?
@@ -0,0 +60,4 @@class DebugTools:"""A collection of debugging tools for the BEC IPython client at cSAXS."""_PURPOSE = " ".join(I don't understand why this is a joined list of strings. Why not just a single string?
@@ -0,0 +113,4 @@if "mcs" not in dev:raise RuntimeError("MCS device is not loaded in the current active BEC session.")def _check_if_ddg_is_loaded(self):probably worth merging these methods into a single one. Something along the lines of
Also, I would suggest to print the exact name in the error message of the device, not capitalized. Otherwise one might be tempted to add a device "MCS" rather than "mcs"
@@ -0,0 +137,4 @@self._check_if_ddg_is_loaded()if mode == "high_frame":burst_frames = np.random.randint(10000, 100000) # between 10000 and 100000just a minor hint: you can set separators for large numbers, e.g. 10_000 instead of 10000. Could make it easier to read. See https://peps.python.org/pep-0515/
@@ -0,0 +246,4 @@return Falsedef open_it_service_page(self):"""Open the overview of IT services hosted by Science IT (Leo Sala) for cSAXS."""let's remove the personal name
@@ -210,3 +278,1 @@if mapped_signal_name == "count_time":value = value / self._mcs_clockvalue = float(value.mean())value = value.tolist() # Convert numpy array to listI guess it can only be numpy, single value or list? Maybe put it as type hint to the signature
351e7a0e3dto11c887b078