Lockbox API changes (from #349)

[lneuhaus]

Here is a copy-paste from issue #349:

But also, I am proposing below a few modification to the Lockbox API for consistency. Moreover, I realize that the unittetsts for these functionalities are quite limited, I will try to add some more unittests as well.

lock_async(): # I propose to add the _async for consistency
      """
      Launches the full lock sequence and returns immediately.
      **Proposed return value**
      Returns a Future that evaluates to True if lock was successful, False otherwise
      """

lock(): # It looks like this function doesn't exist at the moment
        """
        Same as lock_async except it only returns a boolean at the end of the lock sequence
        """

sleep_while_locked(time_to_sleep): # keep it (should be programmed with an Event())
        """
        wait for time_to_sleep and returns True if no unlock occured.
        Otherwise, returns False as soon as unlock occurs.
        """
lock_until_locked_async(**kwds): # _async is there for consistency.
        """
         runs the full lock sequence as many times as necessary.
        **Proposed return value**
        Returns a Future that evaluates to True when the lock is acquired.

lock_until_locked(**kwargs): # should add a timeout
        """
        runs the full lock sequence as many times as necessary and 
        only returns once locked.
        **Proposed return value**
        True if lock was succesful before timeout
        False otherwise.
        """

#==================================================#
#     If I understand correctly, all the following RElock_blabla are equivalent to
#     if not self.is_locked_and_final():
#            self.lock_blabla
#    I would vote to remove these functions, since the two lines above are more explicit...
#==================================================# 
relock(**kwds): # If we keep it, should be renamed relock_async()
        """
        If not locked, runs the full lock sequence once. 
        **current behavior**
        Returns True if locked at the beginning, False otherwise
        **consistent behavior would be**
        Returns a future object that evaluates to True if locked at the end, False otherwise
        """
relock_until_locked_async(**kwds):
        """
        Same as lock_until_locked_async, but does nothing if already locked
        """

relock_until_locked(**kwds):
        """
        Same as lock_until_locked, but returns immediately if already locked ???
        """

[SamuelDeleglise]

So what's your opinion on these ?

[lneuhaus]

Sorry for the delay.

So we agree that ANYTHING_async returns a future, and ANYTHING awaits the result and returns after that, right? I agree that all functions should have these two versions.

• I agree on lock/lock_async. There is however a function lock at the moment, right?
• We could add two arguments to lock: timeout=0 (0=no timeout) and retry_times=1 (0=infinite times). These would make lock_until_locked obsolete. We could however add these on top (basically calling lock_async(retry_times=0)
• sleep_while_locked should be useful, I agree. Can this not be a two-liner in python3? In any case, either it saves many lines, or it serves as a nice example.

On top of this, I find it useful to allow passing arguments to lock() that modify the final locking stage, e.g. lock(setpoint=-0.5, gain=2). There is currently some kind of overcomplicated code with final_stage that was implemented around this, i.e. final_stage is a duplicate of the last element of stages with its properties overwritten by the arguments passed to lock. This way, relock() would be able to use this modified setpoint, while lock would execute the default locking sequence. If you have the time to debug it after changing, I would be okay with the removal of all the special treatment of final_stage and let the last stage be the last element of stages. I would however still allow to pass arguments such as setpoint to lock() which then simply overwrite the values in the default stage. If a user wants to return to a previous "default" sequence, the way to go would be to load a stored locking sequence. If you strongly dislike that, I would favor having a convenience function, e.g. lock_modify_setpoint, but I think this overcomplicates things.

[SamuelDeleglise]

Absolutely, you are right about the _async postfix and the return types. At the moment, there is a lock function but it is returning immediately as you would expect from lock_async instead of blocking until the lock is effective.

for sleep_while_locked, I would probably use the wait function that's defined in async_utils with a timeout. And create a coroutine that returns only when the cavity unlocks by awaiting an unlock event https://docs.python.org/3/library/asyncio-sync.html#asyncio.Event

I can do these updates relatively simply, including removing the final_stage stuff as long as it's only the python3-only branch. Otherwise, I have the feeling I will have to duplicate a lot of stuffs.

By the way, unittests seem to be fine again on python2.7 on develop0.9.3

[lneuhaus]

Sounds good to me, I do not think we need the API changes in Python 2.