Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

WIP: Add fixtures for core tests #1474

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

cpt-majkel
Copy link

@cpt-majkel cpt-majkel commented Jan 15, 2021

Related to #1421

Sets stop flag on stop for each element in multiple elements movement. It results in correct (interrupted) state of the moveables and does not allow to apply backlash.

Copy link
Collaborator

@reszelaz reszelaz left a comment

Choose a reason for hiding this comment

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

Thanks @cpt-majkel for the PR. I still had no time to test it but I have a small suggestion already - see my inline comment. BTW, I think we could easily add an automatic test on the core level (without the need to use Tango).

@@ -358,6 +358,8 @@ def stop(self):
self.debug("Stopping %s %s", ctrl.name,
[e.name for e in elements])
try:
for el in elements:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move all the loop before of the try..except. Here we are just setting an internal flag, nothing can go wrong.

Copy link
Author

Choose a reason for hiding this comment

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

Moved

@reszelaz reszelaz added the bug label Jan 18, 2021
@reszelaz reszelaz added this to the Jan21 milestone Jan 18, 2021
@cpt-majkel cpt-majkel changed the title Set stop flags for each element in multiple elements movement WIP:Set stop flags for each element in multiple elements movement Jan 27, 2021
@reszelaz
Copy link
Collaborator

reszelaz commented Mar 24, 2021

@cpt-majkel, I reviewed the PR and it is looking very good. Especially the testing part with the fixtures. But I still have some questions about the testing part. Better let's organize some online meetings to discuss them.

Meanwhile, in order to not block the integration of the fix and the Jan20 release, I will proceed to manually merge the very first three commits (1bca450, f573fcc and c47263e).
The tests introduced in the subsequent commits confirm the fix for the cases of a motor and a motor group (I have also run it locally) and I have manually tested it for pseudo motors. Everything looks ok.
Many thanks!

@reszelaz reszelaz modified the milestones: Jan21, Jul21 Mar 24, 2021
@reszelaz reszelaz changed the title WIP:Set stop flags for each element in multiple elements movement WIP: Add fixtures for core tests Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants