` + ArrayExpressionPattern + `)?`
+)
+
+var (
+ // expressionPattern matches a trailing run of strictly numeric bracket groups.
+ expressionPattern = regexp.MustCompile(ArrayExpressionPattern + `$`)
+
+ // wholeExpressionPattern matches an expression in its entirety.
+ wholeExpressionPattern = regexp.MustCompile(`^` + ArrayExpressionPattern + `$`)
+
+ // groupPattern matches one bracket group, whose content is one or more comma-separated
+ // dimensions.
+ groupPattern = regexp.MustCompile(`\[([^\]]*)]`)
+
+ // singleDimensionPattern matches one dimension of a group.
+ singleDimensionPattern = regexp.MustCompile(`^(\d+)(?:\.\.(\d+))?(?:;(\d+))?$`)
+
+ // legacyAfterType matches an address in the pre-migration shape "address:TYPE[n]", where the
+ // selection came after the type and meant a count.
+ legacyAfterType = regexp.MustCompile(`^(.+?):([A-Za-z_][A-Za-z_0-9]*(?:\(\d+\))?)\[(\d+)]$`)
+
+ // legacyCountSuffix matches the pre-migration shape "address:TYPE:n", where a count trailed.
+ legacyCountSuffix = regexp.MustCompile(`^(.+?):([A-Za-z_][A-Za-z_0-9]*):(\d+)$`)
+)
+
+// AddressPart is the part of an address before any trailing array expression. An address with no
+// such expression is returned unchanged - including one whose brackets are not numeric, such as
+// an OPC UA string identifier that happens to contain them.
+func AddressPart(address string) string {
+ if loc := expressionPattern.FindStringIndex(address); loc != nil {
+ return address[:loc[0]]
+ }
+ return address
+}
+
+// ExpressionPart is the trailing array expression of an address, or the empty string if it has
+// none.
+func ExpressionPart(address string) string {
+ if match := expressionPattern.FindString(address); match != "" {
+ return match
+ }
+ return ""
+}
+
+// SelectsSingleElement reports whether an expression selects a single element rather than an
+// array, which decides what a tag reports from GetArrayInfo: a bare index yields a scalar, a
+// range yields an array even when it spans one element. [1] is a scalar and [1..1] is an array
+// of one, so equal bounds alone cannot tell them apart - only the written form can.
+//
+// An expression is a single element when every one of its dimensions is a bare index.
+func SelectsSingleElement(expression string) bool {
+ return expression != "" && !strings.Contains(expression, "..")
+}
+
+// ParseArrayExpression parses an array expression into one ArrayInfo per dimension, in written
+// order. An empty expression selects nothing and yields no dimensions.
+//
+// The address is quoted back in any error so the caller can find it.
+func ParseArrayExpression(expression string, address string, constraints AddressConstraints) ([]apiModel.ArrayInfo, error) {
+ if expression == "" {
+ return nil, nil
+ }
+ if !wholeExpressionPattern.MatchString(expression) {
+ return nil, fmt.Errorf("invalid array expression '%s' in tag '%s': expected [index], "+
+ "[lo..hi] or either with a ';base', repeated once per dimension", expression, address)
+ }
+
+ var dimensions []apiModel.ArrayInfo
+ for _, group := range groupPattern.FindAllStringSubmatch(expression, -1) {
+ // A group may hold several dimensions separated by commas - the spelling Allen-Bradley
+ // and others use. "[1..2,3..4]" and "[1..2][3..4]" are the same selection.
+ for _, part := range strings.Split(group[1], ",") {
+ dimension, err := parseDimension(part, address, constraints)
+ if err != nil {
+ return nil, err
+ }
+ dimensions = append(dimensions, dimension)
+ }
+ }
+
+ if len(dimensions) > constraints.MaxDimensions {
+ return nil, fmt.Errorf("array expression '%s' in tag '%s' has %d dimensions, but this "+
+ "protocol carries at most %d", expression, address, len(dimensions), constraints.MaxDimensions)
+ }
+ if constraints.OnlyTrailingDimensionMayBeRange {
+ for i := 0; i < len(dimensions)-1; i++ {
+ // What matters is how the dimension was written, not how wide it turned out to be:
+ // "[1..1]" is a range that happens to span one element, and letting it pass here would
+ // hand the driver a leading range it has no element count for.
+ if dimensions[i].IsRange() {
+ return nil, fmt.Errorf("array expression '%s' in tag '%s' writes dimension %d as "+
+ "a range, but this protocol carries one element count for the whole address, "+
+ "so only the last dimension may be a range",
+ expression, address, i+1)
+ }
+ }
+ }
+ return dimensions, nil
+}
+
+func parseDimension(part string, address string, constraints AddressConstraints) (apiModel.ArrayInfo, error) {
+ match := singleDimensionPattern.FindStringSubmatch(part)
+ if match == nil {
+ return nil, fmt.Errorf("invalid array dimension '%s' in tag '%s'", part, address)
+ }
+
+ lowerBound, err := parseIndex(match[1], part, address)
+ if err != nil {
+ return nil, err
+ }
+ isRange := match[2] != ""
+ upperBound := lowerBound
+ if isRange {
+ if upperBound, err = parseIndex(match[2], part, address); err != nil {
+ return nil, err
+ }
+ }
+ var base uint32
+ if match[3] != "" {
+ if base, err = parseIndex(match[3], part, address); err != nil {
+ return nil, err
+ }
+ }
+
+ if upperBound < lowerBound {
+ return nil, fmt.Errorf("invalid array range '%s' in tag '%s': the upper bound %d is "+
+ "below the lower bound %d", part, address, upperBound, lowerBound)
+ }
+ // The inclusive size is computed in a uint32, so a range spanning more than that would wrap -
+ // [0..4294967295] would report zero elements from a selection the syntax accepted.
+ if (uint64(upperBound) - uint64(lowerBound) + 1) > math.MaxUint32 {
+ return nil, fmt.Errorf("invalid array range '%s' in tag '%s': it spans %d elements, more than can be counted",
+ part, address, uint64(upperBound)-uint64(lowerBound)+1)
+ }
+ if lowerBound < base {
+ return nil, fmt.Errorf("invalid array range '%s' in tag '%s': index %d lies below the "+
+ "declared lower bound %d", part, address, lowerBound, base)
+ }
+ // The bound applies to the offset the protocol actually encodes - the start of the selection
+ // - not to its last element. A CIP request carries a start index and an element count, so
+ // [0..300] is encodable where [300] is not.
+ if lowerBound-base > constraints.MaxIndex {
+ return nil, fmt.Errorf("invalid array range '%s' in tag '%s': index %d is out of range "+
+ "0 to %d for this protocol", part, address, lowerBound-base, constraints.MaxIndex)
+ }
+
+ return &DefaultArrayInfo{
+ LowerBound: lowerBound,
+ UpperBound: upperBound,
+ Base: base,
+ Range: isRange,
+ }, nil
+}
+
+func parseIndex(value string, part string, address string) (uint32, error) {
+ parsed, err := strconv.ParseUint(value, 10, 32)
+ if err != nil {
+ return 0, fmt.Errorf("invalid array range '%s' in tag '%s': '%s' is not a number this "+
+ "protocol can address", part, address, value)
+ }
+ return uint32(parsed), nil
+}
+
+// RenderArrayExpression renders dimensions back to their canonical form: one bracket per
+// dimension, omitting what is defaulted - a base of 0 is dropped, and a bare index stays bare.
+// The comma-separated spelling is accepted on input but never produced, so [1..2,3..4] renders as
+// [1..2][3..4]. A one-element range still renders as a range: [8..8] is an array of one and [8]
+// is a scalar, so collapsing it would change what the address means.
+func RenderArrayExpression(dimensions []apiModel.ArrayInfo) string {
+ if len(dimensions) == 0 {
+ return ""
+ }
+ var sb strings.Builder
+ for _, dimension := range dimensions {
+ sb.WriteString("[")
+ sb.WriteString(strconv.FormatUint(uint64(dimension.GetLowerBound()), 10))
+ if dimension.IsRange() {
+ sb.WriteString("..")
+ sb.WriteString(strconv.FormatUint(uint64(dimension.GetUpperBound()), 10))
+ }
+ if dimension.GetBase() != 0 {
+ sb.WriteString(";")
+ sb.WriteString(strconv.FormatUint(uint64(dimension.GetBase()), 10))
+ }
+ sb.WriteString("]")
+ }
+ return sb.String()
+}
+
+// CurrentFormOf returns how to rewrite an address written before the array notation was unified,
+// and whether one could be worked out.
+//
+// The brackets moved from after the type to before it, and a count became a range. An upgrading
+// user who sees only "does not match pattern" has to work that out from a regex; this hands them
+// the address they meant.
+func CurrentFormOf(address string) (string, bool) {
+ if match := legacyAfterType.FindStringSubmatch(address); match != nil {
+ return match[1] + rangeFor(match[3]) + ":" + match[2], true
+ }
+ if match := legacyCountSuffix.FindStringSubmatch(address); match != nil {
+ return match[1] + rangeFor(match[3]) + ":" + match[2], true
+ }
+ return "", false
+}
+
+func rangeFor(count string) string {
+ elements, err := strconv.Atoi(count)
+ if err != nil || elements <= 1 {
+ return "[0]"
+ }
+ return "[0.." + strconv.Itoa(elements-1) + "]"
+}
+
+// InvalidAddressError reports an address the driver could not parse, naming the form it expected
+// and - when the address looks like one written before the notation was unified - the address to
+// write instead.
+func InvalidAddressError(address string, expectedForm string) error {
+ message := fmt.Sprintf("invalid address '%s': expected %s", address, expectedForm)
+ if current, ok := CurrentFormOf(address); ok {
+ message += fmt.Sprintf(". The array notation moved before the type and a count became a "+
+ "range, so this address is now written '%s'", current)
+ }
+ return fmt.Errorf("%s", message)
+}
diff --git a/plc4go/spi/model/ArrayNotationParser_test.go b/plc4go/spi/model/ArrayNotationParser_test.go
new file mode 100644
index 00000000000..5e03f60b612
--- /dev/null
+++ b/plc4go/spi/model/ArrayNotationParser_test.go
@@ -0,0 +1,377 @@
+/*
+ * 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.
+ */
+
+package model
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// The one definition of the array notation in plc4go, per the grammar contract shared with
+// plc4j (specs/002-unified-array-notation/contracts/array-notation-grammar.md).
+//
+// The cases below are the plc4j suite's cases, deliberately the same ones rather than merely
+// similar: the two bindings share no code, so the only evidence that an address means the same
+// thing in both is that both satisfy the same specification against the same inputs.
+
+// --- the semantics table ---
+
+func TestArrayNotationParser_SingleDimensionResolvesToTheDocumentedOffsets(t *testing.T) {
+ for _, tt := range []struct {
+ expression string
+ size uint32
+ first uint32
+ last uint32
+ }{
+ {"[4]", 1, 4, 4},
+ {"[0..7]", 8, 0, 7},
+ {"[4;1]", 1, 3, 3},
+ {"[4..7;1]", 4, 3, 6},
+ {"[0]", 1, 0, 0},
+ {"[7..7]", 1, 7, 7},
+ } {
+ t.Run(tt.expression, func(t *testing.T) {
+ dimensions, err := ParseArrayExpression(tt.expression, "tag"+tt.expression, Unconstrained)
+ require.NoError(t, err)
+ require.Len(t, dimensions, 1)
+
+ only := dimensions[0]
+ assert.Equal(t, tt.size, only.GetSize(), "size")
+ assert.Equal(t, tt.first, only.GetLowerBound()-only.GetBase(), "first offset")
+ assert.Equal(t, tt.last, only.GetUpperBound()-only.GetBase(), "last offset")
+ })
+ }
+}
+
+// A bare index and a one-element range cover the same element but are not the same selection:
+// the first yields a scalar and the second an array of one.
+func TestArrayNotationParser_ABareIndexIsNotTheSameAsAOneElementRange(t *testing.T) {
+ index, err := ParseArrayExpression("[4]", "tag[4]", Unconstrained)
+ require.NoError(t, err)
+ arrayRange, err := ParseArrayExpression("[4..4]", "tag[4..4]", Unconstrained)
+ require.NoError(t, err)
+
+ assert.NotEqual(t, arrayRange, index)
+ assert.False(t, index[0].IsRange())
+ assert.True(t, arrayRange[0].IsRange())
+ assert.Equal(t, index[0].GetLowerBound(), arrayRange[0].GetLowerBound())
+ assert.Equal(t, uint32(1), index[0].GetSize())
+ assert.Equal(t, uint32(1), arrayRange[0].GetSize())
+}
+
+func TestArrayNotationParser_WrittenBoundsArePreservedNotResolved(t *testing.T) {
+ dimensions, err := ParseArrayExpression("[4..7;1]", "tag[4..7;1]", Unconstrained)
+ require.NoError(t, err)
+
+ assert.Equal(t, uint32(4), dimensions[0].GetLowerBound(), "lower bound is as written")
+ assert.Equal(t, uint32(7), dimensions[0].GetUpperBound(), "upper bound is as written")
+ assert.Equal(t, uint32(1), dimensions[0].GetBase(), "declared base")
+ assert.Equal(t, uint32(4), dimensions[0].GetSize())
+}
+
+func TestArrayNotationParser_BaseDefaultsToZero(t *testing.T) {
+ dimensions, err := ParseArrayExpression("[4]", "tag[4]", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, uint32(0), dimensions[0].GetBase())
+}
+
+func TestArrayNotationParser_MultipleDimensionsKeepTheirWrittenOrder(t *testing.T) {
+ dimensions, err := ParseArrayExpression("[1..2][0..5]", "tag[1..2][0..5]", Unconstrained)
+ require.NoError(t, err)
+
+ require.Len(t, dimensions, 2)
+ assert.Equal(t, uint32(1), dimensions[0].GetLowerBound())
+ assert.Equal(t, uint32(2), dimensions[0].GetUpperBound())
+ assert.Equal(t, uint32(0), dimensions[1].GetLowerBound())
+ assert.Equal(t, uint32(5), dimensions[1].GetUpperBound())
+}
+
+func TestArrayNotationParser_EachDimensionCarriesItsOwnBase(t *testing.T) {
+ dimensions, err := ParseArrayExpression("[4..7;1][7..10;2]", "tag[4..7;1][7..10;2]", Unconstrained)
+ require.NoError(t, err)
+
+ require.Len(t, dimensions, 2)
+ assert.Equal(t, uint32(3), dimensions[0].GetLowerBound()-dimensions[0].GetBase())
+ assert.Equal(t, uint32(6), dimensions[0].GetUpperBound()-dimensions[0].GetBase())
+ assert.Equal(t, uint32(5), dimensions[1].GetLowerBound()-dimensions[1].GetBase())
+ assert.Equal(t, uint32(8), dimensions[1].GetUpperBound()-dimensions[1].GetBase())
+}
+
+// --- the rejection table ---
+
+func TestArrayNotationParser_MalformedExpressionsAreRejected(t *testing.T) {
+ for _, expression := range []string{
+ "[]", // no index
+ "[7..4]", // upper below lower
+ "[0;1]", // resolved offset is negative
+ "[-1]", // negative component
+ "[1..-2]", // negative component
+ "[a]", // non-numeric
+ "[1..x]", // non-numeric
+ "[1..2;]", // empty base
+ "[1..]", // missing upper bound
+ "[..2]", // missing lower bound
+ "[0,]", // trailing comma
+ "[,1]", // leading comma
+ "[0,,1]", // empty dimension
+ "[0, 1]", // space in the list
+ } {
+ t.Run(expression, func(t *testing.T) {
+ _, err := ParseArrayExpression(expression, "tag"+expression, Unconstrained)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "tag"+expression, "the message must name the address")
+ })
+ }
+}
+
+// --- driver constraints ---
+
+func TestArrayNotationParser_IndexBeyondTheProtocolMaximumIsRejected(t *testing.T) {
+ eip := SingleDimension.WithMaxIndex(255)
+
+ _, err := ParseArrayExpression("[255]", "tag[255]", eip)
+ require.NoError(t, err)
+ // The bound is on where the selection starts, not where it ends: a CIP request carries a
+ // start index and an element count, so a long run from an encodable start is fine.
+ _, err = ParseArrayExpression("[0..300]", "tag[0..300]", eip)
+ require.NoError(t, err)
+ _, err = ParseArrayExpression("[256;1]", "tag[256;1]", eip)
+ require.NoError(t, err)
+
+ _, err = ParseArrayExpression("[256]", "tag[256]", eip)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "255", "the message must name the real bound")
+}
+
+func TestArrayNotationParser_MoreDimensionsThanTheProtocolCarriesIsRejected(t *testing.T) {
+ _, err := ParseArrayExpression("[1][2]", "tag[1][2]", SingleDimension)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "1")
+}
+
+func TestArrayNotationParser_InteriorRangeIsRejectedWhereOnlyTheTrailingDimensionMaySpan(t *testing.T) {
+ trailingOnly := Unconstrained.WithOnlyTrailingDimensionMayBeRange(true)
+
+ _, err := ParseArrayExpression("[1][0..3]", "tag[1][0..3]", trailingOnly)
+ require.NoError(t, err)
+
+ _, err = ParseArrayExpression("[0..3][1]", "tag[0..3][1]", trailingOnly)
+ require.Error(t, err)
+
+ // A one-element range is still a range. Judging this by the span let "[1..1][2]" through,
+ // handing the driver a leading range it has no element count for.
+ _, err = ParseArrayExpression("[1..1][2]", "tag[1..1][2]", trailingOnly)
+ require.Error(t, err)
+
+ // A single index in the same position stays legal, which is what the constraint is for.
+ _, err = ParseArrayExpression("[1][2]", "tag[1][2]", trailingOnly)
+ require.NoError(t, err)
+}
+
+// --- splitting an address ---
+
+func TestArrayNotationParser_TrailingExpressionIsSplitFromTheAddress(t *testing.T) {
+ for _, tt := range []struct{ input, address, expression string }{
+ {"myTag[0..7]", "myTag", "[0..7]"},
+ {"myTag", "myTag", ""},
+ {"a.b[2]", "a.b", "[2]"},
+ {"40001[0..3]", "40001", "[0..3]"},
+ {"t[1..2][0..5]", "t", "[1..2][0..5]"},
+ } {
+ t.Run(tt.input, func(t *testing.T) {
+ assert.Equal(t, tt.address, AddressPart(tt.input))
+ assert.Equal(t, tt.expression, ExpressionPart(tt.input))
+ })
+ }
+}
+
+// Only a strictly numeric trailing run counts. An identifier that happens to contain brackets is
+// left on the address.
+func TestArrayNotationParser_NonNumericBracketsAreNotAnArrayExpression(t *testing.T) {
+ assert.Equal(t, "Some[Node]Name", AddressPart("Some[Node]Name"))
+ assert.Equal(t, "", ExpressionPart("Some[Node]Name"))
+}
+
+// --- rendering back ---
+
+func TestArrayNotationParser_RenderingReproducesTheCanonicalForm(t *testing.T) {
+ for _, expression := range []string{
+ "[4]", "[0..7]", "[4;1]", "[4..7;1]", "[1..2][0..5]", "[4..7;1][7..10;2]",
+ } {
+ t.Run(expression, func(t *testing.T) {
+ dimensions, err := ParseArrayExpression(expression, "tag", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, expression, RenderArrayExpression(dimensions))
+ })
+ }
+}
+
+// Canonical form omits what is defaulted - a base of 0 - but never the range form, because
+// dropping that would turn an array of one into a scalar.
+func TestArrayNotationParser_CanonicalFormOmitsDefaultsButKeepsTheRangeForm(t *testing.T) {
+ for _, tt := range []struct{ written, canonical string }{
+ {"[4..4;0]", "[4..4]"},
+ {"[4;0]", "[4]"},
+ {"[0..7;0]", "[0..7]"},
+ } {
+ t.Run(tt.written, func(t *testing.T) {
+ dimensions, err := ParseArrayExpression(tt.written, "tag", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, tt.canonical, RenderArrayExpression(dimensions))
+ })
+ }
+}
+
+func TestArrayNotationParser_AnAbsentExpressionRendersAsNothing(t *testing.T) {
+ assert.Equal(t, "", RenderArrayExpression(nil))
+ dimensions, err := ParseArrayExpression("", "tag", Unconstrained)
+ require.NoError(t, err)
+ assert.Empty(t, dimensions)
+}
+
+// --- the comma spelling ---
+
+// Allen-Bradley and others write the dimensions of one array inside a single bracket. It is the
+// same selection, so it parses the same - and renders back in the one canonical form.
+func TestArrayNotationParser_TheCommaSpellingIsTheSameSelection(t *testing.T) {
+ for _, tt := range []struct{ comma, brackets string }{
+ {"[0,1]", "[0][1]"},
+ {"[1..2,3..4]", "[1..2][3..4]"},
+ {"[1..2;1,3..4;1]", "[1..2;1][3..4;1]"},
+ {"[0,1,2]", "[0][1][2]"},
+ {"[1..2,3]", "[1..2][3]"},
+ } {
+ t.Run(tt.comma, func(t *testing.T) {
+ viaComma, err := ParseArrayExpression(tt.comma, "tag"+tt.comma, Unconstrained)
+ require.NoError(t, err)
+ viaBrackets, err := ParseArrayExpression(tt.brackets, "tag"+tt.brackets, Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, viaBrackets, viaComma)
+ })
+ }
+}
+
+func TestArrayNotationParser_RenderingAlwaysProducesOneBracketPerDimension(t *testing.T) {
+ for _, tt := range []struct{ written, canonical string }{
+ {"[0,1]", "[0][1]"},
+ {"[1..2,3..4]", "[1..2][3..4]"},
+ {"[0][1]", "[0][1]"},
+ {"[0,1][2]", "[0][1][2]"},
+ } {
+ t.Run(tt.written, func(t *testing.T) {
+ dimensions, err := ParseArrayExpression(tt.written, "tag", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, tt.canonical, RenderArrayExpression(dimensions))
+ })
+ }
+}
+
+func TestArrayNotationParser_TheCommaSpellingIsSplitFromTheAddress(t *testing.T) {
+ assert.Equal(t, "myTag", AddressPart("myTag[1..2,3..4]"))
+ assert.Equal(t, "[1..2,3..4]", ExpressionPart("myTag[1..2,3..4]"))
+}
+
+// --- what the caller receives ---
+
+// GetArrayInfo describes the value the caller gets, so a consumer can decide from it alone
+// whether to render a scalar or a list. A bare index is a scalar; a range is an array even when
+// it spans a single element.
+func TestArrayNotationParser_ABareIndexSelectsAScalarButARangeDoesNot(t *testing.T) {
+ for _, tt := range []struct {
+ expression string
+ scalar bool
+ }{
+ {"[1]", true},
+ {"[4]", true},
+ {"[4;1]", true},
+ {"[1][2]", true},
+ {"[1..1]", false},
+ {"[0..7]", false},
+ {"[4..7;1]", false},
+ {"[1][0..5]", false},
+ {"", false},
+ } {
+ t.Run(tt.expression, func(t *testing.T) {
+ assert.Equal(t, tt.scalar, SelectsSingleElement(tt.expression))
+ })
+ }
+}
+
+// --- guidance for addresses written before the migration ---
+
+func TestArrayNotationParser_AnAddressWrittenBeforeTheMigrationIsRewritten(t *testing.T) {
+ for _, tt := range []struct{ legacy, current string }{
+ {"holding-register:1:INT[4]", "holding-register:1[0..3]:INT"},
+ {"%DB42:28.0:BYTE[8]", "%DB42:28.0[0..7]:BYTE"},
+ {"%DB1:0:STRING(40)[3]", "%DB1:0[0..2]:STRING(40)"},
+ {"D100:WORD[2]", "D100[0..1]:WORD"},
+ {"0x4020/0:DINT[4]", "0x4020/0[0..3]:DINT"},
+ {"myTag:DINT:8", "myTag[0..7]:DINT"},
+ {"foo:INT[1]", "foo[0]:INT"},
+ } {
+ t.Run(tt.legacy, func(t *testing.T) {
+ current, ok := CurrentFormOf(tt.legacy)
+ require.True(t, ok, tt.legacy)
+ assert.Equal(t, tt.current, current)
+ })
+ }
+}
+
+func TestArrayNotationParser_AnAddressThatIsNotInTheOldShapeGetsNoRewrite(t *testing.T) {
+ for _, address := range []string{"myTag", "myTag[0..3]:DINT", "holding-register:1[0..3]:INT", "nonsense"} {
+ t.Run(address, func(t *testing.T) {
+ _, ok := CurrentFormOf(address)
+ assert.False(t, ok, address)
+ })
+ }
+}
+
+// --- round trip ---
+
+func TestArrayNotationParser_ASelectionSurvivesBeingRenderedAndParsedAgain(t *testing.T) {
+ for _, written := range []string{
+ "[4]", "[0..7]", "[4;1]", "[4..7;1]", "[0]", "[7..7]",
+ "[1..2][0..5]", "[4..7;1][7..10;2]", "[0][1][2]",
+ "[0,1]", "[1..2,3..4]", "[0,1][2]", "[1..2;1,3..4;1]",
+ } {
+ t.Run(written, func(t *testing.T) {
+ parsed, err := ParseArrayExpression(written, "tag"+written, Unconstrained)
+ require.NoError(t, err)
+ reparsed, err := ParseArrayExpression(RenderArrayExpression(parsed), "tag", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, parsed, reparsed)
+ })
+ }
+}
+
+// A range the syntax accepts but no count can hold must be refused at the parse, not silently
+// wrapped: [0..4294967295] spans 2^32 elements, which is zero in a uint32.
+func TestParseArrayExpression_refusesARangeThatCannotBeCounted(t *testing.T) {
+ _, err := ParseArrayExpression("[0..4294967295]", "%test[0..4294967295]", Unconstrained)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "more than can be counted")
+
+ // One below the limit still parses, and counts what it says.
+ dimensions, err := ParseArrayExpression("[0..4294967294]", "%test[0..4294967294]", Unconstrained)
+ require.NoError(t, err)
+ assert.Equal(t, uint32(4294967295), dimensions[0].GetSize())
+}
diff --git a/plc4go/spi/model/DefaultArrayInfo.go b/plc4go/spi/model/DefaultArrayInfo.go
index 4e074f228ca..ba6c13066fc 100644
--- a/plc4go/spi/model/DefaultArrayInfo.go
+++ b/plc4go/spi/model/DefaultArrayInfo.go
@@ -27,10 +27,19 @@ var _ apiModel.ArrayInfo = &DefaultArrayInfo{}
type DefaultArrayInfo struct {
LowerBound uint32
UpperBound uint32
+ // Base is the array's declared lower bound; 0 for an array that does not declare one.
+ Base uint32
+ // Range records whether the address wrote this dimension as a range. A one-element range is
+ // still a range, so this cannot be derived from the bounds - see apiModel.ArrayInfo.IsRange.
+ Range bool
}
+// GetSize is the number of elements. Both bounds are inclusive, so {0, 7} is eight elements.
+// This used to return UpperBound-LowerBound, treating the upper bound as exclusive, which
+// disagreed with plc4j about the same address and with the drivers that build from an inclusive
+// range.
func (t *DefaultArrayInfo) GetSize() uint32 {
- return t.UpperBound - t.LowerBound
+ return t.UpperBound - t.LowerBound + 1
}
func (t *DefaultArrayInfo) GetLowerBound() uint32 {
@@ -40,3 +49,11 @@ func (t *DefaultArrayInfo) GetLowerBound() uint32 {
func (t *DefaultArrayInfo) GetUpperBound() uint32 {
return t.UpperBound
}
+
+func (t *DefaultArrayInfo) GetBase() uint32 {
+ return t.Base
+}
+
+func (t *DefaultArrayInfo) IsRange() bool {
+ return t.Range
+}
diff --git a/plc4go/spi/model/DefaultArrayInfo_plc4xgen.go b/plc4go/spi/model/DefaultArrayInfo_plc4xgen.go
index 3210bddcc34..cfcd0651acd 100644
--- a/plc4go/spi/model/DefaultArrayInfo_plc4xgen.go
+++ b/plc4go/spi/model/DefaultArrayInfo_plc4xgen.go
@@ -56,6 +56,14 @@ func (d *DefaultArrayInfo) SerializeWithWriteBuffer(ctx context.Context, writeBu
if err := writeBuffer.WriteUint32("upperBound", 32, d.UpperBound); err != nil {
return err
}
+
+ if err := writeBuffer.WriteUint32("base", 32, d.Base); err != nil {
+ return err
+ }
+
+ if err := writeBuffer.WriteBit("range", d.Range); err != nil {
+ return err
+ }
if err := writeBuffer.PopContext("ArrayInfo"); err != nil {
return err
}
diff --git a/plc4go/spi/values/mocks_test.go b/plc4go/spi/values/mocks_test.go
index 4718687783a..5748792c0a5 100644
--- a/plc4go/spi/values/mocks_test.go
+++ b/plc4go/spi/values/mocks_test.go
@@ -1966,6 +1966,50 @@ func (_m *MockArrayInfo) EXPECT() *MockArrayInfo_Expecter {
return &MockArrayInfo_Expecter{mock: &_m.Mock}
}
+// GetBase provides a mock function for the type MockArrayInfo
+func (_mock *MockArrayInfo) GetBase() uint32 {
+ ret := _mock.Called()
+
+ if len(ret) == 0 {
+ panic("no return value specified for GetBase")
+ }
+
+ var r0 uint32
+ if returnFunc, ok := ret.Get(0).(func() uint32); ok {
+ r0 = returnFunc()
+ } else {
+ r0 = ret.Get(0).(uint32)
+ }
+ return r0
+}
+
+// MockArrayInfo_GetBase_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetBase'
+type MockArrayInfo_GetBase_Call struct {
+ *mock.Call
+}
+
+// GetBase is a helper method to define mock.On call
+func (_e *MockArrayInfo_Expecter) GetBase() *MockArrayInfo_GetBase_Call {
+ return &MockArrayInfo_GetBase_Call{Call: _e.mock.On("GetBase")}
+}
+
+func (_c *MockArrayInfo_GetBase_Call) Run(run func()) *MockArrayInfo_GetBase_Call {
+ _c.Call.Run(func(args mock.Arguments) {
+ run()
+ })
+ return _c
+}
+
+func (_c *MockArrayInfo_GetBase_Call) Return(v uint32) *MockArrayInfo_GetBase_Call {
+ _c.Call.Return(v)
+ return _c
+}
+
+func (_c *MockArrayInfo_GetBase_Call) RunAndReturn(run func() uint32) *MockArrayInfo_GetBase_Call {
+ _c.Call.Return(run)
+ return _c
+}
+
// GetLowerBound provides a mock function for the type MockArrayInfo
func (_mock *MockArrayInfo) GetLowerBound() uint32 {
ret := _mock.Called()
@@ -2098,6 +2142,50 @@ func (_c *MockArrayInfo_GetUpperBound_Call) RunAndReturn(run func() uint32) *Moc
return _c
}
+// IsRange provides a mock function for the type MockArrayInfo
+func (_mock *MockArrayInfo) IsRange() bool {
+ ret := _mock.Called()
+
+ if len(ret) == 0 {
+ panic("no return value specified for IsRange")
+ }
+
+ var r0 bool
+ if returnFunc, ok := ret.Get(0).(func() bool); ok {
+ r0 = returnFunc()
+ } else {
+ r0 = ret.Get(0).(bool)
+ }
+ return r0
+}
+
+// MockArrayInfo_IsRange_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'IsRange'
+type MockArrayInfo_IsRange_Call struct {
+ *mock.Call
+}
+
+// IsRange is a helper method to define mock.On call
+func (_e *MockArrayInfo_Expecter) IsRange() *MockArrayInfo_IsRange_Call {
+ return &MockArrayInfo_IsRange_Call{Call: _e.mock.On("IsRange")}
+}
+
+func (_c *MockArrayInfo_IsRange_Call) Run(run func()) *MockArrayInfo_IsRange_Call {
+ _c.Call.Run(func(args mock.Arguments) {
+ run()
+ })
+ return _c
+}
+
+func (_c *MockArrayInfo_IsRange_Call) Return(b bool) *MockArrayInfo_IsRange_Call {
+ _c.Call.Return(b)
+ return _c
+}
+
+func (_c *MockArrayInfo_IsRange_Call) RunAndReturn(run func() bool) *MockArrayInfo_IsRange_Call {
+ _c.Call.Return(run)
+ return _c
+}
+
// String provides a mock function for the type MockArrayInfo
func (_mock *MockArrayInfo) String() string {
ret := _mock.Called()
diff --git a/plc4go/tests/arraynotation/legacy_addresses_test.go b/plc4go/tests/arraynotation/legacy_addresses_test.go
new file mode 100644
index 00000000000..cc5aa297166
--- /dev/null
+++ b/plc4go/tests/arraynotation/legacy_addresses_test.go
@@ -0,0 +1,124 @@
+/*
+ * 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.
+ */
+
+// Package arraynotation holds the cross-driver checks for the unified array notation. They live
+// outside the driver packages because what they assert is a property of the whole binding: the
+// release notes tell an upgrading user that every address whose meaning changed is either
+// rejected with its replacement named, or listed as one of the two silent changes. That claim is
+// only true if it holds for every driver at once, which is what these tests check.
+package arraynotation
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/apache/plc4x/plc4go/internal/ads"
+ "github.com/apache/plc4x/plc4go/internal/eip"
+ "github.com/apache/plc4x/plc4go/internal/firmata"
+ "github.com/apache/plc4x/plc4go/internal/knxnetip"
+ "github.com/apache/plc4x/plc4go/internal/modbus"
+ "github.com/apache/plc4x/plc4go/internal/s7"
+ "github.com/apache/plc4x/plc4go/internal/simulated"
+ "github.com/apache/plc4x/plc4go/internal/slmp"
+ apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+)
+
+type parseTag func(string) (apiModel.PlcTag, error)
+
+// Every pre-migration address in the release notes must be rejected, and the rejection must name
+// the address to write instead. A rejection alone is not enough: the point of moving the
+// brackets was that an upgrade reports the change rather than quietly returning different data,
+// and a user with a configuration full of old addresses needs to be told what to write.
+func TestEveryLegacyAddressIsRejectedWithItsReplacement(t *testing.T) {
+ for _, c := range []struct {
+ driver string
+ parse parseTag
+ address string
+ replacement string
+ }{
+ {"s7", s7.NewTagHandler().ParseTag, "%M100:INT[10]", "%M100[0..9]:INT"},
+ {"s7 string", s7.NewTagHandler().ParseTag, "%DB69.DBX68:WSTRING[3]", "%DB69.DBX68[0..2]:WSTRING"},
+ {"modbus", modbus.NewTagHandler().ParseTag, "holding-register:1:INT[4]", "holding-register:1[0..3]:INT"},
+ {"slmp", slmp.NewTagHandler().ParseTag, "D100:INT[4]", "D100[0..3]:INT"},
+ {"eip", eip.NewTagHandler().ParseTag, "%rate:DINT:4", "%rate[0..3]:DINT"},
+ {"simulated", simulated.NewTagHandler().ParseTag, "RANDOM/foo:INT[4]", "RANDOM/foo[0..3]:INT"},
+ {"knxnetip memory", knxnetip.NewTagHandler().ParseTag, "1.2.3#4B1C:UINT[4]", "1.2.3#4B1C[0..3]:UINT"},
+ {"ads direct", ads.NewTagHandler().ParseTag, "0x4020/0:DINT[4]", "0x4020/0[0..3]:DINT"},
+ // The start-and-count form was a plc4go extension with no counterpart in plc4j.
+ {"ads start-and-count", ads.NewTagHandler().ParseTag, "MAIN.g_arr[2:4]", "MAIN.g_arr[2..5]"},
+ } {
+ t.Run(c.driver, func(t *testing.T) {
+ _, err := c.parse(c.address)
+ require.Error(t, err, c.address)
+ assert.Contains(t, err.Error(), c.replacement,
+ "the rejection must name the address to write instead")
+ })
+ }
+}
+
+// The two addresses that parse before and after, and only change meaning. Neither can be
+// rejected, so the release notes carry them - and these tests are what keeps that list honest.
+func TestTheSilentChangesAreExactlyTheTwoThatAreDocumented(t *testing.T) {
+ // Firmata: [n] was a run of n pins and is now the pin at index n.
+ pin, err := firmata.NewTagHandler().ParseTag("digital:2[3]")
+ require.NoError(t, err)
+ assert.Equal(t, "digital:5", pin.GetAddressString(), "pin 5, not three pins from pin 2")
+ assert.Empty(t, pin.GetArrayInfo(), "one pin is a scalar")
+
+ // ADS: [n] was a count of n elements and is now the element at index n.
+ element, err := ads.NewTagHandler().ParseTag("MAIN.g_arr[3]")
+ require.NoError(t, err)
+ assert.Equal(t, "MAIN.g_arr[3]", element.GetAddressString())
+ assert.Empty(t, element.GetArrayInfo(), "one element is a scalar, not three elements")
+}
+
+// The same address selects the same elements in plc4go as in plc4j. The two bindings share a
+// specification rather than code, so this is asserted case by case; the numbers here are the
+// ones the Java parity tests assert.
+func TestOneAddressMeansOneThingAcrossDrivers(t *testing.T) {
+ for _, c := range []struct {
+ driver string
+ parse parseTag
+ address string
+ }{
+ {"s7", s7.NewTagHandler().ParseTag, "%M100[0..7]:INT"},
+ {"modbus", modbus.NewTagHandler().ParseTag, "holding-register:1[0..7]:INT"},
+ {"slmp", slmp.NewTagHandler().ParseTag, "D100[0..7]:INT"},
+ {"eip", eip.NewTagHandler().ParseTag, "%rate[0..7]:DINT"},
+ {"simulated", simulated.NewTagHandler().ParseTag, "RANDOM/foo[0..7]:INT"},
+ {"firmata", firmata.NewTagHandler().ParseTag, "digital:0[0..7]"},
+ {"ads", ads.NewTagHandler().ParseTag, "MAIN.g_arr[0..7]"},
+ } {
+ t.Run(c.driver, func(t *testing.T) {
+ tag, err := c.parse(c.address)
+ require.NoError(t, err)
+
+ dimensions := tag.GetArrayInfo()
+ require.Len(t, dimensions, 1, "one dimension")
+ assert.Equal(t, uint32(8), dimensions[0].GetSize(), "eight elements")
+ assert.True(t, dimensions[0].IsRange(), "written as a range")
+
+ reparsed, err := c.parse(tag.GetAddressString())
+ require.NoError(t, err, "a rendered address must parse back")
+ assert.Equal(t, tag, reparsed)
+ })
+ }
+}
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/model/ArrayInfo.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/model/ArrayInfo.java
index 9733aa791e3..324c63b8d5f 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/model/ArrayInfo.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/model/ArrayInfo.java
@@ -26,17 +26,45 @@ public interface ArrayInfo {
int getSize();
/**
- * As in PLCs not every array starts at 0, we need to be flexible with this.
- * In the default usage scenario of a simple array [6] this index will be 0 by default.
+ * The lower index of the selection, as it was written in the address. For a single element
+ * such as [6] this is 6, and {@link #getUpperBound()} is 6 as well - a bare index selects
+ * one element, not a range starting at zero.
* @return Returns the index of lower bound of the array.
*/
int getLowerBound();
/**
- * As in PLCs not every array starts at 0, we need to be flexible with this.
- * In the default usage scenario of a simple array [6] this index will be match the array size.
+ * The upper index of the selection, as it was written in the address. For the range [0..7]
+ * this is 7 and {@link #getSize()} is 8, both bounds being inclusive.
* @return Returns the index of upper bound of the array.
*/
int getUpperBound();
+ /**
+ * The array's declared lower bound, as in PLCs not every array starts at 0. An address may
+ * state it explicitly - [4..7;1] selects elements 4 to 7 of an array declared from 1 - so
+ * that the bounds above can be written the way the PLC program declares them. The offset of
+ * an element from the start of the array is its index minus this value.
+ *
+ * Defaults to 0, which is correct for any array that does not declare otherwise.
+ *
+ * @return Returns the index the array is declared to start at.
+ */
+ default int getBase() {
+ return 0;
+ }
+
+ /**
+ * Whether the address wrote this dimension as a range rather than a single index.
+ *
+ *
The two mean different things to a caller: a single index selects one element and yields
+ * a scalar, while a range yields an array - even a range spanning one element. Equal bounds
+ * alone cannot tell them apart, so the written form has to be remembered.
+ *
+ * @return true when the dimension was written as a range.
+ */
+ default boolean isRange() {
+ return getLowerBound() != getUpperBound();
+ }
+
}
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
index 56540bd7ad1..9b874c8ec62 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/AdsTcpConnection.java
@@ -24,6 +24,7 @@
import org.apache.plc4x.java.ads.discovery.readwrite.Constants;
import org.apache.plc4x.java.ads.model.AdsSubscriptionHandle;
import org.apache.plc4x.java.ads.readwrite.*;
+import org.apache.plc4x.java.ads.readwrite.AdsDataTypeArrayInfo;
import org.apache.plc4x.java.ads.resolution.ResolvedAdsTag;
import org.apache.plc4x.java.ads.resolution.TagResolver;
import org.apache.plc4x.java.ads.resolution.ValueDecoder;
@@ -40,6 +41,7 @@
import org.apache.plc4x.java.api.exceptions.PlcRuntimeException;
import org.apache.plc4x.java.api.messages.*;
import org.apache.plc4x.java.api.model.*;
+import org.apache.plc4x.java.api.model.ArrayInfo;
import org.apache.plc4x.java.api.types.ConnectionStateChangeType;
import org.apache.plc4x.java.api.types.PlcResponseCode;
import org.apache.plc4x.java.api.types.PlcSubscriptionType;
@@ -74,6 +76,7 @@
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.util.*;
+import java.util.ArrayList;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.ReentrantLock;
@@ -510,14 +513,14 @@ private ResolvedAdsTag resolveForReadOrWrite(TagResolver resolver, PlcTag tag) {
return switch (tag) {
case null -> throw new PlcInvalidTagException("Tag could not be parsed");
case SymbolicAdsTag s -> resolver.resolve(s);
- case DirectAdsStringTag s -> new ResolvedAdsTag(s.getIndexGroup(), s.getIndexOffset(),
+ case DirectAdsStringTag s -> TagResolver.withDirectSelection(new ResolvedAdsTag(s.getIndexGroup(), s.getIndexOffset(),
computeDirectSize(s.getPlcDataType(), s.getStringLength(), s.getNumberOfElements()),
s.getPlcDataType(), TagResolver.plcValueTypeForName(s.getPlcDataType(), null),
- s.getStringLength(), Collections.emptyList());
- case DirectAdsTag d -> new ResolvedAdsTag(d.getIndexGroup(), d.getIndexOffset(),
+ s.getStringLength(), Collections.emptyList()), s.getArrayInfo());
+ case DirectAdsTag d -> TagResolver.withDirectSelection(new ResolvedAdsTag(d.getIndexGroup(), d.getIndexOffset(),
computeDirectSize(d.getPlcDataType(), 0, d.getNumberOfElements()),
d.getPlcDataType(), TagResolver.plcValueTypeForName(d.getPlcDataType(), null),
- 0, Collections.emptyList());
+ 0, Collections.emptyList()), d.getArrayInfo());
default -> throw new PlcInvalidTagException("Unsupported tag type: " + tag.getClass().getName());
};
}
@@ -1121,8 +1124,8 @@ protected CompletableFuture onBrowseWithInterceptor(PlcBrowse
List arrayInfo = new ArrayList<>(dataType.getArrayInfo().size());
List itemArrayInfo = new ArrayList<>(dataType.getArrayInfo().size());
for (AdsDataTypeArrayInfo a : dataType.getArrayInfo()) {
- arrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound()));
- itemArrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound()));
+ arrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound(), (int) a.getLowerBound(), true));
+ itemArrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound(), (int) a.getLowerBound(), true));
}
DefaultPlcBrowseItem item = new DefaultPlcBrowseItem(
new SymbolicAdsTag(symbol.getName(), plcValueType, arrayInfo), symbol.getName(),
@@ -1178,8 +1181,8 @@ private List getBrowseItems(String basePath, long baseGroupId, lo
List arrayInfo = new ArrayList<>(childDataType.getArrayInfo().size());
List itemArrayInfo = new ArrayList<>(childDataType.getArrayInfo().size());
for (AdsDataTypeArrayInfo a : childDataType.getArrayInfo()) {
- arrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound()));
- itemArrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound()));
+ arrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound(), (int) a.getLowerBound(), true));
+ itemArrayInfo.add(new DefaultArrayInfo((int) a.getLowerBound(), (int) a.getUpperBound(), (int) a.getLowerBound(), true));
}
values.add(new DefaultPlcBrowseItem(
new SymbolicAdsTag(basePath + "." + child.getMainName(), plc4xPlcValueType, arrayInfo),
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
index 2827d0a4b63..2039e146d0c 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/TagResolver.java
@@ -23,9 +23,12 @@
import org.apache.plc4x.java.ads.readwrite.AdsDataTypeTableEntry;
import org.apache.plc4x.java.ads.readwrite.AdsSymbolTableEntry;
import org.apache.plc4x.java.ads.tag.SymbolicAdsTag;
+import org.apache.plc4x.java.api.model.ArrayInfo;
+import org.apache.plc4x.java.spi.drivers.model.ArrayNotationParser;
import org.apache.plc4x.java.api.exceptions.PlcInvalidTagException;
import org.apache.plc4x.java.api.types.PlcValueType;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -70,7 +73,13 @@ public ResolvedAdsTag resolve(SymbolicAdsTag tag) {
+ " option or address the value directly as"
+ " '{IndexGroup}/{IndexOffset}:{TYPE}'.");
}
- AddressParser.AddressPart root = AddressParser.parse(tag.getSymbolicAddress());
+ // The trailing selection is not part of the symbolic path; it says which elements of the
+ // resolved location to read. Its first index is appended to the path's own indices so the
+ // existing bounds checking and lower-bound arithmetic apply to it unchanged.
+ String path = ArrayNotationParser.addressPart(tag.getSymbolicAddress());
+ List selection = tag.getSelection();
+ AddressParser.AddressPart root = withSelectionStart(AddressParser.parse(path), selection);
+
AdsSymbolTableEntry symbol = symbolTable.get(root.baseSegment());
if (symbol == null) {
throw new PlcInvalidTagException("Unknown symbol: " + root.baseSegment());
@@ -80,27 +89,205 @@ public ResolvedAdsTag resolve(SymbolicAdsTag tag) {
throw new PlcInvalidTagException(
"Unknown data type for symbol " + root.baseSegment() + ": " + symbol.getDataTypeName());
}
- return resolvePart(symbol.getGroup(), symbol.getOffset(), dataType,
- root.arrayIndices(), root.child());
+ verifyDeclaredBase(tag, symbol, dataType);
+
+ ResolvedAdsTag resolved = resolvePart(symbol.getGroup(), symbol.getOffset(), dataType,
+ root.arrayIndices(), root.child(), selection);
+ return scaleToSelection(resolved, selection);
+ }
+
+ /**
+ * Appends the first index of each selected dimension to the deepest segment of the path, so
+ * that the location the read starts at is resolved by the same code that resolves an index
+ * written in the path itself.
+ */
+ private static AddressParser.AddressPart withSelectionStart(AddressParser.AddressPart part,
+ List selection) {
+ if (selection.isEmpty()) {
+ return part;
+ }
+ if (part.child() != null) {
+ return new AddressParser.AddressPart(part.baseSegment(), part.arrayIndices(),
+ withSelectionStart(part.child(), selection));
+ }
+ List indices = new ArrayList<>(part.arrayIndices());
+ for (ArrayInfo dimension : selection) {
+ indices.add(dimension.getLowerBound());
+ }
+ return new AddressParser.AddressPart(part.baseSegment(), indices, null);
+ }
+
+ /**
+ * Checks a declared lower bound written in the address against the one the symbol table
+ * declares. The table is authoritative; a base in the address is the user's statement of
+ * intent, and a disagreement means the address was written against a different layout than
+ * the PLC has - which would otherwise read silently shifted data.
+ *
+ * This is the one rule of the notation that cannot be checked while the address is
+ * parsed, because the symbol table is not loaded then.
+ */
+ private void verifyDeclaredBase(SymbolicAdsTag tag, AdsSymbolTableEntry symbol,
+ AdsDataTypeTableEntry dataType) {
+ Integer declared = tag.getDeclaredBase();
+ if (declared == null || dataType.getArrayInfo().isEmpty()) {
+ return;
+ }
+ long actual = dataType.getArrayInfo().get(dataType.getArrayInfo().size() - 1).getLowerBound();
+ if (declared != actual) {
+ throw new PlcInvalidTagException(String.format(
+ "Address '%s' declares the array to start at %d, but %s declares it to start at %d",
+ tag.getSymbolicAddress(), declared, symbol.getName(), actual));
+ }
+ }
+
+ /**
+ * Restates a direct tag's selection in the terms the decoder reads, so a direct array read
+ * returns every element it asked the device for.
+ *
+ *
The size of the request is already multiplied by the element count, but the decoder
+ * builds its lists from {@code remainingArrayInfo}: left empty, it read one element and
+ * discarded the rest of the response - silently, because a shorter value is still a valid
+ * one. This is what {@link #resolve} does for a symbolic tag; a direct tag names
+ * its own location, so its selection is the whole of its shape.
+ */
+ public static ResolvedAdsTag withDirectSelection(ResolvedAdsTag resolved, List selection) {
+ if (selection.isEmpty()) {
+ return resolved;
+ }
+ List dimensions = new ArrayList<>(selection.size());
+ for (ArrayInfo dimension : selection) {
+ dimensions.add(new AdsDataTypeArrayInfo(
+ (long) dimension.getLowerBound(), (long) dimension.getSize()));
+ }
+ return new ResolvedAdsTag(resolved.indexGroup(), resolved.indexOffset(), resolved.sizeInBytes(),
+ resolved.dataTypeName(), PlcValueType.List, resolved.stringLength(), dimensions);
+ }
+
+ /**
+ * Widens a location resolved for a single element to cover the whole selection: the same
+ * start, as many bytes as the selection spans, decoded as a list.
+ *
+ * The shape follows what the address wrote, dimension by dimension: a range contributes a
+ * level of list, a bare index moves the start and collapses. So {@code grid[3,1..3]} is a
+ * flat list of three and {@code grid[1..2,0..4]} is two lists of five, and a range spanning
+ * one element is still a list of one.
+ */
+ private static ResolvedAdsTag scaleToSelection(ResolvedAdsTag resolved, List selection) {
+ long elements = 1;
+ // The decoder builds its lists from the ADS array-info shape, so the selection is
+ // restated in those terms: the dimensions written as ranges, each starting where the
+ // user asked and spanning as many elements.
+ List dimensions = new ArrayList<>(selection.size());
+ for (ArrayInfo dimension : selection) {
+ elements *= dimension.getSize();
+ if (dimension.isRange()) {
+ dimensions.add(new AdsDataTypeArrayInfo(
+ (long) dimension.getLowerBound(), (long) dimension.getSize()));
+ }
+ }
+ if (dimensions.isEmpty()) {
+ // Every dimension the selection named was a bare index, so it named one element of
+ // them: a scalar, or - where the selection named only the outer dimensions - the
+ // whole of what lies inside one, which resolution has already shaped and sized.
+ return resolved;
+ }
+ // A dimension the selection did not name is selected whole, and is still part of the
+ // shape: grid[1..2] on an ARRAY [0..9,0..4] is two rows of five, not a flat two. Those
+ // dimensions are what resolution left over, and their bytes are already in sizeInBytes.
+ dimensions.addAll(resolved.remainingArrayInfo());
+ return new ResolvedAdsTag(resolved.indexGroup(), resolved.indexOffset(),
+ resolved.sizeInBytes() * elements, resolved.dataTypeName(), PlcValueType.List,
+ resolved.stringLength(), dimensions);
+ }
+
+ /**
+ * Holds a trailing selection to what the device declares and to what one read can express.
+ *
+ * A read covers one contiguous run of memory. Scanning outwards from the innermost
+ * dimension, every dimension inside a dimension selecting more than one element must be
+ * selected whole: on an {@code ARRAY [0..9,0..4]}, {@code [0..9,1..3]} names ten separate
+ * three-element runs, and the contiguous block of thirty starting at {@code [0,1]} that one
+ * read returns is not what was asked for. This refuses it; before, that block was returned.
+ *
+ * A selection may name fewer dimensions than the array declares; the ones it does not name
+ * are selected whole, which is what makes {@code grid[1..2]} two whole rows. Those dimensions
+ * are contiguous by construction, so only the named ones are checked here.
+ */
+ private static void verifySelectionIsOneRead(List selection,
+ List declared,
+ String typeName) {
+ if (selection.size() > declared.size()) {
+ throw new PlcInvalidTagException(String.format(
+ "A selection of %d dimension(s) on %s, which declares %d",
+ selection.size(), typeName, declared.size()));
+ }
+ for (int dimension = 0; dimension < selection.size(); dimension++) {
+ ArrayInfo selected = selection.get(dimension);
+ AdsDataTypeArrayInfo available = declared.get(dimension);
+ if (selected.getLowerBound() < available.getLowerBound()
+ || selected.getUpperBound() > available.getUpperBound()) {
+ throw new PlcInvalidTagException(String.format(
+ "Selection [%d..%d] is outside [%d..%d], which %s declares for dimension %d",
+ selected.getLowerBound(), selected.getUpperBound(),
+ available.getLowerBound(), available.getUpperBound(), typeName, dimension));
+ }
+ if (dimension == 0 || selected.getSize() == available.getNumElements()) {
+ continue;
+ }
+ for (int outer = 0; outer < dimension; outer++) {
+ if (selection.get(outer).getSize() > 1) {
+ throw new PlcInvalidTagException(String.format(
+ "A selection of part of dimension %d of %s, while dimension %d spans %d"
+ + " elements, is not one contiguous read - select the whole of the"
+ + " inner dimension, or one element of the outer one",
+ dimension, typeName, outer, selection.get(outer).getSize()));
+ }
+ }
+ }
}
private ResolvedAdsTag resolvePart(long indexGroup, long indexOffset,
AdsDataTypeTableEntry dataType,
List arrayIndices,
- AddressParser.AddressPart child) {
+ AddressParser.AddressPart child,
+ List selection) {
if (!arrayIndices.isEmpty()) {
- return resolveArray(indexGroup, indexOffset, dataType, arrayIndices, child);
+ return resolveArray(indexGroup, indexOffset, dataType, arrayIndices, child, selection);
}
if (child != null) {
- return resolveChild(indexGroup, indexOffset, dataType, child);
+ if (!dataType.getArrayInfo().isEmpty()) {
+ // Omitting the brackets asks for the whole array, so a member access after one
+ // asks for that member of every element - which is not one contiguous read. The
+ // partially indexed form is refused in resolveArray for the same reason; this is
+ // the same rule where no index was given at all. Without it the address would
+ // silently resolve against the first element.
+ throw new PlcInvalidTagException(
+ "Field access requires an array element to be specified for "
+ + dataType.getMainName() + ": an address that omits the index asks for the"
+ + " whole array, and a member of every element is not a single read");
+ }
+ return resolveChild(indexGroup, indexOffset, dataType, child, selection);
}
+ // remainingArrayInfo stays empty for a whole-array read: it means "dimensions still to
+ // be applied", and the decoder reads the full shape from the type table itself. This is
+ // an internal signal, not the caller-facing report - see SymbolicAdsTag#getArrayInfo.
return finalizeLeaf(indexGroup, indexOffset, dataType, Collections.emptyList());
}
private ResolvedAdsTag resolveArray(long indexGroup, long indexOffset,
AdsDataTypeTableEntry dataType,
List arrayIndices,
- AddressParser.AddressPart child) {
+ AddressParser.AddressPart child,
+ List selection) {
+ // The selection's own start indices were appended to the deepest segment of the path, so
+ // this is the array it selects from, and its declared dimensions are the ones to hold it
+ // to. Deeper segments carry it on; there is nothing to check against here.
+ if (child == null && !selection.isEmpty()) {
+ verifySelectionIsOneRead(selection,
+ dataType.getArrayInfo().subList(
+ Math.max(0, arrayIndices.size() - selection.size()), dataType.getArrayInfo().size()),
+ dataType.getMainName());
+ }
if (dataType.getArrayInfo().isEmpty()) {
throw new PlcInvalidTagException(
"Array index applied to non-array type " + dataType.getMainName());
@@ -144,7 +331,7 @@ private ResolvedAdsTag resolveArray(long indexGroup, long indexOffset,
return primitiveLeaf(indexGroup, indexOffset, elementTypeName, elementSize);
}
return resolvePart(indexGroup, indexOffset, elementDataType,
- Collections.emptyList(), child);
+ Collections.emptyList(), child, child == null ? Collections.emptyList() : selection);
}
// Partial-dim read: carry remaining dims so the decoder can produce nested PlcLists.
@@ -161,7 +348,8 @@ private ResolvedAdsTag resolveArray(long indexGroup, long indexOffset,
private ResolvedAdsTag resolveChild(long indexGroup, long indexOffset,
AdsDataTypeTableEntry dataType,
- AddressParser.AddressPart child) {
+ AddressParser.AddressPart child,
+ List selection) {
AdsDataTypeTableEntry fieldEntry = null;
for (AdsDataTypeTableEntry c : dataType.getChildren()) {
if (c.getMainName().equals(child.baseSegment())) {
@@ -184,7 +372,7 @@ private ResolvedAdsTag resolveChild(long indexGroup, long indexOffset,
fieldEntry.getSecondaryName(), fieldEntry.getSize());
}
return resolvePart(indexGroup, indexOffset + fieldEntry.getOffset(),
- fieldType, child.arrayIndices(), child.child());
+ fieldType, child.arrayIndices(), child.child(), selection);
}
private ResolvedAdsTag finalizeLeaf(long indexGroup, long indexOffset,
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsStringTag.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsStringTag.java
index 557fcd4b30b..3315f7a5730 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsStringTag.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsStringTag.java
@@ -18,6 +18,7 @@
*/
package org.apache.plc4x.java.ads.tag;
+import org.apache.plc4x.java.spi.drivers.model.ArrayNotationParser;
import org.apache.plc4x.java.api.exceptions.PlcInvalidTagException;
import org.apache.plc4x.java.spi.buffers.api.WithOption;
import org.apache.plc4x.java.spi.buffers.api.exceptions.BufferException;
@@ -35,13 +36,19 @@ public class DirectAdsStringTag extends DirectAdsTag implements AdsStringTag {
private static final Pattern RESOURCE_STRING_ADDRESS_PATTERN = Pattern.compile("^((0[xX](?[0-9a-fA-F]{1,8}))|(?\\d{1,10}))" +
"/((0[xX](?[0-9a-fA-F]{1,8}))|(?\\d{1,10}))" +
- ":(?STRING|WSTRING)\\((?\\d{1,3})\\)" +
- "(\\[(?\\d{1,10})])?");
+ ArrayNotationParser.ARRAY_GROUP +
+ ":(?STRING|WSTRING)\\((?\\d{1,3})\\)");
private final int stringLength;
public DirectAdsStringTag(long indexGroup, long indexOffset, String adsDataTypeName, int stringLength, Integer numberOfElements) {
- super(indexGroup, indexOffset, adsDataTypeName, numberOfElements);
+ this(indexGroup, indexOffset, adsDataTypeName, stringLength, numberOfElements,
+ (numberOfElements != null) && (numberOfElements > 1));
+ }
+
+ public DirectAdsStringTag(long indexGroup, long indexOffset, String adsDataTypeName, int stringLength,
+ Integer numberOfElements, boolean explicitRange) {
+ super(indexGroup, indexOffset, adsDataTypeName, numberOfElements, explicitRange);
this.stringLength = stringLength;
}
@@ -69,11 +76,20 @@ public static DirectAdsStringTag of(String address) {
String stringLengthString = matcher.group("stringLength");
int stringLength = stringLengthString != null ? Integer.parseInt(stringLengthString) : 0;
- String numberOfElementsString = matcher.group("numberOfElements");
- Integer numberOfElements = numberOfElementsString != null
- ? parseElementCount(numberOfElementsString) : null;
+ int[] selection = selectionOf(matcher, address);
+ // A string element occupies its declared length plus the terminator, doubled for WSTRING -
+ // the same size AdsTcpConnection computes for the read. The offset counts elements, so it
+ // has to be multiplied by that or a selection lands inside an earlier string.
+ indexOffset += (long) selection[0] * bytesPerString(adsDataTypeName, stringLength);
+ Integer numberOfElements = selection[1];
- return new DirectAdsStringTag(indexGroup, indexOffset, adsDataTypeName, stringLength, numberOfElements);
+ return new DirectAdsStringTag(indexGroup, indexOffset, adsDataTypeName, stringLength,
+ numberOfElements, selection[2] == 1);
+ }
+
+ /** What one string of the declared length occupies, terminator included. */
+ private static int bytesPerString(String adsDataTypeName, int stringLength) {
+ return "WSTRING".equals(adsDataTypeName) ? (stringLength + 1) * 2 : (stringLength + 1);
}
public static boolean matches(String address) {
@@ -82,11 +98,12 @@ public static boolean matches(String address) {
@Override
public String getAddressString() {
- String address = String.format("0x%d/%d:%s(%d)", getIndexGroup(), getIndexOffset(), getPlcDataType(), getStringLength());
- if(getNumberOfElements() != 1) {
- address += "[" + getNumberOfElements() + "]";
- }
- return address;
+ // The selection sits before the type, as the pattern accepts it, and the index group is
+ // rendered in the hex its "0x" claims: "0x%d" printed decimal digits behind a hex prefix,
+ // so group 16416 came back as 0x16416 - a different address that also carried the removed
+ // suffix form, and so parsed as nothing at all.
+ return String.format("0x%X/%d%s:%s(%d)", getIndexGroup(), getIndexOffset(),
+ ArrayNotationParser.render(getArrayInfo()), getPlcDataType(), getStringLength());
}
@Override
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsTag.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsTag.java
index ef2c973c227..2e33ca33b20 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsTag.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/tag/DirectAdsTag.java
@@ -18,12 +18,15 @@
*/
package org.apache.plc4x.java.ads.tag;
+import org.apache.plc4x.java.ads.readwrite.AdsDataType;
import org.apache.plc4x.java.api.exceptions.PlcInvalidTagException;
import org.apache.plc4x.java.api.model.ArrayInfo;
import org.apache.plc4x.java.api.types.PlcValueType;
import org.apache.plc4x.java.spi.buffers.api.WithOption;
import org.apache.plc4x.java.spi.buffers.api.exceptions.BufferException;
import org.apache.plc4x.java.spi.buffers.api.WriteBuffer;
+import org.apache.plc4x.java.spi.drivers.model.AddressConstraints;
+import org.apache.plc4x.java.spi.drivers.model.ArrayNotationParser;
import org.apache.plc4x.java.spi.drivers.model.DefaultArrayInfo;
import java.nio.charset.StandardCharsets;
@@ -42,7 +45,8 @@ public class DirectAdsTag implements AdsTag {
private static final Pattern RESOURCE_ADDRESS_PATTERN = Pattern.compile(
"^((0[xX](?[0-9a-fA-F]{1,8}))|(?\\d{1,10}))" +
"/((0[xX](?[0-9a-fA-F]{1,8}))|(?\\d{1,10}))" +
- ":(?\\w+)(\\[(?