ModbusTcpClient constructor BC break between 3.6.9 and 3.7.0
rtek opened this issue · 15 comments
The ModbusTcpClient had a signature change between 3.6 and 3.7 for the port parameter.
see
https://github.com/pymodbus-dev/pymodbus/blob/v3.6.9/pymodbus/client/tcp.py#L61
def __init__(
self,
host: str,
port: int = 502,
framer: Framer = Framer.SOCKET,
source_address: tuple[str, int] | None = None,
**kwargs: Any,
) -> None:
vs
https://github.com/pymodbus-dev/pymodbus/blob/v3.7.0/pymodbus/client/tcp.py#L61
def __init__( # pylint: disable=too-many-arguments
self,
host: str,
framer: FramerType = FramerType.SOCKET,
port: int = 502,
name: str = "comm",
source_address: tuple[str, int] | None = None,
reconnect_delay: float = 0.1,
reconnect_delay_max: float = 300,
timeout: float = 3,
retries: int = 3,
on_connect_callback: Callable[[bool], None] | None = None,
) -> None:
This results in the following error if using positional arguments for port.
Traceback (most recent call last):
File "main.py", line 13, in <module>
client = ModbusTcpClient('localhost', 5502)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "../pymodbus/client/tcp.py", line 158, in __init__
super().__init__(framer, retries)
File "../pymodbus/client/base.py", line 190, in __init__
self.framer: ModbusFramer = FRAMER_NAME_TO_CLASS.get(
^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'str' object is not callable
Reverting to 3.6.9 or using kwargs resolves the issue.
Yes we even documented it, see API_changes.rst
This is not a bug.
Hello - which item referenced this?
https://github.com/pymodbus-dev/pymodbus/blob/dev/API_changes.rst#api-changes-370
Thats the first place I checked and I couldnt find it.
We strongly advise against using positional parameters to set non-positional parameters!
The port= have not changed signature, you are just not using it as the signature defines it.
Got caught by this as well. This could be considered a breaking change needing a major release but I won't complain. It only took me a few minutes to find this issue and fix my code.
We strongly advise against using positional parameters to set non-positional parameters!
If I may suggest, those kwargs could be made keyword-only in the next pymodbus major release (v4).
def __init__( # pylint: disable=too-many-arguments
self,
host: str,
*,
framer: FramerType = FramerType.SOCKET,
port: int = 502,
name: str = "comm",
source_address: tuple[str, int] | None = None,
reconnect_delay: float = 0.1,
reconnect_delay_max: float = 300,
timeout: float = 3,
retries: int = 3,
on_connect_callback: Callable[[bool], None] | None = None,
) -> None:Up to you where to put the limit. Can host be positional?
At this point, you may even consider doing this right away since code using positional parameters is broken already.
First of all, there are no change of signature, port is an optional parameter NOT a positional parameter!
Secondly we have a definition that API changes are only allowed in x.y.0 releases, NOT in x.y.z releases, so going to 3.7.0 means API changes, which are documented in API_changes.rst.
the positional parameters are NOT broken, again the problem is that you do NOT follow the signature in 3.7 nor in earlier versions.
The signature says "port = 502" meaning it is an optional parameter without a fixed position.
We do not use kwargs ! that was removed in 3.7 especially to make the signatures clearly readable.
we have a definition that API changes are only allowed in x.y.0 releases
I assumed that would be only in x.0.0, because this is typical semver. But each project is free to decide otherwise. My fault for not reading the docs.
Anyway, I mainly stopped here for the friendly kw-only suggestion, not to rant about the change.
port is an optional parameter NOT a positional parameter!
Then making it keyword-only as I suggest above is the best way to express and enforce it IMHO.
The signature says "port = 502" meaning it is an optional parameter without a fixed position.
Well, I don't think that is the admitted convention. By default in Python, keyword parameters can be supplied positionally. Hence the kw-only PEP.
We do not use kwargs ! that was removed in 3.7 especially to make the signatures clearly readable.
I'm not asking for
def __init__(self, host: str, **kwargs):
...What I'm talking about is this:
def __init__( # pylint: disable=too-many-arguments
self,
host: str,
*, # <-- args after the star can only be supplied by name, otherwise an exception is raised
framer: FramerType = FramerType.SOCKET,
port: int = 502,
name: str = "comm",
source_address: tuple[str, int] | None = None,
reconnect_delay: float = 0.1,
reconnect_delay_max: float = 300,
timeout: float = 3,
retries: int = 3,
on_connect_callback: Callable[[bool], None] | None = None,
) -> None:Doing this prevents user from supplying args positionally, therefore from being caught by a signature change in the future.
Anyway, just wanted to make my earlier message clearer. I don't mean to argue about this. Code fixed on my side and everything works a treat. Thanks for pymodbus.
Feature added to the 3.8 list, just have to check if that works across all supported python versions.
thanks for the pointer.
No pb.
This was introduced much earlier than 3.9. I can't remember but from a quick search it may well be Python 3.0.