coburnw / python-vxi11-server

A VXI-11 Instrument Server written in Python
27 stars 14 forks source link

correct Usage of Class Variables ? #20

Open ulda opened 3 years ago

ulda commented 3 years ago

Hi Coburn I've just fallen over an incorrect usage of class variables in my own code and I remembered that there is something in python-vxi1-server, too.

Example in our code:

class DeviceRegistry(object):
    _next_device_index = 0
    _registry = {}

    def __init__(self):
        pass

    def register(self, name, device_class):
        if name is None:
            while 'inst' +  str(self._next_device_index) in self._registry:
                self._next_device_index += 1
            name = 'inst' +  str(self._next_device_index)

        if name in self._registry:
            raise KeyError

        item = DeviceItem(device_class)
        item.lock = DeviceLock(name)

        self._registry[name] = item

        return

you can access __next_deviceindex by using self.next_device_index but if you use self._next_device_index +=1you will accidently create a new instance variable to the object, and all further references to self._next_device_index now will reference to this instance variable. the class variable will not reflect the change you made.

to correct this, we sholud write cls._next_device_index

a correct way of handling of class variables can be found in class IntrServer in our code.

Now before I do patching this out , I wonder what is the preferred way to go from here:

A) we want an overall global registry for all devices in this library - this essencially means we handle only one InstrumentServer serving localhost for all connected interfaces. (Because host to '' in Vxi11AbortServer and Vxi11CoreServer, this is what we do now ) And because we currently use only one Instance of InstrumentServer our wrong code somehow magically works here, because instances of Vxi11AbortServer and Vxi11CoreServer share the same DevceRegistry instance.

B) we just want a registry be local to an InstrumentServer instance with its Vxi11AbortServer and Vxi11CoreServer - this way we could convert be able to launch server instances for different local network connections.

What do You think of this?

ulda commented 3 years ago

ok maybe I found my error in thinking: we always register to the local portmap daemon, so (B) is of no use (we always have only one instance of our Vxi11AboutServer and Vxi11CoreServer registered to the portmapper )

coburnw commented 3 years ago

Hi Ulf, good hearing from you again.

The device index is used to create default device names such as instr0, instr1 etc. Because each server by definition has an instr0 device, I suspect a class variable is unnecessary here. I will look into making _next_device_index an instance variable instead, and will do the same for the dictionary also. (looks like lockedincrementer needs some work too.) Be sure to let me know if something here doesnt sound quite right, or I'm simply missing the point... I have been wanting to spend some more time on this project, perhaps this will give me the motivation.

As a side note, class variables and pythons classmethod decorator have always been rather magical for me. Your query nudged me to pop the hood to get a better handle on the inner workings. Thank you.

c.