ufo-kit / concert

Concert is a light-weight experiment control system
http://ufo.kit.edu
GNU Lesser General Public License v3.0
13 stars 4 forks source link

behaviour of move function for Motors #507

Open MarcusZuber opened 1 year ago

MarcusZuber commented 1 year ago

The current implementation of concert.devices.motors.base.move(delta) reads the current motor position and then issues a new motion shifted by delta.

I was thinking about changing the move relatively to the current position target value (if available).

This would mostly result in these changes:

Please comment!

tfarago commented 1 year ago

This definitely makes sense!

sgasilov commented 1 year ago

Hello Marcus, hello Tomas,

I presume "...issues a new motion shifted by delta..." means commanding new absolute position. I'm also not a Tango expert so I don't know how it interacts with your low-level drivers and motion controllers. However, I'm certain that there is something wrong with your argument about "accumulated errors" and "relative moves"... Motion controller and lower level driver should take care about repeatability and accuracy, not Concert. Changing behavior of the "move" command just because one of your stages drifts with time doesn't appear to be the right solution.

I have a distinct impression that you guys have been mixing layers. Another example would be reorganization of Experiment class and new wrappers just for the sake of accommodating all different logical data acquisition sequences which you have.

To sum up, Concert description states "Lightweight control system". I propose to keep it that way.

tfarago commented 1 year ago

Motion controller and lower level driver should take care about repeatability and accuracy, not Concert.

I agree with this one. And I don't like mixing layers. If our controllers are stupid, let's make them not stupid. If not, we must come to an agreement here before this proceeds.

MarcusZuber commented 1 year ago

It is not really mixing layers. Concert features to read the target value and the measured value. (The task of the controller is to match the current value with the target.) The question here is, if a move should be relative to the target or the measured value. Currently we use the measured value as reference, which (by design) is noisy.

sgasilov commented 1 year ago

While I can think of a specific cases when moving based on the desired position can help (first things come to mind are grating stepping or rocking of an analyzer crystal) in general moves must happen relatively to the actual position, not some desired position which wasn't reached because of something or was lost due to something.

I did not even realize that Concert can read both target value and measured value or are we again mixing Tango and Concert? In EPICS there is position feedback which can change all the time and "Set point" PV which is what you call desired target position. In the mid-layer EDC which interface our EPICS (and not only - we have now PI stages to which we talk directly via sockets) to the Concert motor we always take the actual physical position of the motor.