Modbus rework #9141
Modbus rework #9141
Conversation
![telegraf-tiger[bot]](https://avatars.githubusercontent.com/in/94907?s=60&v=4){kind=small}
|
@ssoroka I fully agree that it should see some testing. I will give it a first test-run next week with about 20 devices, but would be happy if we get some more coverage with different devices... |
|
Added some debugging output to show the raw-values on debug to verify type-conversions. Furthermore, extend the README with a trouble-shooting section. Thanks @binueda for his review of the Trouble-shooting section on slack. :-) |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
srebhan
added
area/modbus
feat
|
!retry-failed |
…equest on the wire with a consecutive set of register addresses.
…done to avoid extensive logs during standard debug. Please note, you need to enable both, 'trace_connection=true' as well as debug mode to see the messages.
…it to 'true' for serial connections.
…d force it to 'true' for serial connections." to not clutter this PR. Will add another PR instead. This reverts commit 07487f6.
…This is done to avoid extensive logs during standard debug. Please note, you need to enable both, 'trace_connection=true' as well as debug mode to see the messages." to not clutter this PR. Will add another PR instead. This reverts commit 3d235f1.
|
!retry-failed |
|
If the intent here is to eliminate 8013 and merge in its features there are a few things it needs to be able to do:
In particularly, the "gateway" functionality provided by #8013 is needed, and I don't think it's addressed here. The issue is allowing you to talk to multiple devices attached to a single gateway without having to have separate TCP connections for each unit. This is a common scenario that occurs even when talking to devices that are not traditionally "gateways." As an example, one of the more popular solar inverters out there is the SMA SB series (e.g. SB3000). It has multiple register sets located inside it, and there are occasions where you want to be able to use both at the same time. To select one vs the other you use a different unit address. Essentially, this inverter acts as two "devices" in one. The other common scenario is strings of power meters e.g. the Square D smart breakers where each breaker is its own device (with its own unit address). If this PR addresses the above then it's fine; if it's not, then I don't think this replaces or merges 8013. If I'm wrong please let's discuss. |
|
I run some tests in the following condition: coils = [
{ name = "D1", measurement="somemeasure", address = [260]},
{ name = "D2", measurement="somemeasure", address = [262]},
]
holding_registers = [
{ name = "A", measurement="somemeasure", byte_order = "ABCD", data_type = "FLOAT32-IEEE", scale=1.0, address = [1, 2]},
{ name = "B", measurement="somemeasure", byte_order = "AB", data_type = "FIXED", scale=1.0, address = [3]},
{ name = "C", measurement="somemeasure", byte_order = "AB", data_type = "FIXED", scale=0.1, address = [4]},
{ name = "D", measurement="somemeasure", byte_order = "AB", data_type = "FIXED", scale=0.001, address = [5]},
]and the results are ok for me. Data types are good; I would maybe consider to save booleans for the coils and discrete_inputs as those are boolean by definition in the standard, instead of integers. |
|
@mirkocomparetti-synesis thanks for testing! |
|
@ssoroka testing was successful. :-) |
Looks like new artifacts were built from this PR. Get them here!Artifact URLs |
Required for all PRs:
Restructuring and cleaning the modbus input plugin. This is preparation work to incorporate features from PR #7941 and #8013 as well as various feature-requests around modbus into the current plugin's code-base.
As a side effect the structure of the plugin as well as unit-tests are simplified and IMO easier to follow. The used modbus-library is migrated from the unmaintained goburrow version to the grid-x fork. Additionally, all existing linter-issues are nuked by this PR.