pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.26k stars 922 forks source link

ModbusSimulatorContext doesn't allow write holding registers with `"shared blocks": False` option #1968

Closed realrushen closed 8 months ago

realrushen commented 8 months ago

Versions

Pymodbus Specific

Description

I'm running same configuration for ModbusSimulatorContext as in #1960 issue that can be checked below.

I can read up to 8 holding registers, but i can't write them.

(venv) PS D:\code\MyProjects\pymodbus> modpoll -1 -p 5020 -t 4 -r 1 -c 1 localhost 2
modpoll 3.10 - FieldTalk(tm) Modbus(R) Master Simulator
Copyright (c) 2002-2021 proconX Pty Ltd
Visit https://www.modbusdriver.com for Modbus libraries and tools.

Protocol configuration: MODBUS/TCP, FC6
Slave configuration...: address = 1, start reference = 1, count = 1
Communication.........: localhost, port 5020, t/o 1.00 s, poll rate 1000 ms
Data type.............: 16-bit register, output (holding) register table

Illegal Data Address exception response!

This is what i found when debugging that issue: loop_validate on line 530

# ModbusSimulatorContext object state
>>> address, end_address, fx_write, reg.access, reg.type
(20, 21, True, False, 2)
>>> pprint([(i, r.access) for i, r in enumerate(self.registers)])
[(0, False),
 (1, True),
 (2, True),
 (3, True),
 (4, True),
 (5, True),
 (6, True),
 (7, True),
 (8, True),
 (9, False),
 (10, False),
 (11, False),
 (12, False),
 (13, False),
 (14, False),
 (15, False),
 (16, False),
 (17, False),
 (18, False),
 (19, False),
 (20, False),
 (21, False),
 (22, False),
 (23, False),
 (24, False),
 (25, False),
 (26, False),
 (27, False)]

This problem exist because Setup.handle_write_allowed() setting self.runtime.access attribute to wrong items.

Configuration

demo_config = {
    "setup": {
        "co size": 0,
        "di size": 0,
        "hr size": 8,
        "ir size": 20,
        "shared blocks": False,
        "type exception": False,
        "defaults": {
            "value": {
                "bits": 0x0001,
                "uint16": 1,
                "uint32": 45000,
                "float32": 127.4,
                "string": "X",
            },
            "action": {
                "bits": None,
                "uint16": None,
                "uint32": None,
                "float32": None,
                "string": None,
            },
        },
    },
    "invalid": [],
    "write": [
        [1, 8],
    ],
    "bits": [],
    "uint16": [
       [1, 27]  # not [1, 28] because server is only starting with [1, 27]
    ],
    "uint32": [],
    "float32": [],
    "string": [],
    "repeat": [],
}
janiversen commented 8 months ago

Pull requests are welcome, especially since you know what the problem is.

But as far as I can see handle_write_allowed sets access=True if write is to be allowed, so I am not sure what value you want to be different.

As you can see in your register dump.

Holding register 1-7 is write-able Input register 0 is write-able the rest is readonly.

I think you too, have not read the documentation, that explains the difference between the memory model with starts with 0 and the addressing model which start with 1.

janiversen commented 8 months ago

Closing this, since your register dump, shows it works exactly as defined. Feel free to show a write that produces a wrong result.

btw your comment "because server is only starting with [1, 27]" is wrong, you should be defining 0-27.

realrushen commented 8 months ago

btw your comment "because server is only starting with [1, 27]" is wrong, you should be defining 0-27.

Yeas this is my main misunderstanding, that cause all problems. Thanks for explanation.

I think you too, have not read the documentation, that explains the difference between the memory model with starts with 0 and the addressing model which start with 1.

I really read the documentation and did my best to understand it. But example config (from don't define register 0 that really confuse me. 0 is only in "repeat" block that inconsistent with register defining blocks.

btw what is real life use case of write block? I think its redundant because modbus specification defines write allowing holding registers and coils and they must be writable by default. Maybe we should remove it?

janiversen commented 8 months ago

Well what is the first index in an array, that is 0, so naturally the memory block start with 0, while the modbus addressing start with 1.

The modbus speficication states that only holding registers and coils are writeable, but not that the MUST be writeable, hence the section.

janiversen commented 8 months ago

We are thinking about reworking the configuration, into a more linear mode, because it is less complicated and more inline with how devices document the registers, but unless someone volunteers to work on it, it will take a while.