adds a check for motor limit - #1289
Merged
Merged
Conversation
|
Great job! No new security vulnerabilities introduced in this pull requestCommunicate with Checkmarx by submitting a PR comment with @Checkmarx followed by one of the supported commands. Learn about the supported commands here. |
gkalua
marked this pull request as ready for review
June 12, 2026 15:47
coretl
reviewed
Jun 25, 2026
Collaborator
There was a problem hiding this comment.
Please can we make this so we report the motor name, value that would exceed the limit, and the limit it would exceed
coretl
reviewed
Jun 29, 2026
Comment on lines
+161
to
+164
| if any( | ||
| slice_lower <= motor_lower_limit or slice_upper >= motor_upper_limit | ||
| for motor in motors | ||
| ): |
Collaborator
There was a problem hiding this comment.
Suggested change
| if any( | |
| slice_lower <= motor_lower_limit or slice_upper >= motor_upper_limit | |
| for motor in motors | |
| ): | |
| if slice_lower <= motor_lower_limit or slice_upper >= motor_upper_limit: |
| f" motor limit is {motor_lower_limit} to {motor_upper_limit}" | ||
| ) | ||
|
|
||
| if any(current_velocity > max_velocity for motor in motors): |
coretl
reviewed
Jun 29, 2026
Comment on lines
+151
to
+152
| slice_lower = slice.lower[motor].min() | ||
| slice_upper = slice.upper[motor].max() |
Collaborator
There was a problem hiding this comment.
Just realized, this will fail for a spiral.
I think what you need is something that calculates the slice_min and slice_max as the min and max of slice lower, upper and midpoints
* Write all frames in first h5 file * Fix tests
* Replace AreaDetector writer parameters with ADWriterFactory * Fix tests Co-authored-by: Copilot <copilot@github.com> * Fix docs build * tests: remove redundant operations in tests --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Shihab Suliman <shihab.suliman@diamond.ac.uk>
…1262) * Initial implementation of user defined turnaround times * Remove unused print statements and make turnaround time a class attribute * move turnaround_time out of if statement in from_slice method * Change min_time based on min_interval * Add test for configurable ramp up/ramp down speeds * Changed min_time based on min_interval * PR comments --------- Co-authored-by: Shihab S <162436767+shihab-dls@users.noreply.github.com>
* Create StandardMovable * Fix motor stop * Removed unused variables * Fix stop test * Add docs * Replace motor with signals for MotorMoveLogic * Add tests * Remove unneeded old_position * Use dataclass for MotorMoveLogic * Improve error messages for motor to include motor name. * Correct MockDevice name * Update logic to be more flexbile, apply StandardMovable to SimMotor * Fix tests * Removed unused methods for SimMotrMoveLogic * Make check_move and calculate_timeout optional * Update DemoMotor to use StandardMovable * Add back commented out test * Add explaination for previous failing test / test change * Add generic type for user readback for StandardMovable * Update SimMotor stop test * Improve type checking * Fix derived signal test by improving get_locatable_type to walk full class * Make each method an optional hook * Make use of cached_property to avoid __init__ method * Refactor logic to move WatcherUpdate to MovableLogic * Fix SimMotor stop test * Try to fix CI by adding break * Tets fix for windows test_motor_moving_stopped * Undo change * Test to still if fixes test * Tets windows * Add skip to flakey windows python 3.11 test * Applied feedback * Remove custom SimMotorLogic move to see if handles stop correctly * Fix SimMotor stop * Update EpicsMotor to use EpicsDevice again * Make MovableLogic a dataclass so can remove duplication of setpoint and readback on Subclass MovableLogic * Add some class docs * Fix test * Simplify and increase code coverage for movable test * Fix tests, increase code coverage * Remove assert call_count == 1 for wait_for_call * test_locatable applied feedback * Apply feedback * Add back RuntimeError for StandardMovable to confirm reason for python 3.11 test failure * Update test doc string * Add back old demo motor test_motor_moving_well * Don't add default InstantMovableMock behaviour in epics demo test * Update timeout for movable * Fix SimMotor * Last time I swear... * Move to a single move method in MovableLogic * Revert "Move to a single move method in MovableLogic" This reverts commit 1f37105. * Extract the status reporting out of the move method * Fix motor moving tests to wait for pending tasks to update setpoint * Get units and precision from the readback signal by default * Added docs * Slacken off timing for CI * Address review comments * Add Checkable support * check_value can be pure async, not AsyncStatus * Fix docs: check_move signature and add Checkable to protocol lists * Rename ADR 0013 to 0017 --------- Co-authored-by: Tom Cobb <tom.cobb@diamond.ac.uk> Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>
* First draft of Command classes. WIP * Developing unit tests * fixing datakey * Core command * Core commands with syntax fixes. Removed old tango command code * Added test for behaviour when command is called with kwarg ordering different than signature and command_args list. * linting * removed ... and using pass * wip * Improved command with better static typing * Telling docs to ignore ParamSpec and TypeVar in _command * removed custom error classes. Added device logging * shortened source signature * Removed unnecessary factory functions. Removed unused units and precision arguments * Refactored SoftCommandBackend to enforce annotations at runtime. This means we can rely on annotations of the callback * Refactored call to triggerable * Added runtime validation for Array1D and Sequence types * Made Command a Generic. Using call again instead of Trigger but __call__ still returns a status object. wait_with_error_message in AsyncStatusBase now returns the value of the awaitable so that is it accessible from status.task.result() * Added _call to avoid AsyncStatus.wrap. Using AsyncStatus.wrap changes the return typing of __call__ from T to AsyncStatus. * __call__ now uses the same converters as signal. inspect.signature.bind is used to validate callback arguments against its signature before being passed to the callback * Moved converter creation to init * AsyncStatus no longer returns a value * In AsyncStatus, the value returned by the awaitable can be accessed using the value property * moved _wait_for to _utils for use by command and signal * Moved T_co to _utils. Using P from _utils in Command * Refactored MockCommandBackend to use execute_mock and more closely resemble MockSignalBackend * Update src/ophyd_async/core/_command.py Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> * Telling docs to ignore _utils.T_co * Refactored Command to use execute instead of __call__ * Removed last_return_value from SoftCommandBackend * Simplified execute with AsyncStatus.wrap * Added a converter for MockCommandBackend so it can return a value. In order to do this, MockCommandBackend needs to know the type to return. This necessitates a method in CommandBackend which can be used to retrieve the type Mock should use. A new abstract method get_return_type was added to CommandBackend to accomodate this need. SoftCommandBackend now stored the return type from the callback signature which can then be retrieved by this method. * Improving Mock test coverage * Testing out tango commands * Placeholder command connector * Made tango_command accept a DeviceProxy so a new DeviceProxy does not need to be created * wip * WIP. need to refactor device filler * testing device filler * Probably getting an error here because state is both an attribute and a command * First working draft of TangoCommandBackend. Required a refactor of _tango_transport especially get_python_type to properly handle return types. CommandProxy refactored to no longer use the command character enum. Use of this enum should be removed completely in the future. WIP, need to fix older tests for new command structure * Adding trigger for backwards compatibility with SignalX * Added trigger method to Command for backwards compatibility with SignalX. Trigger calls execute with no args and does not return a value. Refactored tango tests to not use SignalX. Removed some cmd tests. Refactored tests for get_python_type to use better expected types. * passes linting. Had to make copies of some utilities in tango.core._utils to avoid accessing private core._utils. * tigger calls execute with explicitly no arguments * fix linting * fixing device_filler * Fixing type-checking issues * linting * fixing docs * Fixed issue with tango command types not being converted correctly * Improving coverage * removed commented out code * Remove accidentally committed temp directory * removed get_return_value in favour of simply using the datatype attribute similar to SignalBackend. * removing get_return_type from tango backend * test now import make_converter from _soft_signal_backend so it does not need to be exported. * Leaving choice to raise NotConnectedError to the backend * trigger now calls the backend execute directly * Refactored Command.trigger() into a new TriggerableCommand class. This avoids having one class which covers the general case of typed args/return and the special case of no args. Trigger will fail if the backend expects arguments. It will not return any values from the backend. * Tango connector and device filler now infer None/None commands as TriggerableCommand instead of simply Command * Removed commented code * fixing tests * Added test for tango_command factory. factory can now return TriggerableCommand. * Added TriggerableCommand to tango_command return typehint * TangoCommandBackend now takes an inspect.Signature object instead of type[T] * linting * Fixing linting and type checking. Pyright said to replace T with object in tango_command because it only appears once in the signature * In procedural constructions, tango commands will work if the datatype is not specified. This fails for declarative constructions * fix type checking * linting * Added parameter type checking for TangoCommandBackend. Parameter types are stored in the backend as param_types, a list of SignalDatatype. Passing None as the parameter type bypasses the check in the procedural approach but fails in the declarative approach to signal declaration. * removed redundant use of _wait_for in Tango backend * removed _wait_for from tango._utils * Added some null protection to fill_child_signal and fill_child_command * Passing a signature to tango_command does not allow for type hinting. To obey type hinting we need to define a callable stub function and pass that as call_spec to tango_command * added tango_triggerable_command which returns TriggerableCommand. tango_command can no longer return a TriggerableCommand * datatype is not an appropriate attribute for Command as it is for signal because the whole function signature is relevant to the backend. This refactors Command and CommandBackend to instead store a signature attribute which is expected to be an inspect.Signature object. The TangoCommandBackend then validates according to this signature on connect. * linting and type checking * Fixing tests * Many minor improvements in response to Claude review * Added EPICS support for TriggerableCommand Also added ADR, and some typing tweaks * Fix PVI to use TriggerableCommand instead of SignalX * Fix remaining tests * Reproduce problem * Removed TestConfig from tango.testing. Defined two new protocols in tango._utils called AttributeConfig and CommandConfig to check objects against. This should prevent the need to import test machinery when using the tango backend. * Remove dead timeout param from CommandProxy.put; rename ADR to 0018 * Docs: document Command and TriggerableCommand, update tutorial and docstrings * Replace SignalX with TriggerableCommand in Eiger detector * Fix docs --------- Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> Co-authored-by: Tom Cobb <tom.cobb@diamond.ac.uk>
* Added a script that allows a Tango Device Server to be spawned in a subprocess. * Implemented simulated Tango devices to match the equivalent EPICS records. * Created demo classes that communicate with Tango devices and match the EPICS demo classes. * Linting and code formatting. * Improved movement logic of Tango demo motor. Added test of Tango demo * Cleaned up move functions in Tango demo class. * Resolved Tango system test failure. * Bind to localhost for tests. * Ensure clean up of resources when shutting down. * Cleanup code, remove unused Tango test classes. * Added 0.001 to ignore list in documentation conf file. * Test case fixes, use context manager in motor. * Linting and test coverage improvements. * Further tests for Tango server startup added. * DeviceProxy linting issue. * Removed hard coded string TRLs from the Tango demo and replaced with a consistent prefix variable. * Added Tango specific tutorial sections. * Make tnago subprocess handling more robust and update docs * Update docs/tutorials/implementing-devices.md * Update tango demo to use StandardMovable The test command goes in OneOfEverythingTangoDevice * Fix type checking --------- Co-authored-by: Alan Greer <alan.greer@diamond..ac.uk> Co-authored-by: Alan Greer <alan@alangreer.co.uk> Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> Co-authored-by: Tom Cobb <tom.cobb@diamond.ac.uk>
This allows downstream users to use DeviceProcessor to make modified versions of 'init_devices'. A test has been added with one such case. Signed-off-by: Sofia Donato Ferreira <sofia.ferreira@lnls.br>
* Add missing StandardMovable typing * Add to tango epics demo motors * Add test * Add id for test and increase velocity * add typing to test * connect in parallel * Add instanst=False test * Update flakey sim motor stop test --------- Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>
* Simplify failing test * Reduce timeout for CI * Slacken off timing * Slacken once more
* fix: accept enum in PvaCommandBackend * tests: add tests to check enum accepted by backends and float rejected * fix: change signal x record types to bo from ao * chore: remove useless RE fixture argument from new tests
…eLogic (#1300) * Update typing to allow for None for calculate_timeout for MovableLogic * Update docs for default timeout * Update default calculate_timeout back to DEFAULT_TIMEOUT
…hecks (#1303) * perf: cache setattr dispatch in Device to avoid repeated isinstance checks Pre-populate a per-instance _setattr_methods dict in __new__ with known non-Device attributes, and lazily cache the dispatch function for all other attributes on first set. Also fixes a bug where setting a Device child to None would not deregister it from _child_devices, causing connect() to attempt to connect signals with no source. * tests: assert optional device not in child_devices * tests: assert against public api instead of private attribute
* Update typing to allow for None for calculate_timeout for MovableLogic * Update docs for default timeout * Update timeout to be whole move for MovableLogic * Update StandardMovable timeout to apply for whole move * Update to __call__ * Add tests for MoveTimeout * MoveTimeout creation time * Update timeout to CalculatedTimeout * Rename CalculatorTimeout to TimeoutCalculator
* Added documentation note recognizing that Tango attribute and command names may contain special characters that are invalid python syntax. * Rephrasing. Signals can't be because is used in annotations.
…-template to v5.2.0 (#1304) * Update dependency https://github.com/DiamondLightSource/python-copier-template to v5.2.0 * Update ci.yml * Ignore deprecation warning until fixed upstream --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com> Co-authored-by: Tom Cobb <tom.cobb@diamond.ac.uk>
* Renamed TangoDevice.trl to TangoDevice._trl. Removed unused TangoDevice.proxy. Added TangoDevice.get_trl which returns _trl. Added TangoDevice.get_proxy which returns the Tango DeviceProxy object used by the connector. * linting * type checking * type checking
* First draft for callable backed soft signals * Fixed docs. setter in SoftSignalBackend not handles Any * Modified soft signal factories * Reviewer changes * Using maybe_await from bluesky.utils * variable name change * removed some unnecessary checks of _getter * Corrected typehinting of setter * Setpoint and _setter now take converted values * put setpoint in set_value * renamed explanation doc 20. * fixing docs * linting * fixed error. soft_signal_r_and_setter is read-only and does not take a setter * added how-to-use-soft-signals document * linting * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fixed case D example code * The setpoint should not be set again in set_value. This causes _update_value_from_getter to overwrite the setpoint. Removed heterogeneous type tests since we want to have homogeneous setter/signal data types. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fixing docs * removed some comments I accidentally committed --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Tom C (DLS) <101418278+coretl@users.noreply.github.com>
coretl
approved these changes
Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1283
Adds motor and velocity limit validation to
Trajectory.from_sliceAdds validation to
Trajectory.from_slicethat raisesValueErrorbefore any trajectory computation if the slice would violate hardware constraints; motor position limits or max velocity.Tests added for both checks across x and y axes.