pyhys / minimalmodbus

Easy-to-use Modbus RTU and Modbus ASCII implementation for Python.
Apache License 2.0
306 stars 146 forks source link

Instantiate instrument with serial object as parameter? #92

Closed BenediktBurger closed 1 year ago

BenediktBurger commented 1 year ago

Hey, thanks for your work, it looks good.

We plan to integrate your protocol implementation in our instrument package PyMeasure. To make that work, we would like to instantiate the Instrument with our own Serial connection object (or something compatible). For example giving the object as the port parameter or as a new serial_object parameter

Obviously we could add our instrument to your _serialports module variable first, but that seems more like a hack (in our package)

Are you open for that possibility? If so, I could open a pull request with the necessary changes.

For example somthing like that in the Instrument.__init__:

if not isinstance(port, str):
    self.serial = port
elif port not in _serialports or not _serialports[port]:  # line from existing code, prefixed with "el"
j123b567 commented 1 year ago

It would be very useful. In my projects I also abuse _serialports to subvert the serial port before instantiating the instrument.

The advantage of the global _serialports is that you can easily instantiate multiple instruments on the same port without having your own serial port registry. I just can't see far enough if it is better to move this responsibility completely to the user (your proposal) or introduce some functionality to manipulate with minimalmodbus own registry or introduce serial port factory functionality.

pyhys commented 1 year ago

I think this will be pretty straight forward to implement, so I will try to include it into next release. @j123b567 Will the suggested solution work for your use case too?

BenediktBurger commented 1 year ago

Thanks.

pyhys commented 1 year ago

Hi this is now released. I did not use your code exactly, however I listed you in the Authors page: https://minimalmodbus.readthedocs.io/en/stable/authors.html

Thanks for your contribution!

BenediktBurger commented 1 year ago

Thank you for the implementation. It seems reasonable how you implemented it, checking, that the object has all the necessary methods. Thinking about it, it might be possible (as an alternative) to create a typing.Protocol subclass SerialLike, which has those methods and then do an isinstance(port, SerialLike) check (it has to be set to runtime checkable first).

EDIT: An example implementation:

from typing import runtime_checkable, Protocol

@runtime_checkable
class SerialLike(Protocol):
    """Has all the methods of `serial.Serial` necessary for minimalmodbus."""

    def write(self, content: bytes) -> None: ...

    def read(self, count: int) -> bytes: ...

    # similarly the other methods.

Instrument:
    # modifications to minimalmodbus.Instrument:
    def __init__(self, port: Union[str, SerialLike]):
        if isinstance(port, SerialLike):
            self.port = port