ISISComputingGroup / lewis-ess

Let's write intricate simulators!
GNU General Public License v3.0
21 stars 19 forks source link

Separate Adapter and Interface #232

Closed MichaelWedel closed 7 years ago

MichaelWedel commented 7 years ago

For achieving multiple adapters and decoupled network IO, it is necessary to separate the concepts "adapter" and "interface" also in code.

This has several implications, one of the more serious ones being that existing device-interfaces will have to inherit from a new base class.

Apart from that there are a number of smaller changes (binding Cmd/PV needs different targets). We also need to make decisions regarding who creates what. Should the factory as it is now create the Interface and pass it to an Adapter? Where does the adapter come from? One idea I've had was something along these lines:

class EpicsInterface(Interface):
    adapter = EpicsAdapter

Any other ideas?

MikeHart85 commented 7 years ago

Yes, SimulationFactory is probably the best place for it, for now. But I think our construction / initialization / environment setup scheme is still quite awkward even after we introduced that factory.

I think we would benefit from mimicking Qt a bit and having a central Application class (not necessarily by that name) which takes argc/argv and has a run method. All this awkward construction and environment stuff could then live there. scripts/run.py could be simplified drastically or entirely consumed by it, refactored into compact private methods. Everything in SimulationFactory could probably become (a) private method(s) within such a class.

I could imagine scripts/run.py (or just lewis.py if that goes away) turning into something like:

import sys
from lewis.core import LewisApp

if __name__ == '__main__':
    app = LewisApp(sys.argv)
    ret = app.run()
    sys.exit(ret)

For the other thing, I think I would prefer...

class EpicsInterface(Interface):
    def get_adapter_type(self):
        return EpicsAdapter

... since it better signals immutability within this type of interface.

Is this sort of thing...

class Foo(Bar):
    class_attribute = some_value

... a good pattern that we should use in general?

We started using it for Interfaces, because it looks nice, syntactically, in a declarative "this is what I want my interface to be" kind of way. Since then, it has crept into other parts of the framework (and even user submitted devices, see SimulatedJulabo). This is a bit dangerous, because a class attribute is not an instance attribute, and you can get very unexpected behaviour if you forget or just don't notice that you're accessing a class attribute (since usage looks the same).

For example:

>>> class Foobar: 
...     value = []
>>> a, b = Foobar(), Foobar()
>>> a.value.append(5)
>>> b.value
[5]

We should be very careful when/where/why/how we use class attributes.