Skip to content
Merged
2 changes: 1 addition & 1 deletion pymodbus/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class Defaults(Singleton): # pylint: disable=too-few-public-methods
TransactionId = 0
Strict = True
ProtocolId = 0
Slave = 0x00
Slave: int = 0x00
Baudrate = 19200
Parity = "N"
Bytesize = 8
Expand Down
32 changes: 7 additions & 25 deletions pymodbus/repl/client/main.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
"""Pymodbus REPL Entry point."""
# pylint: disable=anomalous-backslash-in-string
# flake8: noqa
import logging
import pathlib
import sys


try:
import click
except ImportError:
print('click not installed!! Install with "pip install click"')
sys.exit(1)
try:
from prompt_toolkit import PromptSession, print_formatted_text
except ImportError:
print(
"prompt toolkit is not installed!! "
'Install with "pip install prompt_toolkit --upgrade"'
)
sys.exit(1)

import click
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work !!! the import needs to be behind a try/except, otherwise you require click to be installed always (you will see the same in client/serial and other places).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import is only needed for REPL, which can't be used anyway without click. the existing code which I removed calls sys.exit(1)

try:
import click
except ImportError:
print('click not installed!! Install with "pip install click"')
sys.exit(1)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but we have had problems, latest with sqlalchemy because python loads the files and thus tries to do the import. The problem arises is the package is referenced (jnit) so for optional libraries we prefer to use try/except.

Having the try/except allows python to pass the module without having click installed (which is optional).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the "modern" way we handle optional packages:

try:
    import serial
except ImportError:
    pass

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1114 was the import error with pyserial, which was caused because it was imported in the generic server class. This is called by the normal server/client code paths.
#1319 was the import error with sqlalchemy and redis, which was caused because they were imported in the datastore. This is called by the normal server/client code paths.

I don't think this is true here, since no other code in the normal server/client code paths calls the REPL. If something did, it would already trigger the (existing) sys.exit(1). I have tested using the pymodbus client without click installed, and it's fine. So the Try/Except is not protecting anything.

As far as I can tell, if click is not installed the REPL is useless. So the Try/Except only changes the ImportError to a NameError.

I can add the Try/Except, of course, but don't think it has any use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your analysis is correct, I was merely preparing for the future where init contain all external classes, and deep links are not allowed.

Lets leave it for now.

from prompt_toolkit import PromptSession, print_formatted_text
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be in try/except.

from prompt_toolkit.auto_suggest import AutoSuggestFromHistory
from prompt_toolkit.formatted_text import HTML
from prompt_toolkit.history import FileHistory
Expand All @@ -46,9 +30,7 @@

_logger = logging.getLogger()

click.disable_unicode_literals_warning = True

TITLE = f"""
TITLE = rf"""
----------------------------------------------------------------------------
__________ _____ .___ __________ .__
\______ \___.__. / \ ____ __| _/ \______ \ ____ ______ | |
Expand Down Expand Up @@ -124,8 +106,8 @@ def convert(self, value, param, ctx):
return None


def process_args(args: list, string: bool = True):
"""Internal function to parse arguments provided on command line.
def _process_args(args: list, string: bool = True):
"""Parse arguments provided on command line.

:param args: Array of argument values
:param string: True if arguments values are strings, false if argument values are integers
Expand Down Expand Up @@ -226,7 +208,7 @@ def _(event):
text = text.strip().split()
cmd = text[0].split(".")[1]
args = text[1:]
kwargs, execute = process_args(args, string=False)
kwargs, execute = _process_args(args, string=False)
if execute:
if text[0] in CLIENT_ATTRIBUTES:
result = Result(getattr(client, cmd))
Expand All @@ -242,7 +224,7 @@ def _(event):
result.raw()
if words[0] == "result.decode":
args = words[1:]
kwargs, execute = process_args(args)
kwargs, execute = _process_args(args)
if execute:
result.decode(**kwargs)
except KeyboardInterrupt:
Expand Down
21 changes: 10 additions & 11 deletions pymodbus/repl/client/mclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,10 @@ def read_input_registers(self, address, count=1, slave=Defaults.Slave, **kwargs)

def readwrite_registers(
self,
read_address,
read_count,
write_address,
write_registers,
read_address=0,
read_count=0,
write_address=0,
values=0,
slave=Defaults.Slave,
**kwargs,
):
Expand All @@ -235,7 +235,7 @@ def readwrite_registers(
:param read_address: register offset to read from
:param read_count: Number of registers to read
:param write_address: register offset to write to
:param write_registers: List of register values to write (comma separated)
:param values: List of register values to write (comma separated)
:param slave: Modbus slave unit ID
:param kwargs:
:return:
Expand All @@ -244,7 +244,7 @@ def readwrite_registers(
read_address=read_address,
read_count=read_count,
write_address=write_address,
write_registers=write_registers,
values=values,
slave=slave,
**kwargs,
)
Expand Down Expand Up @@ -321,15 +321,14 @@ def report_slave_id(self, slave=Defaults.Slave, **kwargs):
}
return ExtendedRequestSupport._process_exception(resp)

def read_exception_status(self, **kwargs):
"""Read tcontents of eight Exception Status output.

In a remote device.
def read_exception_status(self, slave=Defaults.Slave, **kwargs):
"""Read contents of eight Exception Status output in a remote device.

:param slave: Modbus slave unit ID
:param kwargs:
:return:
"""
request = ReadExceptionStatusRequest(**kwargs)
request = ReadExceptionStatusRequest(slave, **kwargs)
resp = self.execute(request) # pylint: disable=no-member
if not resp.isError():
return {"function_code": resp.function_code, "status": resp.status}
Expand Down
4 changes: 2 additions & 2 deletions pymodbus/repl/server/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Pymodbus REPL Server

Pymodbus REPL server helps to quicky spin an [asynchronous server](../../../examples/common/asyncio_server.py) from command line.
Pymodbus REPL server helps to quickly start an [asynchronous server](../../../examples/common/asyncio_server.py) from the command line.

Support both `Modbus TCP` and `Modbus RTU` server.

Expand All @@ -9,7 +9,7 @@ Some features offered are

---
1. Runs a [reactive server](../../server/reactive/main.py) in `REPL` and `NON REPL` mode.
2. Exposes REST API's to manipulate the behaviour of the server in non repl mode.
2. Exposes REST API's to manipulate the behaviour of the server in non REPL mode.
3. Ability to manipulate the out-going response dynamically (either via REPL console or via REST API request).
4. Ability to delay the out-going response dynamically (either via REPL console or via REST API request).
5. Auto revert to normal response after pre-defined number of manipulated responses.
Expand Down
8 changes: 3 additions & 5 deletions pymodbus/repl/server/main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
"""Repl server main."""
from __future__ import annotations

import asyncio
import json
import sys
Expand Down Expand Up @@ -71,11 +69,11 @@ def servers(incomplete: str) -> List[str]:
return _completer(incomplete, _servers)


def process_extra_args(extra_args: list[str], modbus_config: dict) -> dict:
def process_extra_args(extra_args: List[str], modbus_config: dict) -> dict:
"""Process extra args passed to server."""
options_stripped = [x.strip().replace("--", "") for x in extra_args[::2]]
extra_args = dict(list(zip(options_stripped, extra_args[1::2])))
for option, value in extra_args.items():
extra_args_dict = dict(list(zip(options_stripped, extra_args[1::2])))
for option, value in extra_args_dict.items():
if option in modbus_config:
try:
modbus_config[option] = type(modbus_config[option])(value)
Expand Down
4 changes: 2 additions & 2 deletions pymodbus/server/simulator/http_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ def __init__(
"server_json": [None, self.build_json_server],
}
for entry in self.generator_html: # pylint: disable=consider-using-dict-items
file = os.path.join(self.web_path, "generator", entry)
with open(file, encoding="utf-8") as handle:
html_file = os.path.join(self.web_path, "generator", entry)
with open(html_file, encoding="utf-8") as handle:
self.generator_html[entry][0] = handle.read()
self.refresh_rate = 0
self.register_filter: List[int] = []
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ aiohttp==3.8.3
typer[all]==0.7.0
prompt-toolkit==3.0.36
pygments==2.14.0
click>=8.0.0

# install:serial
pyserial>=3.5
Expand Down
24 changes: 12 additions & 12 deletions test/test_repl_client.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""Test client sync."""
from pymodbus.repl.client.main import process_args
from pymodbus.repl.client.main import _process_args
from pymodbus.server.reactive.default_config import DEFAULT_CONFIG


Expand All @@ -10,39 +10,39 @@ def test_repl_default_config():


def test_repl_client_process_args():
"""Test argument processing in repl.client.main ( process_args function)."""
resp = process_args(["address=11"], False)
"""Test argument processing in repl.client.main ( _process_args function)."""
resp = _process_args(["address=11"], False)
assert resp == ({"address": 11}, True)

resp = process_args(["address=0x11"], False)
resp = _process_args(["address=0x11"], False)
assert resp == ({"address": 17}, True)

resp = process_args(["address=0b11"], False)
resp = _process_args(["address=0b11"], False)
assert resp == ({"address": 3}, True)

resp = process_args(["address=0o11"], False)
resp = _process_args(["address=0o11"], False)
assert resp == ({"address": 9}, True)

resp = process_args(["address=11", "value=0x10"], False)
resp = _process_args(["address=11", "value=0x10"], False)
assert resp == ({"address": 11, "value": 16}, True)

resp = process_args(["value=11", "address=0x10"], False)
resp = _process_args(["value=11", "address=0x10"], False)
assert resp == ({"address": 16, "value": 11}, True)

resp = process_args(["address=0b11", "value=0x10"], False)
resp = _process_args(["address=0b11", "value=0x10"], False)
assert resp == ({"address": 3, "value": 16}, True)

try:
resp = process_args(["address=0xhj", "value=0x10"], False)
resp = _process_args(["address=0xhj", "value=0x10"], False)
except ValueError:
pass

try:
resp = process_args(["address=11ah", "value=0x10"], False)
resp = _process_args(["address=11ah", "value=0x10"], False)
except ValueError:
pass

try:
resp = process_args(["address=0b12", "value=0x10"], False)
resp = _process_args(["address=0b12", "value=0x10"], False)
except ValueError:
pass