Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to PVPostionerSoftDone #1005

Merged

Conversation

gfabbris
Copy link
Collaborator

Main proposed changes:

  • Keeps setpoint and readback as normal EpicsSignal. But now, changes to the setpoint are sent to the target through a subscription.
  • Forces the done signal to be False when the device is moved.
  • Changes the default value of use_target to False. There is one disadvantage of using the target signal: if you change the setpoint "manually" in EPICS (and the device moves to the new setpoint), the Bluesky device will not know that it is inposition because target was not updated. Therefore, I recommend to set use_target=True only when EPICS changes the setpoint value while ramping (like in the Lakeshore 340).
  • Removes python 3.8 from unit tests. It is leading to an import error, see error.

@gfabbris gfabbris added the bug label Jul 22, 2024
@gfabbris gfabbris requested a review from prjemian July 22, 2024 19:32
@gfabbris gfabbris self-assigned this Jul 22, 2024
@prjemian
Copy link
Contributor

I'm looking at PR #984 (April 2024) which has other changes for PVPositionerSoftDone. This PR was posted (July 2024) after that one

@gfabbris
Copy link
Collaborator Author

@prjemian: Could you look at this PR before merging the other one? This is actually modifying the 916-PVPositionerSoftDone-update brach. We could chat about it in tomorrow's meeting.

@@ -98,7 +98,7 @@ jobs:
strategy:
matrix:
python-version:
- "3.8"
# - "3.8" # TODO: Breaking
Copy link
Contributor

Choose a reason for hiding this comment

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

We're really close to EOL for Py3.8. This would push the entire repo to drop it. What breaks for Py3.8 now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I came across this import error.

@prjemian
Copy link
Contributor

Py3.8 build failed with this message:
==================================== ERRORS ====================================
_______ ERROR collecting apstools/callbacks/tests/test_440_specwriter.py _______
ImportError while importing test module '/home/runner/work/apstools/apstools/apstools/callbacks/tests/test_440_specwriter.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/_pytest/python.py:493: in importtestmodule
    mod = import_path(
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/_pytest/pathlib.py:582: in import_path
    importlib.import_module(module_name)
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:961: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:961: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:961: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:975: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:671: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:843: in exec_module
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
apstools/__init__.py:6: in <module>
    from setuptools_scm import get_version
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools_scm/__init__.py:8: in <module>
    from ._config import DEFAULT_LOCAL_SCHEME
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools_scm/_config.py:17: in <module>
    from ._integration.pyproject_reading import (
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools_scm/_integration/pyproject_reading.py:9: in <module>
    from .setuptools import read_dist_name_from_setup_cfg
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools_scm/_integration/setuptools.py:10: in <module>
    import setuptools
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools/__init__.py:21: in <module>
    from .dist import Distribution
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools/dist.py:29: in <module>
    from . import _entry_points
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools/_entry_points.py:6: in <module>
    from jaraco.text import yield_lines
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools/_vendor/jaraco/text/__init__.py:12: in <module>
    from jaraco.context import ExceptionTrap
../../../micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/setuptools/_vendor/jaraco/context.py:17: in <module>
    from backports import tarfile
E   ImportError: cannot import name 'tarfile' from 'backports' (/home/runner/micromamba/envs/anaconda-test-env-py-3.8/lib/python3.8/site-packages/backports/__init__.py)
=========================== short test summary info ============================
ERROR apstools/callbacks/tests/test_440_specwriter.py
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.[23](https://github.com/gfabbris/apstools/actions/runs/10044498274/job/27760058119#step:15:24)s ===============================

@prjemian
Copy link
Contributor

@gfabbris Looks like there are changes in aps_undulator.py in this branch that are already in the main branch. Can you update your branch so its actual changes are clearer?

@gfabbris
Copy link
Collaborator Author

@prjemian: Now that you merged main into the 916-PVPositionerSoftDone-update branch f6c366d the undulator stuff should be identical, but I didn't find a way to force this PR to recheck the files changed. Should I close this PR and open a new one?

@prjemian
Copy link
Contributor

Is that easier? It would help me to catch up.

@gfabbris
Copy link
Collaborator Author

It seems to me that it is easier, and, maybe more importantly, safer.

@gfabbris gfabbris marked this pull request as draft August 26, 2024 22:07
@gfabbris gfabbris merged commit afe2cdc into BCDA-APS:916-PVPositionerSoftDone-update Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants