Add the DomeWindScreen interface by wschoenell · Pull Request #267 · astroufsc/chimera

wschoenell · 2026-07-25T02:38:57Z

Declares the wind-screen dome role next to DomeSlit/DomeFlap: a screen positioned in altitude, in decimal degrees with horizon = 0 and zenith = 90. Also corrects the DomeFlap docstring ('Dome with Slit' -> 'Dome with Flap').

Interface — move_screen(alt), get_screen(), is_screen_moving(), abort_screen(), plus the screen_begin(alt) / screen_complete(alt, status) events. A move takes tens of seconds, so callers need the motion state to wait on it and the abort to stop it — without an abort, screen_complete(ABORTED) was unreachable. A position request is accepted with the slit closed and applied by the time it opens.

Config (in both DomeConfig and __config__): screen_track, screen_offset, screen_min_alt/screen_max_alt, screen_alt_resolution, screen_park_position, screen_timeout.

Base implementation — while the dome is in Track mode, the control loop keeps the screen at telescope altitude + screen_offset, clamped to the travel limits and deadbanded by screen_alt_resolution; park_on_shutdown stows the screen; get_metadata writes DOME_WSC, alongside a new DOME_FLP flap key. All of it is guarded by features("DomeWindScreen") — DomeBase does not inherit the interface, drivers opt in by mixing it in.

Driver and CLI — FakeDome is the reference implementation (timed move, honours abort, reports ABORTED). chimera-dome gains --move-screen ALT, prints the screen altitude in --info and aborts the screen on ^C. 21 tests cover signature conformance against the interface, the move/limits/abort lifecycle, event payloads, tracking (target, deadband, opt-out) and the control-loop integration.

No hardware driver implements it yet: the LNA 40 cm dome has no wind screen. The closest real target is the LCO Swope TCS, whose screen is jog-based (SCREENPOS GO UP|DOWN|STOP plus a position readout and a fully-open limit switch) — it would need stop_screen() and an is_screen_open(), left out of this PR.

Declares the wind-screen dome role (move_screen/get_screen + windscreen_begin/complete events) alongside DomeSlit/DomeFlap, and fixes the DomeFlap docstring (was 'Dome with Slit'). Interface only - no driver implements it yet; drafted so the in-progress work is not lost. Not deployed on opd-40 (the LNA dome has no wind screen).

…stroufsc#253 over master Audit follow-up. Reset every file that was diverging from upstream/master without an open PR back to master, per review decisions: - imagerequest.py: revert the dome-sync blast-radius containment (astroufsc#261 closed unmerged - a dome that cannot sync SHOULD fail the program); - DomeWindScreen + sextractor source-extractor fix: moved to their own draft PRs (astroufsc#267, astroufsc#268), dropped here (not needed on opd-40); - bus.py _outbound_dead vestige: dead after the merged bus refactor; - cli/guide.py: superseded by cli/guider.py (merged astroufsc#246); - misc core (constants, manager, proxy, fakes, old tests, AGENTS): master. Deploy branch now = upstream/master + astroufsc#255 + astroufsc#259 + astroufsc#253 content only.

The event was published as 'windscren_begin': EventWrapperDispatcher publishes under func.__name__, so the typo was the wire name and every subscriber would have to misspell it too. The default on windscreen_complete's status was inert and misleading: the event body never runs, the dispatcher forwards the call args verbatim, so firing it without arguments publishes an empty payload and breaks subscribers that declare the parameter. Status is now required and the events carry the altitude, like slew_begin/slew_complete carry the position. Also: name the screen consistently (screen_*), document units and datum (decimal degrees, horizon = 0, zenith = 90 -- ASCOM's Dome.Altitude convention), document the out-of-range exception, give get_screen the '...' body the other value-returning declarations in this module have, and export the dome sub-interfaces from __all__.

A screen move takes tens of seconds, so callers need is_screen_moving() to wait on it and abort_screen() to stop it -- without an abort there was also no way to ever reach screen_complete(ABORTED). ASCOM covers the same ground with Slewing/AbortSlew, which apply to dome altitude too. The config is what makes the screen usable without driver-private settings: travel limits for the range check, a resolution deadband and an offset for tracking, a park position for shutdown and a timeout for drivers, matching the slit's open/close_timeout. Both DomeConfig and __config__ carry it, they are duplicated in this module.

DomeBase gains the tracking half of the contract: while in Track mode the control loop keeps the screen at telescope alt + screen_offset, clamped to the travel limits and deadbanded by screen_alt_resolution, and park_on_shutdown now stows the screen too. Both are guarded by features("DomeWindScreen") -- DomeBase does not claim the interface, so only drivers that mix it in pay for it. FakeDome is the reference implementation, with a timed move that honours abort_screen and reports ABORTED, so the interface has something to test against. chimera-dome grows --move-screen, prints the screen altitude in --info and aborts the screen on ^C; get_metadata writes DOME_WSC.

wschoenell · 2026-07-25T19:20:09Z

+
+        if self.features("DomeWindScreen"):
+            metadata.append(
+                ("DOME_WSC", f"{self.get_screen():.2f}", "Dome wind screen altitude")


add flap too

Done in e4c63f2: get_metadata writes DOME_FLP next to DOME_SLT. Since every dome inherits DomeFlap, features() cannot tell us whether the driver really has one, so the key is emitted only when is_flap_open() does not raise NotImplementedError.

make the flap resolution mechanism same as windscreen

Done in 176d360. DomeBase no longer inherits DomeFlap, so features("DomeFlap") now answers whether the driver actually has one — the same opt-in mixin the wind screen uses. get_metadata drops the NotImplementedError probe, FakeDome mixes DomeFlap in explicitly, and chimera-dome refuses --open-flap/--close-flap on flapless domes and prints the flap state in --info.

Heads-up on the fallout: out-of-tree drivers that implement the flap methods on DomeBase without declaring the interface — chimera-astelco (AstelcoDome) and chimera-domesync (DomeSync) — need DomeFlap added to their bases to keep advertising it. One-line change in each.

Also merged current master into the branch: the only conflict was in chimera-dome's __abort__, where #266 dropped the copy.copy(self.dome) workaround; kept the upstream form and added the screen abort on top. PR is mergeable again.

wschoenell · 2026-07-25T19:21:31Z

+    is positioned in altitude.
+
+    Altitudes are in decimal degrees, horizon = 0 and zenith = 90, the same
+    convention used by ASCOM's Dome.Altitude.


remove ascom reference

Removed in e4c63f2 — the docstring now just states the convention (decimal degrees, horizon = 0, zenith = 90).

get_metadata now writes DOME_FLP next to DOME_SLT. Every dome inherits DomeFlap, so features() says nothing about whether the driver implements it: the NotImplementedError from DomeBase is what tells us the dome has no flap, and the key is skipped for those.

…screen # Conflicts: # src/chimera/cli/dome.py

DomeBase no longer inherits DomeFlap, so features("DomeFlap") answers whether the driver really has one instead of always saying yes -- the same opt-in mixin the wind screen uses. get_metadata drops the NotImplementedError probe, and chimera-dome refuses --open-flap / --close-flap on flapless domes and prints the flap state in --info. Out-of-tree drivers with a flap (chimera-astelco, chimera-domesync) need DomeFlap added to their bases to keep advertising it.

phsilva

some renaming ideas... domes could have flat screens too, so maybe always add wind prefix in commands here to make sure about what which screen we are referring to.

phsilva · 2026-07-28T00:36:29Z

+    clamped to ['screen_min_alt', 'screen_max_alt'].
+    """
+
+    def move_screen(self, alt: float) -> None:


Suggested change

def move_screen(self, alt: float) -> None:

def move_wind_screen(self, alt: float) -> None:

Applied in 06e1715.

phsilva · 2026-07-28T00:37:10Z

+        @rtype: None
+        """
+
+    def get_screen(self) -> float:


Suggested change

def get_screen(self) -> float:

def get_wind_screen_alt(self) -> float:

Applied in 06e1715.

phsilva · 2026-07-28T00:38:17Z

+        """
+        ...
+
+    def is_screen_moving(self) -> bool:


Suggested change

def is_screen_moving(self) -> bool:

def is_wind_screen_moving(self) -> bool:

Applied in 06e1715.

phsilva · 2026-07-28T00:38:36Z

+        """
+        ...
+
+    def abort_screen(self) -> None:


Suggested change

def abort_screen(self) -> None:

def abort_wind_screen_move(self) -> None:

Applied in 06e1715.

phsilva · 2026-07-28T00:40:03Z

+        """
+
+    @event
+    def screen_begin(self, alt: float) -> None:


Suggested change

def screen_begin(self, alt: float) -> None:

def wind_screen_move_begin(self, alt: float) -> None:

Applied in 06e1715.

phsilva · 2026-07-28T00:40:30Z

+        """
+
+    @event
+    def screen_complete(self, alt: float, status: DomeStatus) -> None:


Suggested change

def screen_complete(self, alt: float, status: DomeStatus) -> None:

def wind_screen_move_complete(self, alt: float, status: DomeStatus) -> None:

Applied in 06e1715.

phsilva · 2026-07-28T00:41:33Z

        self.out("OK")

+    @action(
+        long="move-screen",


Suggested change

long="move-screen",

long="move-wind-screen",

Applied in 06e1715.

Renames from phsilva's review -- a dome can carry a flat screen too, so 'screen' alone does not say which one: move_screen -> move_wind_screen get_screen -> get_wind_screen_alt is_screen_moving -> is_wind_screen_moving abort_screen -> abort_wind_screen_move screen_begin -> wind_screen_move_begin screen_complete -> wind_screen_move_complete --move-screen -> --move-wind-screen The config keys, the DomeBase helpers and the FakeDome internals follow the same prefix (screen_track -> wind_screen_track and so on), since the ambiguity is the same there.

wschoenell · 2026-07-28T01:02:41Z

All seven renames applied in 06e1715:

before	after
`move_screen`	`move_wind_screen`
`get_screen`	`get_wind_screen_alt`
`is_screen_moving`	`is_wind_screen_moving`
`abort_screen`	`abort_wind_screen_move`
`screen_begin`	`wind_screen_move_begin`
`screen_complete`	`wind_screen_move_complete`
`--move-screen`	`--move-wind-screen`

Carried the same prefix into the parts you did not annotate, since the flat-screen ambiguity is identical there: the config keys (wind_screen_track, wind_screen_offset, wind_screen_min_alt/wind_screen_max_alt, wind_screen_alt_resolution, wind_screen_park_position, wind_screen_timeout), the DomeBase helpers and the FakeDome internals, plus the prose in the docstrings. The FITS key was already DOME_WSC. Test file renamed to test_dome_wind_screen.py; 23 tests still pass.

codecov · 2026-07-28T01:06:49Z

Codecov Report

❌ Patch coverage is 73.83178% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.01%. Comparing base (e92487b) to head (df5c159).
⚠️ Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
src/chimera/cli/dome.py	0.00%	19 Missing ⚠️
src/chimera/instruments/dome.py	75.86%	5 Missing and 2 partials ⚠️
src/chimera/instruments/fakedome.py	95.23%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
+ Coverage   51.57%   52.01%   +0.44%     
==========================================
  Files         102      102              
  Lines        8521     8614      +93     
  Branches      978      997      +19     
==========================================
+ Hits         4395     4481      +86     
+ Misses       3928     3924       -4     
- Partials      198      209      +11

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

wschoenell added 3 commits July 25, 2026 11:51

wschoenell commented Jul 25, 2026

View reviewed changes

wschoenell changed the title ~~Add the DomeWindScreen interface (draft)~~ Add the DomeWindScreen interface Jul 25, 2026

wschoenell marked this pull request as ready for review July 25, 2026 19:35

wschoenell requested a review from phsilva July 25, 2026 19:35

wschoenell added 2 commits July 25, 2026 12:41

Merge remote-tracking branch 'upstream/master' into feature/dome-wind…

fb98a61

…screen # Conflicts: # src/chimera/cli/dome.py

phsilva requested changes Jul 28, 2026

View reviewed changes

phsilva approved these changes Jul 29, 2026

View reviewed changes

Merge branch 'master' into feature/dome-windscreen

df5c159

phsilva merged commit d2f918a into astroufsc:master Jul 29, 2026
1 check passed

wschoenell deleted the feature/dome-windscreen branch July 29, 2026 20:33

	def move_screen(self, alt: float) -> None:
	def move_wind_screen(self, alt: float) -> None:

	def get_screen(self) -> float:
	def get_wind_screen_alt(self) -> float:

	def is_screen_moving(self) -> bool:
	def is_wind_screen_moving(self) -> bool:

	def abort_screen(self) -> None:
	def abort_wind_screen_move(self) -> None:

	def screen_begin(self, alt: float) -> None:
	def wind_screen_move_begin(self, alt: float) -> None:

	def screen_complete(self, alt: float, status: DomeStatus) -> None:
	def wind_screen_move_complete(self, alt: float, status: DomeStatus) -> None:

Uh oh!

Conversation

wschoenell commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phsilva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wschoenell commented Jul 28, 2026

Uh oh!

codecov Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wschoenell commented Jul 25, 2026 •

edited

Loading

codecov Bot commented Jul 28, 2026 •

edited

Loading