feat(omny): add holler to tomo superuser accounts; refresh OPEN_ISSUES.md
CI for csaxs_bec / test (push) Failing after 5s

Mirko is testing live from his own machine as OS user "holler" with no
active_account set, so the superuser fallback (os.getenv("USER")) needs
"holler" in the allow-list to see the advanced tomo_parameters() options.

OPEN_ISSUES.md had gone stale relative to everything fixed this session:
the rotation-offset section still described it as an open, hardware-review
-only question (resolved days ago once the real DMC controller source
confirmed the real hardware code was always correct -- only the simulator
was missing the offset), and the checklist still said scans were blocked
entirely (a full omny_fermat_scan now runs end to end). Rewrote both
sections to reflect current reality, added a "Current status" summary at
the top so a fresh session can orient in one read, and noted the
_SUPERUSER_ACCOUNTS change and that advanced tomo mode testing is next.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLrD7sVYGLAzsQjLVJpCgt
This commit is contained in:
x01dcandClaude Sonnet 5 committed 2026-09-02 12:37:01 +02:00
1 parent e0213697bc
commit bd129b6313
2 files changed
+85 -60

No files matched your search

@@ -1,11 +1,34 @@
# OMNY open issues (as of 2026-08-26)
# OMNY open issues (as of 2026-09-02)
Tracks what the first OMNY live-smoke-test session found: bugs (fixed and unfixed) and
what's still untested. Written so a future session can pick this up without re-deriving
it. Companion docs: `../../flomni/AI_docs/SIMULATED_ENDSTATIONS.md` (the sim architecture
and the fullest write-up of the rotation-offset bug below) and
`../../flomni/AI_docs/FLOMNI_TO_LAMNI_COMPARISON.md` (the flomni→lamni migration this
OMNY work is meant to mirror).
Tracks the live-testing effort for bringing OMNY up to flomni's/BEC's current standards on
branch `update-omny`: bugs found (fixed and unfixed) and what's still untested. Written so a
future session can pick this up without re-deriving it. Companion docs:
`../../flomni/AI_docs/SIMULATED_ENDSTATIONS.md` (the sim architecture) and
`../../flomni/AI_docs/FLOMNI_TO_LAMNI_COMPARISON.md` (the flomni→lamni migration this OMNY work
is meant to mirror).
## Current status
A full `omny_fermat_scan` (via `omny.tomo_scan_projection(<angle>)`) now runs end to end against
`simulated_omny.yaml` without crashing — rotation, feedback, and the scan itself all complete.
Getting there took several rounds of live-tested fixes; see the dated sections below for the
full trace of each. GUI flows (`omny.omnygui_show_progress()`, `omnygui_show_xeyealign()`,
`omnygui_show_omnycam_parking()`/`omnygui_show_omnycam_samplestage()`) also work now, ported onto
flomni's current `gui_tools.py` API.
**Not yet started**: testing the advanced tomo modes (golden ratio, 4/8 sub-tomogram) added
earlier this branch — this is the next thing being tested live. `holler` (Mirko's account) was
added to `_SUPERUSER_ACCOUNTS` (`omny.py`) alongside `gac-x01dc` so `tomo_parameters()` shows the
advanced options when testing from his own machine, where `bec.active_account` is unset and the
superuser check falls back to the OS login.
**Still open / deferred** (unchanged from before, not touched this pass):
- `x_ray_eye_align.py` is still LamNI-derived, unadapted code beyond the two call-site renames
that unblocked the GUI-open crash — see its own section below. The GUI itself opens fine now;
the alignment *logic* underneath it is still wrong for OMNY.
- The full gripper sample-transfer chain (`otransfer_put_sample()`/`otransfer_get_sample()`),
dewar/temperature methods, sample storage bookkeeping, and `omny_alignment_mixin.py`/
`omny_optics_mixin.py` beyond what scans exercise — see "Checklist" section below.
## Standing question: OMNY should follow flOMNI, not LamNI
@@ -18,41 +41,42 @@ like LamNI, and is a general lens to apply going forward: if fixing an OMNY bug
choosing what the "correct" shape looks like, default to whatever flOMNI already does,
not whatever LamNI does — a LamNI-shaped OMNY file is more likely a leftover copy-paste
than a deliberate design choice. See the `x_ray_eye_align.py` finding below for a concrete
example already found this way.
example already found this way, and the `omny_fermat_scan` rotation-crash section further
down for a case where following flomni's exact pattern was the fix.
## Bug found, NOT fixed — needs Mirko's call before touching it
## `osamroy` rotation offset — resolved (real hardware code was correct; only the simulator was wrong)
**`osamroy` (rotation axis) permanently fails every `.move()`/`umv()` call.** Full
technical write-up: `SIMULATED_ENDSTATIONS.md`'s OMNY live-test section. Short version:
`OMNYGalilReadbackSignal._socket_get()` (`devices/omny/galil/ogalil_ophyd.py:56-60`) adds
a documented `+25`° offset on *read* only ("we introduce an offset of 25 to the rotation
axis... that way we just do tomography from 0 to 180 degrees"). Nothing subtracts it back
out on the *write* side (`OMNYGalilMotor.move()`, same file) or in that method's own
success check, so `readback (= raw + 25)` is compared against the raw target passed to
`.move()` — always off by exactly 25, for any target. This is real, unmodified hardware-
facing code (not a sim artifact — the sim just relays it faithfully), and it
unconditionally blocks `feedback_enable_with_reset()` (`rt_omny_ophyd.py:526-533`), which
blocks every scan needing interferometer feedback, i.e. effectively all of them.
Originally logged here as an unresolved, hardware-review-only question: whether
`OMNYGalilMotor.move()` (`devices/omny/galil/ogalil_ophyd.py`) or `feedback_enable_with_reset()`
(`rt_omny_ophyd.py`) had the wrong sign/space for the documented `+25`° read-side offset on the
rotation axis (`OMNYGalilReadbackSignal._socket_get()`), since nothing appeared to subtract it
back out on the write side. **Resolved once the real Galil DMC controller source became
available** (`controller1.dmc`, `contrID=3` handler): the real controller firmware already
applies `ntarget=ntarget-25` on write for this axis — `OMNYGalilMotor.move()` was never buggy.
Only the **simulator** (`csaxs_bec/devices/sim/sim_galil.py`) was missing this, silently passing
through raw un-offset moves. Fixed in `SimGalilState.start_move_from_newpar()`: applies the same
`-25` when the port/axis matches the OMNY rotation stage, mirroring the real controller exactly.
Also seeded `osamroy`'s `sim_initial_position: -25` in `simulated_omny.yaml` so a fresh sim
session starts at displayed 0° instead of 25° (round-trips through the same offset via
`OMNYGalilReadbackSignal`'s `+25` read-side conversion).
Two plausible fixes, and only Mirko can say which matches real hardware intent:
1. `OMNYGalilMotor.move()` should convert the caller's display-space target to raw
(subtract 25) before sending it over the wire, for this axis — i.e. `.move(0)` means
"go to displayed zero."
2. Callers are expected to already pass raw targets, in which case
`feedback_enable_with_reset()`'s `osamroy.obj.move(0, wait=True)` and its
`np.fabs(readback) > 0.1` guard (which reads *display*-space) are the ones using the
wrong space, and should target/compare against `-25` instead of `0`.
Separately, and unrelated to this offset: `omny_rotation()`'s own `.set()` call had a different,
real bug (targeting `osamroy.user_setpoint` instead of the root `osamroy` device), which caused a
readback-progressbar crash later in the scan. See the `omny_fermat_scan` section below — already
fixed, do not confuse with this offset issue.
Do not guess-fix this one — a wrong sign/direction on a real rotation-stage offset is the
kind of thing that should get hardware-review, not silent inference from code reading.
## `x_ray_eye_align.py` — worse than the crash that got fixed
## `x_ray_eye_align.py` — GUI now opens, underlying logic still wrong
The construction-time crash (`self.lamni.align.reset_correction()` called on an object
still mid-construction) is fixed (matches the fix LamNI's own copy of this file already
got — see its `__init__` for the rationale comment). But that only unblocks
*construction*. The rest of `omny/x_ray_eye_align.py` is still an **unadapted copy of
`LamNI/x_ray_eye_align.py`**, not code actually written for OMNY:
got — see its `__init__` for the rationale comment). The two GUI-open call sites
(`self.lamni.lamnigui_show_xeyealign()`/`lamnigui_show_xeyealign_fittab()` — LamNI-only method
names that don't exist on `OMNYGuiTools`) were also fixed, renamed to
`omnygui_show_xeyealign()`/`omnygui_show_xeyealign_fittab()`, so `omny.align.align()` now gets
past the GUI-open step without crashing.
The rest of `omny/x_ray_eye_align.py` is still an **unadapted copy of `LamNI/x_ray_eye_align.py`**,
not code actually written for OMNY:
- Docstring: `"""BEC-GUI-based X-ray eye alignment for LamNI. ... The key difference from
the FlOMNI alignment is that LamNI needs *both* x and y fits..."""` — describes LamNI
vs flOMNI, doesn't mention OMNY at all.
@@ -62,38 +86,36 @@ got — see its `__init__` for the rationale comment). But that only unblocks
- `LAMNI_ALIGNMENT_ANGLES = [k * 45 for k in range(8)]` (8 angles, full 360°, laminography-
specific) — presumably wrong for OMNY, which (per the flOMNI/LamNI comparison doc) is
the kind of full-rotation tomography flOMNI's alignment already handles differently.
- References devices/methods (`lsamrot`, `lfzp_in`, `omnyfsh`, `omny_xray_gui`) that mix LamNI
and OMNY naming conventions — needs a careful pass against OMNY's actual device inventory.
None of this was exercised in the live smoke test beyond the constructor no longer
crashing — `start_x_ray_eye_alignment()` / `xrayeye_update_frame()` were never called.
Given the "OMNY ≈ flOMNI, not LamNI" principle above, the right fix is almost certainly to
port OMNY's version from `flomni/x_ray_eye_align.py` (440 lines, the flOMNI-specific
implementation) the way `LamNI/x_ray_eye_align.py` was itself adapted from flOMNI's,
rather than patching the current LamNI-derived file in place.
None of the alignment logic itself has been exercised live yet — only the constructor and the
GUI-open step. Given the "OMNY ≈ flOMNI, not LamNI" principle above, the right fix is almost
certainly to port OMNY's version from `flomni/x_ray_eye_align.py` (440 lines, the flOMNI-specific
implementation) the way `LamNI/x_ray_eye_align.py` was itself adapted from flOMNI's, rather than
patching the current LamNI-derived file in place. Not started.
## Checklist items not yet run (smoke test only covered 3 of 4)
## Checklist items not yet run
From the live session against `simulated_omny.yaml` / `bec --session omny`:
- **Passed**: config load, a translation-axis move (`osamx`, `osamy`), one lightweight
transfer (`omny.otransfer_park_slot(6)`).
- **Blocked**: any scan (see rotation-offset bug above) — includes `omny.tomo_scan()`
itself, never reached.
- **Passed** (live, against `simulated_omny.yaml` / `bec --session omny`): config load,
translation-axis moves (`osamx`, `osamy`), a lightweight transfer (`omny.otransfer_park_slot(6)`),
`feedback_enable_with_reset()`, a full `omny_fermat_scan` via `omny.tomo_scan_projection(<angle>)`
end to end, and the GUI flows listed under "Current status" above.
Still completely untested (not attempted yet, not known-broken — just unexplored):
Still untested (not attempted yet, not known-broken — just unexplored):
- **Advanced tomo modes** (golden ratio, 4/8 sub-tomogram, `tomo_parameters()`'s superuser-gated
options) — about to be tested next.
- The **full gripper sample-transfer chain**: `otransfer_put_sample()` /
`otransfer_get_sample()` (the actual pin pickup/dropoff with the `confirm`/`atconfir`
handshake), as opposed to the lighter `otransfer_park_slot()` used for the smoke test.
- **X-ray eye alignment** end-to-end (blocked in practice by the section above, but worth
retesting once that file is properly ported).
handshake), as opposed to the lighter `otransfer_park_slot()` used so far.
- **X-ray eye alignment** end-to-end alignment logic (GUI now opens fine, see section above —
the actual click-through alignment procedure itself hasn't been exercised).
- **Dewar / temperatures** (`omny_dewar`, `omny_temperatures` devices) — they load and
construct (`SimOMNYDewar`/`SimOMNYTemperatures`), but no `omny.py`-level method that
reads/uses them was called.
reads/uses them has been called live.
- **Sample storage** (`omny_samples`) beyond it loading with seeded slots — no
`omny.py`-level storage-bookkeeping method exercised.
- `omny_alignment_mixin.py`, `omny_optics_mixin.py`, and the rest of `omny_rt.py` beyond
`feedback_enable_with_reset`/`feedback_disable`/`feedback_status` — none of these were
called.
- GUI-dependent flows (`omnygui_show_gui()` etc.) — the smoke test ran `--nogui`; nothing
that needs a real BEC-widgets window was exercised.
- `omny_alignment_mixin.py`, `omny_optics_mixin.py` beyond what a fermat scan's own setup touches.
## Known, already-documented, separate from this session
@@ -46,9 +46,12 @@ class OMNYError(Exception):
# modes (golden ratio, 4/8 sub-tomogram) in OMNY.tomo_parameters(). Regular users only
# ever see "1: 2 equally spaced sub-tomograms". active_account is a Redis-backed
# pgroup value, set explicitly via `bec-set-account <pgroup>` -- it is NOT derived
# from the OS login, so testing as this account requires running
# `bec-set-account gac-x01dc` first (or wiring pgroup "gac-x01dc" some other way).
_SUPERUSER_ACCOUNTS: frozenset[str] = frozenset({"gac-x01dc"})
# from the OS login, so testing as one of these accounts requires either running
# `bec-set-account <name>` first, or (dev/test sessions where active_account is
# unset) simply being logged in as that OS user, since _is_superuser_account()
# falls back to os.getenv("USER") when active_account is blank. "holler" is
# Mirko's account, used for live testing this branch.
_SUPERUSER_ACCOUNTS: frozenset[str] = frozenset({"gac-x01dc", "holler"})
class OMNYInitStagesMixin: