rework frappy to be a device instead of a command

+ improve proposed config mechanism

commands.py

@@ -29,83 +29,6 @@ from servicemanager import FrappyManager, SeaManager
 SERVICES = FrappyManager.services
 
 
-def all_info(all_cfg, prefix='currently configured: '):
-    info = []
-    addkwd = False
-    for srv in SERVICES:
-        cfginfo = all_cfg.get(srv)
-        if cfginfo is None:
-            addkwd = True
-        elif addkwd:
-            info.append('%s=%r' % (srv, cfginfo))
-        else:
-            info.append(repr(cfginfo))
-    return f"{prefix}frappy({', '.join(info)})"
-
-
-def frappy_start(**services):
-    """start/stop  frappy servers
-
-    for example: frappy_start(main='xy', stick='')
-    - restart main server with cfg='xy'
-    - stop stick server
-    - do not touch addons server
-
-    in addition, if a newly given cfg is already used on a running server,
-    this cfg is removed from the server (remark: cfg might be a comma separated list)
-    """
-    frappy_config = session.devices.get('frappy_config')
-    for service in SERVICES:
-        if services.get(service) == '':
-            seaconn = session.devices.get(f'se_sea_{service}')
-            if seaconn and seaconn._attached_secnode:
-                seaconn.communicate('frappy_remove %s' % service)
-    used_cfg = {}
-    all_cfg = {}
-    new_cfg = []
-    remove_cfg = []
-    for service in reversed(SERVICES):
-        secnode = session.devices.get('se_' + service)
-        cfginfo = services.get(service)
-        if cfginfo is not None:
-            if cfginfo:
-                new_cfg.append((service, secnode, cfginfo))
-            else:
-                remove_cfg.append(secnode)
-            if secnode:
-                secnode('')
-        if secnode:
-            all_cfg[service] = secnode.get_info()
-
-        # check cfg is not used twice
-        for cfg in (cfginfo or '').split(','):
-            cfg = cfg.strip()
-            if cfg:
-                prev = used_cfg.get(cfg)
-                if prev:
-                    raise ValueError('%r can not be used in both %s and %s' % (cfg, prev, service))
-                used_cfg[cfg] = service
-
-    if new_cfg:
-        for service, secnode, cfginfo in new_cfg:
-            nodename = 'se_' + service
-            if not secnode:
-                AddSetup('frappy_' + service)
-                secnode = session.devices[nodename]
-            secnode(cfginfo)
-            all_cfg[service] = secnode.get_info()
-            CreateDevice(nodename)
-        cleanup_defunct()
-        CreateAllDevices()
-    if frappy_config:
-        frappy_config.set_envlist()
-    else:
-        applyAliasConfig()
-    for secnode in remove_cfg:
-        secnode.disable()
-    return all_cfg
-
-
 @usercommand
 def set_se_list():
     fc = get_frappy_config()
@@ -113,61 +36,6 @@ def set_se_list():
         fc.set_envlist()
 
 
-@usercommand
-@helparglist('main [, stick [, addons]]')
-def frappy(*args, main=None, stick=None, addons=None, force=False):
-    """(re)start frappy server(s) with given configs and load setup if needed
-
-    - without argument: list running frappy servers, restart failed frappy servers
-    - frappy('<cfg>'): if available, the standard stick is added too
-    - frappy(''): the stick is removed too
-    - addons are not changed when not given
-    - frappy(main='<cfg>')      # main cfg is changed, but stick is kept
-    - frappy('restart')         # restart all frappy servers
-    - frappy(stick='restart')   # restart stick frappy server
-    """
-    fc = get_frappy_config()
-    if not fc:
-        return
-    stickarg = stick
-    confirmed = SeaManager().get_cfg(config.instrument, 'sea', True).split('/', 1)[0]
-    if args:
-        if main is not None:
-            raise TypeError('got multiple values for main')
-        main = args[0]
-        if len(args) == 1:  # special case: main given as single argument
-            if main == 'restart':
-                stick = 'restart'
-                addons = 'restart'
-            elif stick is None:  # auto stick
-                if main == '':
-                    stick = ''  # remove stick with main
-                else:
-                    stickcfg = main + 'stick'
-                    if FrappyManager().is_cfg(config.instrument, 'stick', stickcfg):
-                        # if a default stick is available, start this also
-                        stick = stickcfg
-                    else:
-                        stick = ''  # remove stick when main has changed
-        else:
-            if stick is not None:
-                raise TypeError('got multiple values for stick')
-            stick, *alist = args[1:]
-            if alist:
-                if addons is not None:
-                    raise TypeError('got multiple values for addons')
-                addons = ','.join(alist)
-    elif main is None and stick is None and addons is None:  # bare frappy() command
-        fc.show_config(fc.check_services())
-        return
-    if confirmed and confirmed != main and main not in (None, 'restart') and not force:
-        session.log.warning('%r is plugged to the cryostat control rack', confirmed)
-        cmd = all_info({'main': main, 'stick': stickarg, 'addons': addons}, '')[:-1] + ', force=True)'
-        session.log.warning(f'if you are sure, use: %s', cmd)
-        raise TypeError('refuse to override plugged device')
-    fc.show_config(fc.start_services(main, stick, addons))
-
-
 @usercommand
 def frappy_main(*args):
     raise NameError('frappy_main(<cfg>) is no longer avaiable, use frappy(<cfg>) instead')
@@ -204,10 +72,3 @@ def frappy_list(service=None):
 
     FrappyManager().do_listcfg(config.instrument, service or 'main', prt)
     session.log.info('\n%s', '\n'.join(content))
-
-
-@usercommand
-def frappy_changed():
-    fc = get_frappy_config()
-    if fc:
-        fc.changed()

devices.py

@@ -27,18 +27,17 @@ SEC Node with added functionality for starting and stopping frappy servers
 connected to a SEC node
 """
 
-import time
 import threading
 from nicos import config, session
 from nicos.core import Override, Param, Moveable, status, POLLER, SIMULATION, DeviceAlias, \
     Device, anytype, listof
-from nicos.devices.secop.devices import SecNodeDevice
+from nicos.devices.secop.devices import SecNodeDevice, NicosSecopClient
 from nicos.core.utils import USER, User, createThread
 from nicos.services.daemon.script import RequestError, ScriptRequest
 from nicos.utils.comparestrings import compare
 from nicos.devices.secop.devices import get_attaching_devices
 from nicos.commands.basic import AddSetup, CreateAllDevices, CreateDevice
-from servicemanager import FrappyManager
+from servicemanager import FrappyManager, SeaManager
 
 SERVICES = FrappyManager.services
 
@@ -107,9 +106,9 @@ def all_info(all_cfg, prefix='currently configured: '):
 
 def get_frappy_config():
     try:
-        return session.devices['frappy_config']
+        return session.devices['frappy']
     except KeyError:
-        session.log.error("'frappy_config' is not available - 'frappy' setup is not loaded")
+        session.log.error("the frappy device is not available - 'frappy' setup is not loaded")
         return None
 
 
@@ -155,16 +154,17 @@ class FrappyConfig(Device):
     def handle_notifications(self):
         controller = session.daemon_device._controller
         while True:
-            self._trigger_change.wait()
+            # we do not wait for ever here, because there might be changes
+            # on an unconnected service
+            self._trigger_change.wait(15)
             self._trigger_change.clear()
-            time.sleep(2)
+            while self._trigger_change.wait(2):  # triggered again within 2 sec
-            if self._trigger_change.is_set():
+                self._trigger_change.clear()
-                continue
             try:
                 cfgs = self.check_services()
-                guess_info = self.to_consider(cfgs)
+                changes, state, remarks = self.to_consider(cfgs)
-                if (cfgs, guess_info) != self._previous_shown and (guess_info or not self._servers_loaded):
+                if state != self._previous_shown and changes:
-                    cmd = 'frappy_changed()'
+                    cmd = 'frappy.has_changed()  # inserted automatically when frappy or sea servers changed'
                     controller.new_request(ScriptRequest(cmd, None, User('guest', USER)))
             except RequestError as e:
                 session.log.error(f'can not queue request {e!r}')
@@ -174,16 +174,18 @@ class FrappyConfig(Device):
         
         for a potential "please consider calling frappy(...)" message
         """
-        guess_info = {}
+        error, proposed, state, remarks = FrappyManager().get_server_state(config.instrument, cfgs)
-        for service, guess in FrappyManager().guess_cfgs(config.instrument, cfgs).items():
+        changes = dict(proposed)
-            proposed = guess.get('proposed')
+        for service, guess in proposed.items():
-            if proposed:
+            if guess is True:
-                guess_info[service] = proposed
+                changes.pop(service)
-            else:
+        disconnected = set()
-                missing = guess.get('missing')
+        for service, info in cfgs.items():
-                if missing:
+            if info == '<disconnected>':
-                    guess_info[service] = [m + '?' for m in missing]
+                disconnected.add(service)
-        return guess_info
+                if not changes.get(service):
+                    changes[service] = ''
+        return changes, (proposed,) + state, remarks
 
     def check_services(self):
         cfgs = {}
@@ -209,7 +211,10 @@ class FrappyConfig(Device):
             if cfg == '':
                 seaconn = session.devices.get(f'se_sea_{service}')
                 if seaconn and seaconn._attached_secnode:
-                    seaconn.communicate('frappy_remove %s' % service)
+                    try:
+                        seaconn.communicate('frappy_remove %s' % service)
+                    except Exception:
+                        pass
         used_cfg = {}
         all_cfg = {}
         new_cfg = {}
@@ -264,21 +269,89 @@ class FrappyConfig(Device):
         self._cache.put(self, 'config', all_cfg)
         return all_cfg
 
-    def show_config(self, allcfg):
+    def __call__(self, *args, main=None, stick=None, addons=None, force=False):
-        guess_info = self.to_consider(allcfg)
+        """(re)start frappy server(s) with given configs and load setup if needed
-        # remove 'frappy_changed()' commands in script queue
+
+        - without argument: list running frappy servers, restart failed frappy servers
+        - frappy('<cfg>'): if available, the standard stick is added too
+        - frappy(''): the stick is removed too
+        - addons are not changed when not given
+        - frappy(main='<cfg>')      # main cfg is changed, but stick is kept
+        - frappy('restart')         # restart all frappy servers
+        - frappy(stick='restart')   # restart stick frappy server
+        """
+        stickarg = stick
+        confirmed = SeaManager().get_cfg(config.instrument, 'sea', True).split('/', 1)[0]
+        if args:
+            if main is not None:
+                raise TypeError('got multiple values for main')
+            main = args[0]
+            if len(args) == 1:  # special case: main given as single argument
+                if main == 'restart':
+                    stick = 'restart'
+                    addons = 'restart'
+                elif stick is None:  # auto stick
+                    if main == '':
+                        stick = ''  # remove stick with main
+                    else:
+                        stickcfg = main + 'stick'
+                        if FrappyManager().is_cfg(config.instrument, 'stick', stickcfg):
+                            # if a default stick is available, start this also
+                            stick = stickcfg
+                        else:
+                            stick = ''  # remove stick when main has changed
+            else:
+                if stick is not None:
+                    raise TypeError('got multiple values for stick')
+                stick, *alist = args[1:]
+                if alist:
+                    if addons is not None:
+                        raise TypeError('got multiple values for addons')
+                    addons = ','.join(alist)
+        elif main is None and stick is None and addons is None:  # bare frappy() command
+            self.show_config(self.check_services(), True)
+            return
+        if confirmed and confirmed != main and main not in (None, 'restart') and not force:
+            session.log.warning('%r is plugged to the cryostat control rack', confirmed)
+            cmd = all_info({'main': main, 'stick': stickarg, 'addons': addons}, '')[:-1] + ', force=True)'
+            session.log.warning(f'if you are sure, use: %s', cmd)
+            raise TypeError('refuse to override plugged device')
+        self.show_config(self.start_services(main, stick, addons))
+
+    def show_config(self, allcfg, show_server_state=False):
+        changes, state, remarks = self.to_consider(allcfg)
+        if show_server_state == 'auto':
+            show_server_state = state != self._previous_shown
+        if show_server_state:
+            proposed, frappycfgs, seacfgs = state
+            rows = [['server', 'frappy', 'sea', '']]
+            for key, remark in remarks.items():
+                rows.append([key if key in ('main', 'stick') else 'addons',
+                             frappycfgs.get(key, ''), seacfgs.get(key, ''), remark])
+            wid = [max(len(v) for v in column) for column in zip(*rows)]
+            # insert title underlines
+            rows.insert(1, ['-' * w for w in wid[:-1]] + [''])
+            for row in rows:
+                session.log.info('%s', '  '.join(v.ljust(w) for w, v in zip(wid, row)))
+            session.log.info('')
+        # remove 'frappy.has_changed()' commands in script queue
         controller = session.daemon_device._controller
-        controller.block_requests(r['reqid'] for r in controller.get_queue() if r['script'] == 'frappy_changed()')
+        controller.block_requests(r['reqid'] for r in controller.get_queue()
-        self._previous_shown = allcfg, guess_info
+                                  if r['script'].startswith('frappy.has_changed()'))
+        self._previous_shown = state
         session.log.info(all_info(allcfg))
 
-        if guess_info:
+        if changes:
-            info = all_info(guess_info, '')
+            info = all_info(changes, 'proposed cfg changes: ')
-            session.log.warning('please consider to call:')
             session.log.info(info)
+            session.log.warning('please consider to call: frappy.update() for doing above changes')
             if '?' in info:
                 session.log.warning("but create cfg files first for items marked with '?'")
 
+    def update(self):
+        changes = self.to_consider(self.check_services())[0]
+        self.show_config(self.start_services(**changes))
+
     def initial_restart_cfg(self, service):
         """get cfg for (re)start of the service
 
@@ -290,7 +363,7 @@ class FrappyConfig(Device):
         if self._servers_loaded:
             return None
         if self._initial_config is None:
-            success = True
+            # we do this only once for all services
             fm = FrappyManager()
             running = fm.get_cfg(config.instrument, None)
             cache = self._getCache()
@@ -309,30 +382,15 @@ class FrappyConfig(Device):
                     cfgs['stick'] = running_stick
                 else:
                     cfgs.pop('stick', None)
-            for serv, guess in fm.guess_cfgs(config.instrument, cfgs).items():
+            error, proposed, state, remarks = fm.get_server_state(config.instrument, cfgs)
-                ok = guess.get('ok', [])
+            self._initial_config = proposed
-                for proposed in guess.get('proposed', []):
+            if not error:
-                    if isinstance(proposed, str):  # start only when ambiguous
+                self._previous_shown = state  # otherwise the server state will be shown on startup
-                        ok.append(proposed)
-                    else:
-                        success = False
-                if ok:
-                    cfgs[serv] = ','.join(ok)
-            if not success:
-                cfgs = {}
-            result = {}
-            for serv, cfg in cfgs.items():
-                runcfg = running.get(serv)
-                if runcfg != cfg:
-                    result[serv] = cfg
-                elif runcfg:
-                    result[serv] = True  # restart not needed as cfg has not changed
-            self._initial_config = result
         return self._initial_config.get(service)
     
-    def changed(self):
+    def has_changed(self, show_server_state='auto'):
         self._servers_loaded = True
-        self.show_config(self.check_services())
+        self.show_config(self.check_services(), show_server_state)
 
     def remove_aliases(self):
         for meaning in self.meanings:
@@ -485,15 +543,13 @@ class FrappyNode(SecNodeDevice, Moveable):
         if mode == SIMULATION or session.sessiontype == POLLER:
             super().doInit(mode)
         else:
-            fc = session.devices.get('frappy_config')
+            fc = session.devices.get('frappy')
             if fc:
                 cfg = fc.initial_restart_cfg(self.service)
                 if isinstance(cfg, str):  # may also be None or True
                     self.restart(cfg)
                 if cfg is None:  # None means: server is not running, and does not need to be restarted
-                    # connect in background, as the server might be started later
+                    self._disconnect()
-                    createThread('connect', self._connect)
-                    # TODO: check if it is not better to add a try_period argument to SecNode._connect()
                     return
             try:
                 self._connect()
@@ -594,9 +650,15 @@ class FrappyNode(SecNodeDevice, Moveable):
             self._cache.put(self, 'value', cfg)
         self._setROParam('target', cfg)
 
+    def _disconnect(self):
+        super()._disconnect()
+        self._setROParam('target', '')
+
     def get_info(self):
         result = self.doRead() or ''
         code, text = self.status()
+        if not result and self.target:
+            return '<disconnected>'
         if code == status.OK or result == '':
             return result
         if (code, text) == (status.ERROR, 'reconnecting'):

setups/frappy.py

@@ -4,10 +4,8 @@ group = 'optional'
 modules = ['nicos_sinq.frappy_sinq.commands']
 
 devices = dict(
-    frappy_config = device(
+    frappy = device(
         'nicos_sinq.frappy_sinq.devices.FrappyConfig',
-        # frappy_config does not need to be visible
-        visibility = [],
         # the possible SECoP connections
         nodes = ['se_main', 'se_stick', 'se_addons'],
         #
@@ -64,5 +62,5 @@ printinfo("  frappy('')                           # remove main SE apparatus")
 printinfo("  frappy()                             # show the current SE configuration")
 printinfo("=======================================================================================")
 set_se_list()
-frappy_changed()
+frappy.has_changed()
 '''