Proof-of-concept: Add network support (IPv4Transport)

[hperrey]

This PR adds basic support for a network transport by adding the necessary classes IPv4Transport and IPv4TransportHandle that communicate over socket to the pyseabreeze backend.

It supports multicast to discover devices on the local network. Tests were done using an HDX spectrometer connected via ethernet (direct link).

Please note that this is a best-effort proof-of-concept at this point:

• no few exceptions from the socket library are handled yet
• there is no discovery of devices, only registration of known devices via api.add_ipv4_device_location; this could be replaced by multicast discovery
• the HDX device is hard-coded to IPv4Transport; having both transports there caused issues
• some methods are basically placeholders and still require implementation
• no extensive tests beyond reading intensities has been done yet nor have any unit tests been implemented

Feedback on proper integration into the framework and best-practices welcome!

Sample run:

In [1]: import seabreeze.pyseabreeze as psb
   ...: # specifying a network adapter will enable multicast:
   ...: api = psb.SeaBreezeAPI(network_adapter="192.168.254.200")
   ...: # alternatively: add device and address manually: 
   ...: # api.add_ipv4_device_location("HDX", "192.168.254.254", 57357)
   ...: devices = api.list_devices()
Waiting to receive multicast response(s)

In [2]: devices
Out[2]: [<SeaBreezeDevice QE65000:QEPB01234>, <SeaBreezeDevice HDX:HDX01234>]

In [3]: import seabreeze
   ...: seabreeze.use('pyseabreeze')
   ...: from seabreeze.spectrometers import Spectrometer

In [4]: spec = Spectrometer(device=devices[1])

In [5]: spec.intensities(0,1)
Out[5]: 
array([1978.05266071, 1332.67455752, 1433.36236484, ..., 1415.41637616,
       1417.41034014, 1416.41335706])

In [6]: spec.model
Out[6]: 'HDX'

(note that I replaced the serial in the output above)

Left to be done:

• [x] fix remaining typing errors
• [x] test that both USB and ethernet work with the HDX
• [x] consider to refactorize the packet generation for the multicast request and the model name matching
• [x] implement shutdown and initialize of IPv4Transport (or remove TODO comment)
• [ ] multicast feature: investigate why requests for multicast group and port do not work (or remove implementation)

Addresses #240.

[hperrey]

I have been playing around with multicast, both the feature accessible via OBP and as a means to discover connected spectrometers.

With the feature implementation, I ran into unexpected issues: the message types in my version of the "Firmware and Advanced Communications: OCEAN HDX Firmware" for retrieving multicast settings from the device are only triggering a "unknown message type" error. The only command of those working is get_multicast_enable_state.

The discovery via multicast looks promising and I hope to add a commit implementing a first version of that soon.

Then I will be looking into the failing tests and pre-commit CI checks...

[hperrey]

Ok, I believe that I have multicast discovery working now :)

[ap--]

Thank you so much for contributing @hperrey ❤️

This looks great already. I don't have access to an HDX spectrometer (or any ocean optics/insight spectrometer with ethernet support), so I won't be able to test this, but it seems you have the important parts already running 👏 🎉

I'm currently traveling, but will allocate some time tomorrow afternoon to review your PR!

Cheers, Andreas

[hperrey]

This looks really great already. I made several comments.

We'd still need to expose this in seabreeze.spectrometers.list_devices or as a new. I wonder what the nicest interface would be.

Thanks for the detailed review! I fixed most issues you highlighted but have had questions on a few others that I commented on.

Regarding exposing this via the API: I thought this would work already out-of-the box, but there seems to be some road-block still:

In [1]: import seabreeze
   ...: seabreeze.use('pyseabreeze')
   ...: import seabreeze.pyseabreeze as psb
   ...: api = psb.SeaBreezeAPI(network_adapter="192.168.254.200")
   ...: from seabreeze.spectrometers import list_devices, Spectrometer

In [2]: list_devices()
Out[2]: [<SeaBreezeDevice QE65000:QEPB0123>]

In [3]: api.list_devices()
Out[3]: [<SeaBreezeDevice QE65000:QEPB0123>, <SeaBreezeDevice HDX:HDX0123>]

In [4]: list_devices()
Out[4]: [<SeaBreezeDevice QE65000:QEPB0123>, <SeaBreezeDevice HDX:HDX0123>]

I assume that the key-word argument is not passed on in the first call to spectrometer.list_devices? This is on a machine with several interfaces, so specifying the right one is necessary. Simply repeating the call to spectrometer.list_devices does not help, one has to call api.list_devices() once. However, I thought that that was what was called under the hood? Have not walked this through with pdb yet though.

[hperrey]

mypy is still complaining about the following type errors which I have not been able to localize:

src/seabreeze/pyseabreeze/devices.py:317: error: "ABCMeta" has no attribute "supported_model"  [attr-defined]
src/seabreeze/pyseabreeze/api.py:60: error: Invalid index type "Union[Tuple[int, int, int, int], Tuple[str, int]]" for "WeakValueDictionary[Tuple[int, int, int, int], SeaBreezeDevice]"; expected type "Tuple[int, int, int, int]"  [index]
src/seabreeze/pyseabreeze/api.py:62: error: Invalid index type "Union[Tuple[int, int, int, int], Tuple[str, int]]" for "WeakValueDictionary[Tuple[int, int, int, int], SeaBreezeDevice]"; expected type "Tuple[int, int, int, int]"  [index]

Any hints on where this might come from would be appreciated :)

[hperrey]

Only this type error remains:

src/seabreeze/pyseabreeze/devices.py:317: error: "ABCMeta" has no attribute "supported_model"  [attr-defined]

[hperrey]

Concerning the interface: I am thinking about reconsidering when and how long to open the sockets, as the current interface is almost working:

In [1]: import seabreeze
   ...: seabreeze.use('pyseabreeze')
   ...: import seabreeze.pyseabreeze as psb
   ...: api = psb.SeaBreezeAPI(network_adapter="192.168.254.200")
   ...: from seabreeze.spectrometers import list_devices, Spectrometer

In [2]: list_devices()
Out[2]: [<SeaBreezeDevice QE65000:QEPB0123>]

In [3]: api.list_devices()
Out[3]: [<SeaBreezeDevice QE65000:QEPB0123>, <SeaBreezeDevice HDX:HDX01234>]

In [4]: list_devices()
Out[4]: [<SeaBreezeDevice QE65000:QEPB0123>, <SeaBreezeDevice HDX:HDX01234>]

In [5]: spectrometer = Spectrometer.from_serial_number("HDX01234")
---------------------------------------------------------------------------
SeaBreezeError                            Traceback (most recent call last)
Cell In[5], line 1
----> 1 spectrometer = Spectrometer.from_serial_number("HDX01234")

File ~/src/python-seabreeze/src/seabreeze/spectrometers.py:140, in Spectrometer.from_serial_number(cls, serial)
    138 if dev.serial_number == str(serial):
    139     if dev.is_open:
--> 140         raise cls._backend.SeaBreezeError("Device already opened.")
    141     else:
    142         return cls(dev)

SeaBreezeError: Device already opened.

That will require a little bit of refactorization though but would allow to use the API in the same way as for USB

[hperrey]

All type errors are now fixed :)

[hperrey]

I have changed how the socket is managed and now the interface works almost as expected:

In [1]: import seabreeze
   ...: seabreeze.use('pyseabreeze')
   ...: import seabreeze.pyseabreeze as psb
   ...: api = psb.SeaBreezeAPI(network_adapter="192.168.254.200")
   ...: from seabreeze.spectrometers import Spectrometer

In [2]: api.list_devices()
Out[2]: [<SeaBreezeDevice QE65000:QEPB0123>, <SeaBreezeDevice HDX:HDX01234>]

In [3]: spec = Spectrometer.from_serial_number("HDX01234")

In [4]: spec.intensities(0,1)
Out[4]: 
array([1970.06733923, 1338.65538922, 1418.40732539, ..., 1409.43453638,
       1409.43453638, 1409.43453638])

The call to api.list_devices() is needed, or from_serial_number() will not see the HDX (see comment above). However, after that, things seem to be working :)

[ap--]

backend config

The way to do the configuration of a backend in the current design would be the following:

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/seabreeze/backends.py#L60-L67

here we would need to add code like:

    if backend == "pyseabreeze":
        if "pyusb_backend" in kwargs:
            pyusb_backend = kwargs.pop("pyusb_backend")
            BackendConfig.api_kwargs["pyusb_backend"] = pyusb_backend
        if "network_adapter" in kwargs:
            network_adapter = kwargs.pop("network_adapter")
            BackendConfig.api_kwargs["network_adapter"] = network_adapter
    ...

Then using the ethernet discovery would look like this in code:

import seabreeze
seabreeze.use("pyseabreeze", network_adapter="192.168.1.200")

from seabreeze.spectrometers import Spectrometer

spec = Spectrometer.from_serial_number("...")

adding lazy backend retrieval

With a small change to seabreeze.spectrometers we should also be able to not have to split the imports:

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/seabreeze/spectrometers.py#L22-L23

should be lazy:

# get the backend and add some functions/classes to this module
_lib: SeaBreezeBackend

def __getattr__(name):
    global _lib
    if name == "_lib":
        _lib = seabreeze.backends.get_backend()
        return _lib
    elif name == "SeabreezeDevice":
        ...  # same for this one (right now they are eagerly created)
    elif name == "SeabreezeError":
        ...  # same for this one
    else:
        raise AttributeError(name)

This should allow us to write code like this:

import seabreeze
from seabreeze.spectrometer import Spectrometer

seabreeze.use("pyseabreeze", network_adapter="192.168.1.200")

spec = Spectrometer.from_serial_number("...")

[hperrey]

backend config

The way to do the configuration of a backend in the current design would be the following:

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/seabreeze/backends.py#L60-L67

here we would need to add code like:

    if backend == "pyseabreeze":
        if "pyusb_backend" in kwargs:
            pyusb_backend = kwargs.pop("pyusb_backend")
            BackendConfig.api_kwargs["pyusb_backend"] = pyusb_backend
        if "network_adapter" in kwargs:
            network_adapter = kwargs.pop("network_adapter")
            BackendConfig.api_kwargs["network_adapter"] = network_adapter
    ...

Then using the ethernet discovery would look like this in code:

import seabreeze
seabreeze.use("pyseabreeze", network_adapter="192.168.1.200")

from seabreeze.spectrometers import Spectrometer

spec = Spectrometer.from_serial_number("...")

Yes, this approach works!

In [1]: import seabreeze
   ...: seabreeze.use("pyseabreeze", network_adapter="192.168.254.200")
   ...: 
   ...: from seabreeze.spectrometers import Spectrometer

In [2]: spec = Spectrometer.from_serial_number("HDX01234")

In [3]: spec.intensities(0,1)
Out[3]: 
array([1967.07287765, 1325.69702018, 1418.40732539, ..., 1398.4680335 ,
       1406.44364585, 1397.47109175])

The approach with lazy loading would require to create get_device and get_error similarly to get_backend? The interface does look nicer. However, I don't quite understand that part of the code well enough.

For now, I think I am satisfied with what 0e5a59b brings to the table.

The tests still fail though, mainly due to the multicast feature that unfortunately doesn't even work -- I am not sure whether it is a mistake on my side, or that the specs are not correct for that FW version or something like that. Anyway, that feature is not really necessary, so I could remove everything but the get_multicast_enabled (iirc) command which seem to work.

[hperrey]

I have tried connecting to the HDX using my Ubuntu laptop but ran into #133 .. ethernet, on the other hand, works fine :)

[ap--]

This is great! I am happy to merge this. For now I would remove the extra functionality you added to the Multicast Feature (the 3 methods the tests complain about), to keep feature parity with cseabreeze. (Although it might not really be enabled at all there...) We can add it back later down the line. I will add some text to the readme with a code snippet example.

fyi, the multicast get state / set state feature in cseabreeze uses these two command codes:

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/libseabreeze/include/vendors/OceanOptics/protocols/obp/constants/OBPMessageTypes.h#L86-L87

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/libseabreeze/src/vendors/OceanOptics/protocols/obp/exchanges/OBPGetMulticastEnableExchange.cpp#L38-L44

https://github.com/ap--/python-seabreeze/blob/a8fef98b14eb10e8bb4c5a3eb1b03e35995051fa/src/libseabreeze/src/vendors/OceanOptics/protocols/obp/exchanges/OBPSetMulticastEnableExchange.cpp#L39-L47

Let me know if you still want to make changes, or if I should take over and merge and prepare a new release!

Cheers, Andreas 😃

[hperrey]

Great! I'd be more than fine with you going forward with the merge and release -- thank you!

I might revisit the multicast feature, but more importantly I would like to work with the buffered readout that the HDX allows. But that is for another day and PR :)