Skip to content

Remove logic for checking if zoom level is available#1018

Merged
DominicOram merged 7 commits intomainfrom
remove_zoom_level_check
Jan 27, 2025
Merged

Remove logic for checking if zoom level is available#1018
DominicOram merged 7 commits intomainfrom
remove_zoom_level_check

Conversation

@shihab-dls
Copy link
Contributor

Fixes #884

EPICS enum records should error if provided an unspecified value; thus, this PR removes logic checking for unspecified values in the ZoomController.

Instructions to reviewer on how to test:

  1. Test if setting the str signal level to an unspecified zoom level raises an error.

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@shihab-dls shihab-dls requested a review from a team as a code owner January 24, 2025 16:58
@codecov
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.50%. Comparing base (aab178f) to head (a867b55).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
- Coverage   97.51%   97.50%   -0.01%     
==========================================
  Files         152      152              
  Lines        6351     6342       -9     
==========================================
- Hits         6193     6184       -9     
  Misses        158      158              

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

Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works but the error message is obtuse (see bluesky/ophyd-async#750). If there is some reason we can't improve the error in ophyd_async then we may want to keep this. I suggest we wait for that issue to have some discussion

Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this will get fixed in ophyd_async (bluesky/ophyd-async#752) so happy to merge this

@DominicOram DominicOram merged commit dbcd945 into main Jan 27, 2025
19 checks passed
@DominicOram DominicOram deleted the remove_zoom_level_check branch January 27, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove check for zoom levels in ZoomController

2 participants