sardana-org / sardana

Moved to GitLab: https://gitlab.com/sardana-org/sardana
39 stars 52 forks source link

Revise macro protection mechanism #1325

Open tiagocoutinho opened 4 years ago

tiagocoutinho commented 4 years ago

When writing a macro class in sardana it is possible to define the methods on_abort(self) and on_stop(self) to recover or handle error/stop cases respectively.

While this seems useful at first glance, it has some limitations:

The original motivation was that we (wrongly) assumed that asking the macro developer to write try/catch or context managers would be too complicated and we wanted to provide an easier interface. Turns out that, as usual, the solution provided by the language is a much better mechanism then a local invention based on wrong assumptions :-)

IMHO the python try/except and context manager mechanisms are easier and more powerful mechanisms and therefore better suited to handle these scenarios.

We could provide helpers and/or examples on most common cases: Here is an example of a context manager that ensures the motor comes back to the original position:

from contextlib import contextmanager 
from sardana.macroserver.macro import macro

@contextmanager
def ensure_motor_position(motor):
    initial_position = motor.position
    try:
        yield initial_position
    finally:
        motor.move(initial_position)

@macro()
def my_macro(self):
    th = self.getMoveable('th')
    with ensure_motor_position(th):
        th.move(55)
        1 / 0   # ups! something wrong here. No worries, th will always go back to start

Therefore, I propose to:

reszelaz commented 4 years ago

We have briefly discussed this issue on the last follow-up meeting, and later talking with @teresanunez the POV of DESY is:

on_stop, on_abort functions: this is also a feature which was asked for by our BL scientists. It would be a problem for us, if this disappears.

tiagocoutinho commented 4 years ago

ok, we could keep on_stop and on_abort but change the implementation to something like:

class Macro:
    def _run(self):
        with macro_guard(self):
            return self.run()

(disclaimer: I am making a suggestion before seeing the code)

My point is that the callbacks would be called inside the run procedure. This way it would be safe to make any macro API calls inside de callbacks.