You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by "TomNewChao (via GitHub)" <gi...@apache.org> on 2026/10/03 04:06:26 UTC
[PR] feat(plc4net): add the Modbus TCP and RTU drivers
TomNewChao opened a new pull request, #2792:
URL: https://github.com/apache/plc4x/pull/2792
## Summary
- add the Modbus TCP (`modbus-tcp`) and RTU (`modbus-rtu`) drivers on top of the generated protocol model; the PDUs, the TCP and RTU ADUs, the RTU CRC and the register values all come from the model that `plc4net-code-gen` generates from `modbus.mspec`, so the drivers encode and decode no bytes themselves
- check the generated model in under `plc4net/drivers/modbus/model` and add a CI job that regenerates it and compares
- support plc4j's tag syntax `{area}:{address}[:{TYPE}]`, with the address counted from 1, for coils, discrete inputs, holding registers and input registers with the types BOOL, INT, UINT and WORD; a read or write handles one value per tag
- follow plc4j's connection parameters `default-unit-identifier` and `request-timeout-ms`; the older `unit-identifier` and `request-timeout` are still read, and a malformed value is an error rather than a default
- fix two generator defects that the model exposed (see Context) and add regression coverage that needs no hardware
## Context
The rest of plc4j's tag syntax (array selections, string lengths, tag configuration, extended registers, the short forms such as `40001`, the wider data types) is rejected with an explanation for now. The S7 and KNX drivers are separate changes.
Two generator fixes are part of this change because the Modbus model needs them:
- generated files are written without a final newline, as `.editorconfig` requires
- a case of a discriminated type that does not pin a discriminator read from the wire now keeps the value it was parsed with, as plc4j does. `ModbusPDUError` pins `errorFlag` but not `functionFlag`, so a parsed exception response serialized with function code 0, and because the generated RTU parser checks the CRC over the re-serialized PDU, every RTU exception response failed its checksum
## Validation
- `dotnet build plc4net/plc4net.sln --configuration Debug --no-restore --no-incremental --nologo`
- 0 warnings, 0 errors
- `dotnet test plc4net/plc4net.sln --configuration Debug --no-build --no-restore --nologo`
- 231 Modbus, 251 SPI and 141 code-generation tests passed
- the shared Modbus TCP, RTU and ASCII `ParserSerializerTestsuite` vectors round-trip through the model, and the requests and answers of plc4j's driver test data for the supported tags produce the same PDUs
- both connections are covered against a scripted transport, and the TCP driver against a loopback Modbus slave over real sockets
- `dotnet run --project plc4net/tools/code-gen -- modbus protocols/modbus/src/main/resources/protocols/modbus/modbus.mspec <dir>` reproduces the checked-in model byte for byte
- `git diff --check`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4245823755
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
This lookahead scan allocates repeatedly (`new[] { ... }` on each offset, and `buffered[offset..]` creates a new array copy). In noisy-line scenarios this can become allocation-heavy. Consider (mandatory for efficiency) switching these helpers to work on `ReadOnlySpan<byte>`/`Span<byte>` (e.g., compute `ExpectedLength` from three bytes directly, and pass `buffered.AsSpan(offset, length)` into an `IsFrame(ReadOnlySpan<byte>)` overload) to avoid per-iteration allocations.
##########
plc4net/drivers/modbus/ModbusTag.cs:
##########
@@ -0,0 +1,212 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using System.Globalization;
+using System.Text.RegularExpressions;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.exceptions;
+using org.apache.plc4net.model;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// A Modbus tag: an address on a Modbus device.
+ /// </summary>
+ /// <remarks>
+ /// <para>
+ /// The syntax is the one plc4j uses, so the same address names the same register from
+ /// either language: <c>{area}:{address}[:{TYPE}]</c>.
+ /// </para>
+ /// <list type="bullet">
+ /// <item><c>coil:{address}</c> - a single bit, read and written (functions 0x01, 0x05).</item>
+ /// <item><c>discrete-input:{address}</c> - a single bit, read only (0x02).</item>
+ /// <item><c>holding-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read and
+ /// written (0x03, 0x06); <c>INT</c> unless stated.</item>
+ /// <item><c>input-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read only (0x04).</item>
+ /// </list>
+ /// <para>
+ /// The address is the register number as a Modbus user counts it, from 1: the request
+ /// carries <c>address - 1</c>, so <c>holding-register:1</c> is the register at wire
+ /// address 0.
+ /// </para>
+ /// <para>
+ /// Array selections, string lengths, tag configuration, extended registers, the short
+ /// forms (<c>40001</c>, <c>4x00001</c>) and the wider data types are accepted by plc4j
+ /// but not supported here yet; they are rejected rather than read as something else.
+ /// </para>
+ /// </remarks>
+ public sealed class ModbusTag : IPlcTag
+ {
+ public enum TagType
+ {
+ Coil,
+ HoldingRegister,
+ DiscreteInput,
+ InputRegister
+ }
+
+ /// <summary>The highest register number: wire address 65535.</summary>
+ public const int MaxAddress = 65536;
+
+ // [0-9], not \d: in .NET \d matches every Unicode decimal digit, so a
+ // non-ASCII address would pass the pattern and blow up int.Parse below
+ // instead of failing here as a PlcInvalidFieldException.
+ private static readonly Regex AddressPattern = new Regex(
+ @"^(?<area>coil|discrete-input|holding-register|input-register):(?<address>[0-9]{1,9})(:(?<datatype>[A-Za-z_]+))?$",
+ RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
+
+ public ModbusTag(TagType type, int address, ModbusDataType dataType)
+ {
+ if (address < 1 || address > MaxAddress)
+ {
+ throw new PlcInvalidFieldException(
+ $"The Modbus address must be between 1 and {MaxAddress}, but was {address}.");
+ }
+ if (!IsSupported(type, dataType))
+ {
+ throw new PlcInvalidFieldException(
+ $"The data type {dataType} is not supported for {AreaName(type)} tags yet; " +
+ $"supported: {SupportedTypes(type)}.");
+ }
+
+ Type = type;
+ Address = address;
+ DataType = dataType;
+ }
+
+ public TagType Type { get; }
+
+ /// <summary>The register number, counted from 1.</summary>
+ public int Address { get; }
+
+ /// <summary>The address the request carries: <see cref="Address"/> - 1.</summary>
+ public ushort WireAddress => (ushort)(Address - 1);
+
+ public ModbusDataType DataType { get; }
+
+ /// <summary>Whether the tag is a single bit (a coil or a discrete input) rather than a register.</summary>
+ public bool IsBit => Type == TagType.Coil || Type == TagType.DiscreteInput;
+
+ /// <summary>
+ /// Parses a Modbus tag address such as <c>coil:1</c> or <c>holding-register:10:UINT</c>.
+ /// </summary>
+ /// <exception cref="PlcInvalidFieldException">The address is not a supported Modbus tag address.</exception>
+ public static ModbusTag Parse(string tagAddress)
+ {
+ if (string.IsNullOrWhiteSpace(tagAddress))
+ {
+ throw new PlcInvalidFieldException("The Modbus tag address must not be empty.");
+ }
+
+ var match = AddressPattern.Match(tagAddress.Trim());
+ if (!match.Success)
+ {
+ throw new PlcInvalidFieldException(
+ $"'{tagAddress}' is not a supported Modbus tag address. Expected " +
+ "{area}:{address}[:{TYPE}], for example holding-register:1:INT; the areas are coil, " +
+ "discrete-input, holding-register and input-register, and the address counts from 1. " +
+ "Array selections, string lengths, tag configuration, extended registers and short " +
+ "addresses such as 40001 are not supported yet.");
+ }
+
+ var type = ParseArea(match.Groups["area"].Value);
+ var address = int.Parse(match.Groups["address"].Value, CultureInfo.InvariantCulture);
+
+ ModbusDataType dataType;
+ var typeGroup = match.Groups["datatype"];
+ if (typeGroup.Success)
+ {
+ if (!TryParseDataType(typeGroup.Value, out dataType))
+ {
+ throw new PlcInvalidFieldException($"Unknown Modbus data type '{typeGroup.Value}'.");
+ }
+ }
+ else
+ {
+ dataType = type == TagType.Coil || type == TagType.DiscreteInput
+ ? ModbusDataType.BOOL
+ : ModbusDataType.INT;
+ }
+
+ return new ModbusTag(type, address, dataType);
+ }
+
+ public override string ToString() => $"{AreaName(Type)}:{Address}:{DataType}";
+
+ private static TagType ParseArea(string area)
+ {
+ switch (area.ToLowerInvariant())
+ {
+ case "coil":
+ return TagType.Coil;
+ case "discrete-input":
+ return TagType.DiscreteInput;
+ case "holding-register":
+ return TagType.HoldingRegister;
+ default:
+ return TagType.InputRegister;
+ }
+ }
Review Comment:
The `default` branch maps any unknown area to `InputRegister`. Since unknown areas are rejected elsewhere today, this likely won’t trigger, but it creates a silent-fallback hazard if the regex is later loosened or parsing is reused elsewhere. Prefer an explicit failure in the default branch (e.g., throw `PlcInvalidFieldException`) so new/typo areas can’t be misinterpreted as `input-register`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247839924
##########
plc4net/drivers/modbus/ModbusTag.cs:
##########
@@ -0,0 +1,212 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using System.Globalization;
+using System.Text.RegularExpressions;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.exceptions;
+using org.apache.plc4net.model;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// A Modbus tag: an address on a Modbus device.
+ /// </summary>
+ /// <remarks>
+ /// <para>
+ /// The syntax is the one plc4j uses, so the same address names the same register from
+ /// either language: <c>{area}:{address}[:{TYPE}]</c>.
+ /// </para>
+ /// <list type="bullet">
+ /// <item><c>coil:{address}</c> - a single bit, read and written (functions 0x01, 0x05).</item>
+ /// <item><c>discrete-input:{address}</c> - a single bit, read only (0x02).</item>
+ /// <item><c>holding-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read and
+ /// written (0x03, 0x06); <c>INT</c> unless stated.</item>
+ /// <item><c>input-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read only (0x04).</item>
+ /// </list>
+ /// <para>
+ /// The address is the register number as a Modbus user counts it, from 1: the request
+ /// carries <c>address - 1</c>, so <c>holding-register:1</c> is the register at wire
+ /// address 0.
+ /// </para>
+ /// <para>
+ /// Array selections, string lengths, tag configuration, extended registers, the short
+ /// forms (<c>40001</c>, <c>4x00001</c>) and the wider data types are accepted by plc4j
+ /// but not supported here yet; they are rejected rather than read as something else.
+ /// </para>
+ /// </remarks>
+ public sealed class ModbusTag : IPlcTag
+ {
+ public enum TagType
+ {
+ Coil,
+ HoldingRegister,
+ DiscreteInput,
+ InputRegister
+ }
+
+ /// <summary>The highest register number: wire address 65535.</summary>
+ public const int MaxAddress = 65536;
+
+ // [0-9], not \d: in .NET \d matches every Unicode decimal digit, so a
+ // non-ASCII address would pass the pattern and blow up int.Parse below
+ // instead of failing here as a PlcInvalidFieldException.
+ private static readonly Regex AddressPattern = new Regex(
+ @"^(?<area>coil|discrete-input|holding-register|input-register):(?<address>[0-9]{1,9})(:(?<datatype>[A-Za-z_]+))?$",
+ RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
+
+ public ModbusTag(TagType type, int address, ModbusDataType dataType)
+ {
+ if (address < 1 || address > MaxAddress)
+ {
+ throw new PlcInvalidFieldException(
+ $"The Modbus address must be between 1 and {MaxAddress}, but was {address}.");
+ }
+ if (!IsSupported(type, dataType))
+ {
+ throw new PlcInvalidFieldException(
+ $"The data type {dataType} is not supported for {AreaName(type)} tags yet; " +
+ $"supported: {SupportedTypes(type)}.");
+ }
+
+ Type = type;
+ Address = address;
+ DataType = dataType;
+ }
+
+ public TagType Type { get; }
+
+ /// <summary>The register number, counted from 1.</summary>
+ public int Address { get; }
+
+ /// <summary>The address the request carries: <see cref="Address"/> - 1.</summary>
+ public ushort WireAddress => (ushort)(Address - 1);
+
+ public ModbusDataType DataType { get; }
+
+ /// <summary>Whether the tag is a single bit (a coil or a discrete input) rather than a register.</summary>
+ public bool IsBit => Type == TagType.Coil || Type == TagType.DiscreteInput;
+
+ /// <summary>
+ /// Parses a Modbus tag address such as <c>coil:1</c> or <c>holding-register:10:UINT</c>.
+ /// </summary>
+ /// <exception cref="PlcInvalidFieldException">The address is not a supported Modbus tag address.</exception>
+ public static ModbusTag Parse(string tagAddress)
+ {
+ if (string.IsNullOrWhiteSpace(tagAddress))
+ {
+ throw new PlcInvalidFieldException("The Modbus tag address must not be empty.");
+ }
+
+ var match = AddressPattern.Match(tagAddress.Trim());
+ if (!match.Success)
+ {
+ throw new PlcInvalidFieldException(
+ $"'{tagAddress}' is not a supported Modbus tag address. Expected " +
+ "{area}:{address}[:{TYPE}], for example holding-register:1:INT; the areas are coil, " +
+ "discrete-input, holding-register and input-register, and the address counts from 1. " +
+ "Array selections, string lengths, tag configuration, extended registers and short " +
+ "addresses such as 40001 are not supported yet.");
+ }
+
+ var type = ParseArea(match.Groups["area"].Value);
+ var address = int.Parse(match.Groups["address"].Value, CultureInfo.InvariantCulture);
+
+ ModbusDataType dataType;
+ var typeGroup = match.Groups["datatype"];
+ if (typeGroup.Success)
+ {
+ if (!TryParseDataType(typeGroup.Value, out dataType))
+ {
+ throw new PlcInvalidFieldException($"Unknown Modbus data type '{typeGroup.Value}'.");
+ }
+ }
+ else
+ {
+ dataType = type == TagType.Coil || type == TagType.DiscreteInput
+ ? ModbusDataType.BOOL
+ : ModbusDataType.INT;
+ }
+
+ return new ModbusTag(type, address, dataType);
+ }
+
+ public override string ToString() => $"{AreaName(Type)}:{Address}:{DataType}";
+
+ private static TagType ParseArea(string area)
+ {
+ switch (area.ToLowerInvariant())
+ {
+ case "coil":
+ return TagType.Coil;
+ case "discrete-input":
+ return TagType.DiscreteInput;
+ case "holding-register":
+ return TagType.HoldingRegister;
+ default:
+ return TagType.InputRegister;
+ }
+ }
Review Comment:
Fixed in 1b8be3f7d25f4a044afbd5ca375c97dc5ca6e33d. ParseArea now explicitly recognizes input-register and throws PlcInvalidFieldException for every other value, so an unknown area can no longer be interpreted as an input register. Added regression coverage for unknown area names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#issuecomment-5970615003
@sruehl this PR is ready for review.
This PR adds the Modbus TCP and RTU drivers for plc4net, built on the model generated from modbus.mspec, using plc4j's tag syntax and connection parameters. It also contains the two generator fixes that model needs.
The build and all plc4net tests pass locally. The .NET workflows are waiting for a committer's approval before they can run. Could you please take a look? Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247737566
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
Follow-up: I squashed the two follow-up commits. The complete fix is now in 1b8be3f7d2 (fix(plc4net): harden Modbus response validation); the implementation and validation described above are unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247737566
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
Follow-up: I squashed the two follow-up commits. The complete fix is now in 1b8be3f7d2 (fix(plc4net): harden Modbus response validation); the implementation and validation described above are unchanged.
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
Fixed in 1b8be3f. The RTU lookahead scan now derives the expected length from scalar bytes, uses a span only for CRC validation, and parses the candidate through a bounded ReadBuffer window over the original receive array. It no longer allocates a header array, a range-slice array, or candidate.ToArray() for each scanned offset. The window also prevents the generated parser from reading beyond the candidate frame. Added ReadBuffer window boundary coverage; the full plc4net solution passes (252 SPI, 261 Modbus, 141 code-gen tests).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247708897
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
Fixed in e4a44e4ca3. The RTU lookahead scan now derives the expected length from scalar bytes, uses a span only for CRC validation, and parses the candidate through a bounded ReadBuffer window over the original receive array. It no longer allocates a header array, a range-slice array, or candidate.ToArray() for each scanned offset. The window also prevents the generated parser from reading beyond the candidate frame. Added ReadBuffer window boundary coverage; the full plc4net solution passes (252 SPI, 261 Modbus, 141 code-gen tests).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247856627
##########
plc4net/drivers/modbus/ModbusTag.cs:
##########
@@ -0,0 +1,212 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using System.Globalization;
+using System.Text.RegularExpressions;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.exceptions;
+using org.apache.plc4net.model;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// A Modbus tag: an address on a Modbus device.
+ /// </summary>
+ /// <remarks>
+ /// <para>
+ /// The syntax is the one plc4j uses, so the same address names the same register from
+ /// either language: <c>{area}:{address}[:{TYPE}]</c>.
+ /// </para>
+ /// <list type="bullet">
+ /// <item><c>coil:{address}</c> - a single bit, read and written (functions 0x01, 0x05).</item>
+ /// <item><c>discrete-input:{address}</c> - a single bit, read only (0x02).</item>
+ /// <item><c>holding-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read and
+ /// written (0x03, 0x06); <c>INT</c> unless stated.</item>
+ /// <item><c>input-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read only (0x04).</item>
+ /// </list>
+ /// <para>
+ /// The address is the register number as a Modbus user counts it, from 1: the request
+ /// carries <c>address - 1</c>, so <c>holding-register:1</c> is the register at wire
+ /// address 0.
+ /// </para>
+ /// <para>
+ /// Array selections, string lengths, tag configuration, extended registers, the short
+ /// forms (<c>40001</c>, <c>4x00001</c>) and the wider data types are accepted by plc4j
+ /// but not supported here yet; they are rejected rather than read as something else.
+ /// </para>
+ /// </remarks>
+ public sealed class ModbusTag : IPlcTag
+ {
+ public enum TagType
+ {
+ Coil,
+ HoldingRegister,
+ DiscreteInput,
+ InputRegister
+ }
+
+ /// <summary>The highest register number: wire address 65535.</summary>
+ public const int MaxAddress = 65536;
+
+ // [0-9], not \d: in .NET \d matches every Unicode decimal digit, so a
+ // non-ASCII address would pass the pattern and blow up int.Parse below
+ // instead of failing here as a PlcInvalidFieldException.
+ private static readonly Regex AddressPattern = new Regex(
+ @"^(?<area>coil|discrete-input|holding-register|input-register):(?<address>[0-9]{1,9})(:(?<datatype>[A-Za-z_]+))?$",
+ RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
+
+ public ModbusTag(TagType type, int address, ModbusDataType dataType)
+ {
+ if (address < 1 || address > MaxAddress)
+ {
+ throw new PlcInvalidFieldException(
+ $"The Modbus address must be between 1 and {MaxAddress}, but was {address}.");
+ }
+ if (!IsSupported(type, dataType))
+ {
+ throw new PlcInvalidFieldException(
+ $"The data type {dataType} is not supported for {AreaName(type)} tags yet; " +
+ $"supported: {SupportedTypes(type)}.");
+ }
+
+ Type = type;
+ Address = address;
+ DataType = dataType;
+ }
+
+ public TagType Type { get; }
+
+ /// <summary>The register number, counted from 1.</summary>
+ public int Address { get; }
+
+ /// <summary>The address the request carries: <see cref="Address"/> - 1.</summary>
+ public ushort WireAddress => (ushort)(Address - 1);
+
+ public ModbusDataType DataType { get; }
+
+ /// <summary>Whether the tag is a single bit (a coil or a discrete input) rather than a register.</summary>
+ public bool IsBit => Type == TagType.Coil || Type == TagType.DiscreteInput;
+
+ /// <summary>
+ /// Parses a Modbus tag address such as <c>coil:1</c> or <c>holding-register:10:UINT</c>.
+ /// </summary>
+ /// <exception cref="PlcInvalidFieldException">The address is not a supported Modbus tag address.</exception>
+ public static ModbusTag Parse(string tagAddress)
+ {
+ if (string.IsNullOrWhiteSpace(tagAddress))
+ {
+ throw new PlcInvalidFieldException("The Modbus tag address must not be empty.");
+ }
+
+ var match = AddressPattern.Match(tagAddress.Trim());
+ if (!match.Success)
+ {
+ throw new PlcInvalidFieldException(
+ $"'{tagAddress}' is not a supported Modbus tag address. Expected " +
+ "{area}:{address}[:{TYPE}], for example holding-register:1:INT; the areas are coil, " +
+ "discrete-input, holding-register and input-register, and the address counts from 1. " +
+ "Array selections, string lengths, tag configuration, extended registers and short " +
+ "addresses such as 40001 are not supported yet.");
+ }
+
+ var type = ParseArea(match.Groups["area"].Value);
+ var address = int.Parse(match.Groups["address"].Value, CultureInfo.InvariantCulture);
+
+ ModbusDataType dataType;
+ var typeGroup = match.Groups["datatype"];
+ if (typeGroup.Success)
+ {
+ if (!TryParseDataType(typeGroup.Value, out dataType))
+ {
+ throw new PlcInvalidFieldException($"Unknown Modbus data type '{typeGroup.Value}'.");
+ }
+ }
+ else
+ {
+ dataType = type == TagType.Coil || type == TagType.DiscreteInput
+ ? ModbusDataType.BOOL
+ : ModbusDataType.INT;
+ }
+
+ return new ModbusTag(type, address, dataType);
+ }
+
+ public override string ToString() => $"{AreaName(Type)}:{Address}:{DataType}";
+
+ private static TagType ParseArea(string area)
+ {
+ switch (area.ToLowerInvariant())
+ {
+ case "coil":
+ return TagType.Coil;
+ case "discrete-input":
+ return TagType.DiscreteInput;
+ case "holding-register":
+ return TagType.HoldingRegister;
+ default:
+ return TagType.InputRegister;
+ }
+ }
Review Comment:
Fixed in 1b8be3f7d25f4a044afbd5ca375c97dc5ca6e33d. ParseArea now explicitly recognizes input-register and throws PlcInvalidFieldException for every other value, so an unknown area can no longer be interpreted as an input register. Added regression coverage for unknown area names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247839924
##########
plc4net/drivers/modbus/ModbusTag.cs:
##########
@@ -0,0 +1,212 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using System.Globalization;
+using System.Text.RegularExpressions;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.exceptions;
+using org.apache.plc4net.model;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// A Modbus tag: an address on a Modbus device.
+ /// </summary>
+ /// <remarks>
+ /// <para>
+ /// The syntax is the one plc4j uses, so the same address names the same register from
+ /// either language: <c>{area}:{address}[:{TYPE}]</c>.
+ /// </para>
+ /// <list type="bullet">
+ /// <item><c>coil:{address}</c> - a single bit, read and written (functions 0x01, 0x05).</item>
+ /// <item><c>discrete-input:{address}</c> - a single bit, read only (0x02).</item>
+ /// <item><c>holding-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read and
+ /// written (0x03, 0x06); <c>INT</c> unless stated.</item>
+ /// <item><c>input-register:{address}[:INT|UINT|WORD]</c> - a 16-bit register, read only (0x04).</item>
+ /// </list>
+ /// <para>
+ /// The address is the register number as a Modbus user counts it, from 1: the request
+ /// carries <c>address - 1</c>, so <c>holding-register:1</c> is the register at wire
+ /// address 0.
+ /// </para>
+ /// <para>
+ /// Array selections, string lengths, tag configuration, extended registers, the short
+ /// forms (<c>40001</c>, <c>4x00001</c>) and the wider data types are accepted by plc4j
+ /// but not supported here yet; they are rejected rather than read as something else.
+ /// </para>
+ /// </remarks>
+ public sealed class ModbusTag : IPlcTag
+ {
+ public enum TagType
+ {
+ Coil,
+ HoldingRegister,
+ DiscreteInput,
+ InputRegister
+ }
+
+ /// <summary>The highest register number: wire address 65535.</summary>
+ public const int MaxAddress = 65536;
+
+ // [0-9], not \d: in .NET \d matches every Unicode decimal digit, so a
+ // non-ASCII address would pass the pattern and blow up int.Parse below
+ // instead of failing here as a PlcInvalidFieldException.
+ private static readonly Regex AddressPattern = new Regex(
+ @"^(?<area>coil|discrete-input|holding-register|input-register):(?<address>[0-9]{1,9})(:(?<datatype>[A-Za-z_]+))?$",
+ RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
+
+ public ModbusTag(TagType type, int address, ModbusDataType dataType)
+ {
+ if (address < 1 || address > MaxAddress)
+ {
+ throw new PlcInvalidFieldException(
+ $"The Modbus address must be between 1 and {MaxAddress}, but was {address}.");
+ }
+ if (!IsSupported(type, dataType))
+ {
+ throw new PlcInvalidFieldException(
+ $"The data type {dataType} is not supported for {AreaName(type)} tags yet; " +
+ $"supported: {SupportedTypes(type)}.");
+ }
+
+ Type = type;
+ Address = address;
+ DataType = dataType;
+ }
+
+ public TagType Type { get; }
+
+ /// <summary>The register number, counted from 1.</summary>
+ public int Address { get; }
+
+ /// <summary>The address the request carries: <see cref="Address"/> - 1.</summary>
+ public ushort WireAddress => (ushort)(Address - 1);
+
+ public ModbusDataType DataType { get; }
+
+ /// <summary>Whether the tag is a single bit (a coil or a discrete input) rather than a register.</summary>
+ public bool IsBit => Type == TagType.Coil || Type == TagType.DiscreteInput;
+
+ /// <summary>
+ /// Parses a Modbus tag address such as <c>coil:1</c> or <c>holding-register:10:UINT</c>.
+ /// </summary>
+ /// <exception cref="PlcInvalidFieldException">The address is not a supported Modbus tag address.</exception>
+ public static ModbusTag Parse(string tagAddress)
+ {
+ if (string.IsNullOrWhiteSpace(tagAddress))
+ {
+ throw new PlcInvalidFieldException("The Modbus tag address must not be empty.");
+ }
+
+ var match = AddressPattern.Match(tagAddress.Trim());
+ if (!match.Success)
+ {
+ throw new PlcInvalidFieldException(
+ $"'{tagAddress}' is not a supported Modbus tag address. Expected " +
+ "{area}:{address}[:{TYPE}], for example holding-register:1:INT; the areas are coil, " +
+ "discrete-input, holding-register and input-register, and the address counts from 1. " +
+ "Array selections, string lengths, tag configuration, extended registers and short " +
+ "addresses such as 40001 are not supported yet.");
+ }
+
+ var type = ParseArea(match.Groups["area"].Value);
+ var address = int.Parse(match.Groups["address"].Value, CultureInfo.InvariantCulture);
+
+ ModbusDataType dataType;
+ var typeGroup = match.Groups["datatype"];
+ if (typeGroup.Success)
+ {
+ if (!TryParseDataType(typeGroup.Value, out dataType))
+ {
+ throw new PlcInvalidFieldException($"Unknown Modbus data type '{typeGroup.Value}'.");
+ }
+ }
+ else
+ {
+ dataType = type == TagType.Coil || type == TagType.DiscreteInput
+ ? ModbusDataType.BOOL
+ : ModbusDataType.INT;
+ }
+
+ return new ModbusTag(type, address, dataType);
+ }
+
+ public override string ToString() => $"{AreaName(Type)}:{Address}:{DataType}";
+
+ private static TagType ParseArea(string area)
+ {
+ switch (area.ToLowerInvariant())
+ {
+ case "coil":
+ return TagType.Coil;
+ case "discrete-input":
+ return TagType.DiscreteInput;
+ case "holding-register":
+ return TagType.HoldingRegister;
+ default:
+ return TagType.InputRegister;
+ }
+ }
Review Comment:
Fixed in 1b8be3f7d25f4a044afbd5ca375c97dc5ca6e33d. ParseArea now explicitly recognizes input-register and throws PlcInvalidFieldException for every other value, so an unknown area can no longer be interpreted as an input register. Added regression coverage for unknown area names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl merged PR #2792:
URL: https://github.com/apache/plc4x/pull/2792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4net): add the Modbus TCP and RTU drivers
Posted by "TomNewChao (via GitHub)" <gi...@apache.org>.
TomNewChao commented on code in PR #2792:
URL: https://github.com/apache/plc4x/pull/2792#discussion_r4247708897
##########
plc4net/drivers/modbus/ModbusRtuMessageCodec.cs:
##########
@@ -0,0 +1,164 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+using System;
+using org.apache.plc4net.drivers.modbus.readwrite.model;
+using org.apache.plc4net.spi.drivers;
+using org.apache.plc4net.spi.generation;
+using org.apache.plc4net.spi.transports;
+
+namespace org.apache.plc4net.drivers.modbus
+{
+ /// <summary>
+ /// Frames Modbus RTU responses. An RTU frame carries no length field, so the
+ /// length is derived from the function code (and, for reads, the byte count).
+ /// Once that many bytes are there, the frame is accepted only if the generated
+ /// <see cref="ModbusRtuADU"/> parses it, which includes the CRC; otherwise one byte is
+ /// dropped and the search goes on, so noise in front of a frame, a corrupted frame, or the
+ /// echo of the request that a half-duplex adapter sends back cannot hide the good frame
+ /// behind it.
+ /// </summary>
+ internal sealed class ModbusRtuMessageCodec : MessageCodecBase<ModbusRtuADU>
+ {
+ // Address, function code, and the byte that is a byte count for reads
+ // and an exception code for errors.
+ private const int HeaderSize = 3;
+
+ // A read response carries at most 250 bytes of data (125 registers), so
+ // the whole frame stays within the 256-byte RTU limit.
+ private const int MaxByteCount = 250;
+
+ // How much of the buffer is searched for a good frame behind a header that
+ // announces more than has arrived. The bound also caps the resync cost on a
+ // persistently noisy line, where every offset re-parses a candidate frame. A
+ // valid answer that starts further into the buffer is not found and ends as
+ // a request timeout.
+ private const int LookaheadBytes = 512;
+
+ public ModbusRtuMessageCodec(ITransportInstance transport, Action<ModbusRtuADU> handler)
+ : base("Modbus RTU", transport, handler)
+ {
+ }
+
+ protected override int GetMinimumHeaderSize() => HeaderSize;
+
+ protected override int CalculateTotalMessageSize(byte[] header, int availableBytes)
+ {
+ var total = ExpectedLength(header);
+ if (availableBytes < total)
+ {
+ // The frame this header announces has not arrived in full, so wait for it -
+ // unless a whole good frame is already buffered behind it. The header was
+ // noise then, and waiting would keep that frame from being looked at (noise
+ // that happens to announce a long read response would hold up the real
+ // answer until the timeout).
+ if (HasCompleteFrameBehind(availableBytes))
+ {
+ throw new MessageCodecException("Noise in front of a frame.");
+ }
+ return total;
+ }
+
+ if (!IsFrame(TransportInstance.PeekReadableBytes(total)))
+ {
+ // Not a frame after all (a failed CRC, most likely): the base class
+ // drops a byte and tries again from the next one.
+ throw new MessageCodecException("Not a valid Modbus RTU frame.");
+ }
+ return total;
+ }
+
+ private bool HasCompleteFrameBehind(int availableBytes)
+ {
+ var buffered = TransportInstance.PeekReadableBytes(Math.Min(availableBytes, LookaheadBytes));
+ for (var offset = 1; offset + HeaderSize <= buffered.Length; offset++)
+ {
+ int length;
+ try
+ {
+ length = ExpectedLength(new[] { buffered[offset], buffered[offset + 1], buffered[offset + 2] });
+ }
+ catch (MessageCodecException)
+ {
+ continue; // nothing that could be a frame starts here
+ }
+
+ if (offset + length <= buffered.Length
+ && IsFrame(buffered[offset..(offset + length)]))
+ {
+ return true;
+ }
+ }
Review Comment:
Fixed in 1b8be3f7d25f4a044afbd5ca375c97dc5ca6e33d. The RTU lookahead scan now derives the expected length from scalar bytes, uses a span only for CRC validation, and parses the candidate through a bounded ReadBuffer window over the original receive array. It no longer allocates a header array, a range-slice array, or candidate.ToArray() for each scanned offset. The window also prevents the generated parser from reading beyond the candidate frame. Added ReadBuffer window boundary coverage; the full plc4net solution passes (252 SPI, 261 Modbus, 141 code-gen tests).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org