You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by GitBox <gi...@apache.org> on 2022/11/07 15:14:23 UTC

[GitHub] [plc4x] chrisdutz opened a new pull request, #576: Feature/cdutz/go ads ng (Streamlining of PLC4X API in PLC4Go and PLC4J)

chrisdutz opened a new pull request, #576:
URL: https://github.com/apache/plc4x/pull/576

   Hi all ... feedback highly appreciated


-- 
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: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [plc4x] sruehl commented on a diff in pull request #576: Feature/cdutz/go ads ng (Streamlining of PLC4X API in PLC4Go and PLC4J)

Posted by GitBox <gi...@apache.org>.
sruehl commented on code in PR #576:
URL: https://github.com/apache/plc4x/pull/576#discussion_r1015563894


##########
plc4go/internal/ads/Browser.go:
##########
@@ -0,0 +1,270 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"strings"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	model2 "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/utils"
+)
+
+func (m *Connection) Browse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest) <-chan apiModel.PlcBrowseRequestResult {
+	return m.BrowseWithInterceptor(ctx, browseRequest, func(result apiModel.PlcBrowseItem) bool {
+		return true
+	})
+}
+
+func (m *Connection) BrowseWithInterceptor(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool) <-chan apiModel.PlcBrowseRequestResult {
+	result := make(chan apiModel.PlcBrowseRequestResult)
+	go func() {
+		responseCodes := map[string]apiModel.PlcResponseCode{}
+		results := map[string][]apiModel.PlcBrowseItem{}
+		for _, queryName := range browseRequest.GetQueryNames() {
+			query := browseRequest.GetQuery(queryName)
+			responseCodes[queryName], results[queryName] = m.BrowseQuery(ctx, browseRequest, interceptor, queryName, query)
+		}
+		browseResponse := model2.NewDefaultPlcBrowseResponse(browseRequest, results, responseCodes)
+		result <- &model2.DefaultPlcBrowseRequestResult{
+			Request:  browseRequest,
+			Response: &browseResponse,
+			Err:      nil,
+		}
+	}()
+	return result
+}
+
+func (m *Connection) BrowseQuery(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	switch query.(type) {
+	case SymbolicPlcQuery:
+		return m.executeSymbolicAddressQuery(ctx, query.(SymbolicPlcQuery))
+	default:
+		return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+	}
+}
+
+func (m *Connection) executeSymbolicAddressQuery(ctx context.Context, query SymbolicPlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	var err error
+
+	// First read the sizes of the data type and symbol table, if needed.
+	var tableSizes model.AdsTableSizes
+	if m.dataTypeTable == nil || m.symbolTable == nil {
+		tableSizes, err = m.readDataTypeTableAndSymbolTableSizes(ctx)
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the data type table, if needed.
+	if m.dataTypeTable == nil {
+		m.dataTypeTable, err = m.readDataTypeTable(ctx, tableSizes.GetDataTypeLength(), tableSizes.GetDataTypeCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the symbol table, if needed.
+	if m.symbolTable == nil {
+		m.symbolTable, err = m.readSymbolTable(ctx, tableSizes.GetSymbolLength(), tableSizes.GetSymbolCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Process the data type and symbol tables to produce the response.
+	fields := m.filterSymbols(query.GetSymbolicAddressPattern())
+	return apiModel.PlcResponseCode_OK, fields
+}
+
+func (m *Connection) filterSymbols(filterExpression string) []apiModel.PlcBrowseItem {
+	if len(filterExpression) == 0 {
+		return nil
+	}
+	addressSegments := strings.Split(filterExpression, ".")
+
+	// The symbol name consists of the first two segments of the address
+	// Some addresses only have one segment, so in that case we'll simply use that.
+	symbolName := addressSegments[0]
+	remainingSegments := addressSegments[1:]
+	if len(addressSegments) > 0 {
+		symbolName = symbolName + "." + remainingSegments[0]
+		remainingSegments = remainingSegments[1:]
+	}
+
+	if symbol, ok := m.symbolTable[symbolName]; !ok {
+		// Couldn't find the base symbol
+		return nil
+	} else if len(remainingSegments) == 0 {
+		// TODO: Convert the symbol itself into a PlcBrowseField
+		return nil
+	} else {

Review Comment:
   else is unnecessary here as the if above has a return



##########
plc4go/internal/ads/Browser.go:
##########
@@ -0,0 +1,270 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"strings"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	model2 "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/utils"
+)
+
+func (m *Connection) Browse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest) <-chan apiModel.PlcBrowseRequestResult {
+	return m.BrowseWithInterceptor(ctx, browseRequest, func(result apiModel.PlcBrowseItem) bool {
+		return true
+	})
+}
+
+func (m *Connection) BrowseWithInterceptor(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool) <-chan apiModel.PlcBrowseRequestResult {
+	result := make(chan apiModel.PlcBrowseRequestResult)
+	go func() {
+		responseCodes := map[string]apiModel.PlcResponseCode{}
+		results := map[string][]apiModel.PlcBrowseItem{}
+		for _, queryName := range browseRequest.GetQueryNames() {
+			query := browseRequest.GetQuery(queryName)
+			responseCodes[queryName], results[queryName] = m.BrowseQuery(ctx, browseRequest, interceptor, queryName, query)
+		}
+		browseResponse := model2.NewDefaultPlcBrowseResponse(browseRequest, results, responseCodes)
+		result <- &model2.DefaultPlcBrowseRequestResult{
+			Request:  browseRequest,
+			Response: &browseResponse,
+			Err:      nil,
+		}
+	}()
+	return result
+}
+
+func (m *Connection) BrowseQuery(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	switch query.(type) {
+	case SymbolicPlcQuery:
+		return m.executeSymbolicAddressQuery(ctx, query.(SymbolicPlcQuery))
+	default:
+		return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+	}
+}
+
+func (m *Connection) executeSymbolicAddressQuery(ctx context.Context, query SymbolicPlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	var err error
+
+	// First read the sizes of the data type and symbol table, if needed.
+	var tableSizes model.AdsTableSizes
+	if m.dataTypeTable == nil || m.symbolTable == nil {
+		tableSizes, err = m.readDataTypeTableAndSymbolTableSizes(ctx)
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the data type table, if needed.
+	if m.dataTypeTable == nil {
+		m.dataTypeTable, err = m.readDataTypeTable(ctx, tableSizes.GetDataTypeLength(), tableSizes.GetDataTypeCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the symbol table, if needed.
+	if m.symbolTable == nil {
+		m.symbolTable, err = m.readSymbolTable(ctx, tableSizes.GetSymbolLength(), tableSizes.GetSymbolCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Process the data type and symbol tables to produce the response.
+	fields := m.filterSymbols(query.GetSymbolicAddressPattern())
+	return apiModel.PlcResponseCode_OK, fields
+}
+
+func (m *Connection) filterSymbols(filterExpression string) []apiModel.PlcBrowseItem {
+	if len(filterExpression) == 0 {
+		return nil
+	}
+	addressSegments := strings.Split(filterExpression, ".")
+
+	// The symbol name consists of the first two segments of the address
+	// Some addresses only have one segment, so in that case we'll simply use that.
+	symbolName := addressSegments[0]
+	remainingSegments := addressSegments[1:]
+	if len(addressSegments) > 0 {
+		symbolName = symbolName + "." + remainingSegments[0]
+		remainingSegments = remainingSegments[1:]
+	}
+
+	if symbol, ok := m.symbolTable[symbolName]; !ok {
+		// Couldn't find the base symbol
+		return nil
+	} else if len(remainingSegments) == 0 {

Review Comment:
   else is unnecessary here as the if above has a return



##########
plc4go/internal/ads/Browser.go:
##########
@@ -0,0 +1,270 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"strings"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	model2 "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/utils"
+)
+
+func (m *Connection) Browse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest) <-chan apiModel.PlcBrowseRequestResult {
+	return m.BrowseWithInterceptor(ctx, browseRequest, func(result apiModel.PlcBrowseItem) bool {
+		return true
+	})
+}
+
+func (m *Connection) BrowseWithInterceptor(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool) <-chan apiModel.PlcBrowseRequestResult {
+	result := make(chan apiModel.PlcBrowseRequestResult)
+	go func() {
+		responseCodes := map[string]apiModel.PlcResponseCode{}
+		results := map[string][]apiModel.PlcBrowseItem{}
+		for _, queryName := range browseRequest.GetQueryNames() {
+			query := browseRequest.GetQuery(queryName)
+			responseCodes[queryName], results[queryName] = m.BrowseQuery(ctx, browseRequest, interceptor, queryName, query)
+		}
+		browseResponse := model2.NewDefaultPlcBrowseResponse(browseRequest, results, responseCodes)
+		result <- &model2.DefaultPlcBrowseRequestResult{
+			Request:  browseRequest,
+			Response: &browseResponse,
+			Err:      nil,
+		}
+	}()
+	return result
+}
+
+func (m *Connection) BrowseQuery(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	switch query.(type) {
+	case SymbolicPlcQuery:
+		return m.executeSymbolicAddressQuery(ctx, query.(SymbolicPlcQuery))
+	default:
+		return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+	}
+}
+
+func (m *Connection) executeSymbolicAddressQuery(ctx context.Context, query SymbolicPlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	var err error
+
+	// First read the sizes of the data type and symbol table, if needed.
+	var tableSizes model.AdsTableSizes
+	if m.dataTypeTable == nil || m.symbolTable == nil {
+		tableSizes, err = m.readDataTypeTableAndSymbolTableSizes(ctx)
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the data type table, if needed.
+	if m.dataTypeTable == nil {
+		m.dataTypeTable, err = m.readDataTypeTable(ctx, tableSizes.GetDataTypeLength(), tableSizes.GetDataTypeCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the symbol table, if needed.
+	if m.symbolTable == nil {
+		m.symbolTable, err = m.readSymbolTable(ctx, tableSizes.GetSymbolLength(), tableSizes.GetSymbolCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Process the data type and symbol tables to produce the response.
+	fields := m.filterSymbols(query.GetSymbolicAddressPattern())
+	return apiModel.PlcResponseCode_OK, fields
+}
+
+func (m *Connection) filterSymbols(filterExpression string) []apiModel.PlcBrowseItem {
+	if len(filterExpression) == 0 {
+		return nil
+	}
+	addressSegments := strings.Split(filterExpression, ".")
+
+	// The symbol name consists of the first two segments of the address
+	// Some addresses only have one segment, so in that case we'll simply use that.
+	symbolName := addressSegments[0]
+	remainingSegments := addressSegments[1:]
+	if len(addressSegments) > 0 {
+		symbolName = symbolName + "." + remainingSegments[0]
+		remainingSegments = remainingSegments[1:]
+	}
+
+	if symbol, ok := m.symbolTable[symbolName]; !ok {
+		// Couldn't find the base symbol
+		return nil
+	} else if len(remainingSegments) == 0 {
+		// TODO: Convert the symbol itself into a PlcBrowseField
+		return nil
+	} else {
+		symbolDataTypeName := symbol.GetDataTypeName()
+		if symbolDataType, ok := m.dataTypeTable[symbolDataTypeName]; !ok {
+			// Couldn't find data type
+			return nil
+		} else {
+			return m.filterDataTypes(symbolName, symbolDataType, symbolDataTypeName, remainingSegments)
+		}
+	}
+}
+
+/*
+func LALALA(){
+	for (AdsSymbolTableEntry symbol : symbolTable.values()) {
+		// Get the datatype of this entry.
+		AdsDataTypeTableEntry dataType = dataTypeTable.get(symbol.getDataTypeName());
+		if (dataType == null) {
+			System.out.printf("couldn't find datatype: %s%n", symbol.getDataTypeName());
+			continue;
+		}
+		String itemName = (symbol.getComment() == null || symbol.getComment().isEmpty()) ? symbol.getName() : symbol.getComment();
+		// Convert the plc value type from the ADS specific one to the PLC4X global one.
+		org.apache.plc4x.java.api.types.PlcValueType plc4xPlcValueType = org.apache.plc4x.java.api.types.PlcValueType.valueOf(getPlcValueTypeForAdsDataType(dataType).toString());
+
+		// If this type has children, add entries for its children.
+		List<PlcBrowseItem> children = getBrowseItems(symbol.getName(), symbol.getGroup(), symbol.getOffset(), !symbol.getFlagReadOnly(), dataType);
+
+		// Populate a map of protocol-dependent options.
+		Map<String, PlcValue> options = new HashMap<>();
+		options.put("comment", new PlcSTRING(symbol.getComment()));
+		options.put("group-id", new PlcUDINT(symbol.getGroup()));
+		options.put("offset", new PlcUDINT(symbol.getOffset()));
+		options.put("size-in-bytes", new PlcUDINT(symbol.getSize()));
+
+		if(plc4xPlcValueType == org.apache.plc4x.java.api.types.PlcValueType.List) {
+			List<PlcBrowseItemArrayInfo> arrayInfo = new ArrayList<>();
+			for (AdsDataTypeArrayInfo adsDataTypeArrayInfo : dataType.getArrayInfo()) {
+				arrayInfo.add(new DefaultBrowseItemArrayInfo(
+					adsDataTypeArrayInfo.getLowerBound(), adsDataTypeArrayInfo.getUpperBound()));
+			}
+			// Add the type itself.
+			values.add(new DefaultListPlcBrowseItem(symbol.getName(), itemName, plc4xPlcValueType, arrayInfo,
+				true, !symbol.getFlagReadOnly(), true, children, options));
+		} else {
+			// Add the type itself.
+			values.add(new DefaultPlcBrowseItem(symbol.getName(), itemName, plc4xPlcValueType, true,
+				!symbol.getFlagReadOnly(), true, children, options));
+		}
+	}
+	DefaultPlcBrowseResponse response = new DefaultPlcBrowseResponse(browseRequest, PlcResponseCode.OK, values);
+
+}
+*/
+
+func (m *Connection) filterDataTypes(parentName string, currentType model.AdsDataTypeTableEntry, currentPath string, remainingAddressSegments []string) []apiModel.PlcBrowseItem {
+	if len(remainingAddressSegments) == 0 {
+		arrayInfo := []apiModel.ArrayInfo{}
+		for _, ai := range currentType.GetArrayInfo() {
+			arrayInfo = append(arrayInfo, model2.DefaultArrayInfo{
+				LowerBound: ai.GetLowerBound(),
+				UpperBound: ai.GetUpperBound(),
+			})
+		}
+		foundField := &model2.DefaultPlcBrowseItem{
+			Field: SymbolicPlcField{
+				PlcField: PlcField{
+					arrayInfo: arrayInfo,
+				},
+				SymbolicAddress: parentName,
+			},
+			Name:         parentName,
+			DataTypeName: currentType.GetDataTypeName(),
+			Readable:     false,
+			Writable:     false,
+			Subscribable: false,
+			Options:      nil,
+		}
+		return []apiModel.PlcBrowseItem{foundField}
+	}
+
+	currentAddressSegment := remainingAddressSegments[0]
+	remainingAddressSegments = remainingAddressSegments[1:]
+	for _, child := range currentType.GetChildren() {
+		if child.GetPropertyName() == currentAddressSegment {
+			childTypeName := child.GetDataTypeName()
+			if symbolDataType, ok := m.dataTypeTable[childTypeName]; !ok {
+				// TODO: Couldn't find data type with the name defined in the protperty.
+				return nil
+			} else {
+				return m.filterDataTypes(parentName+"."+child.GetPropertyName(), symbolDataType,
+					currentPath+"."+currentAddressSegment, remainingAddressSegments)
+			}
+		}
+	}
+	// TODO: Couldn't find property with the given name.

Review Comment:
   TODO: is that meant to be fixed?



##########
plc4go/internal/ads/Interactions.go:
##########
@@ -0,0 +1,261 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+)
+
+func (m *Connection) ExecuteAdsReadDeviceInfoRequest(ctx context.Context) (model.AdsReadDeviceInfoResponse, error) {
+	responseChannel := make(chan model.AdsReadDeviceInfoResponse)
+	go func() {
+		request := m.NewAdsReadDeviceInfoRequest()
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadDeviceInfoResponse)
+				responseChannel <- response
+				close(responseChannel)

Review Comment:
   are you sure that is the right thing to do? closing the channel directly?



##########
plc4go/internal/ads/Interactions.go:
##########
@@ -0,0 +1,261 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+)
+
+func (m *Connection) ExecuteAdsReadDeviceInfoRequest(ctx context.Context) (model.AdsReadDeviceInfoResponse, error) {
+	responseChannel := make(chan model.AdsReadDeviceInfoResponse)
+	go func() {
+		request := m.NewAdsReadDeviceInfoRequest()
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadDeviceInfoResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error reading device info: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsReadRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, length uint32) (model.AdsReadResponse, error) {
+	responseChannel := make(chan model.AdsReadResponse)
+	go func() {
+		request := m.NewAdsReadRequest(indexGroup, indexOffset, length)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second*5); err != nil {
+			close(responseChannel)
+		} else {
+			//			close(responseChannel)

Review Comment:
   outcommended code here



##########
plc4go/internal/ads/Interactions.go:
##########
@@ -0,0 +1,261 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+)
+
+func (m *Connection) ExecuteAdsReadDeviceInfoRequest(ctx context.Context) (model.AdsReadDeviceInfoResponse, error) {
+	responseChannel := make(chan model.AdsReadDeviceInfoResponse)
+	go func() {
+		request := m.NewAdsReadDeviceInfoRequest()
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadDeviceInfoResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error reading device info: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsReadRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, length uint32) (model.AdsReadResponse, error) {
+	responseChannel := make(chan model.AdsReadResponse)
+	go func() {
+		request := m.NewAdsReadRequest(indexGroup, indexOffset, length)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second*5); err != nil {
+			close(responseChannel)
+		} else {
+			//			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error reading: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsWriteRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, data []byte) (model.AdsWriteResponse, error) {
+	responseChannel := make(chan model.AdsWriteResponse)
+	go func() {
+		request := m.NewAdsWriteRequest(indexGroup, indexOffset, data)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsWriteResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error writing: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsReadWriteRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, readLength uint32, items []model.AdsMultiRequestItem, writeData []byte) (model.AdsReadWriteResponse, error) {
+	responseChannel := make(chan model.AdsReadWriteResponse)
+	go func() {
+		request := m.NewAdsReadWriteRequest(indexGroup, indexOffset, readLength, items, writeData)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadWriteResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error writing: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsAddDeviceNotificationRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, length uint32, transmissionMode model.AdsTransMode, maxDelay uint32, cycleTime uint32) (model.AdsAddDeviceNotificationResponse, error) {
+	responseChannel := make(chan model.AdsAddDeviceNotificationResponse)
+	go func() {
+		request := m.NewAdsAddDeviceNotificationRequest(indexGroup, indexOffset, length, transmissionMode, maxDelay, cycleTime)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsAddDeviceNotificationResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error writing: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsDeleteDeviceNotificationRequest(ctx context.Context, notificationHandle uint32) (model.AdsDeleteDeviceNotificationResponse, error) {
+	responseChannel := make(chan model.AdsDeleteDeviceNotificationResponse)
+	go func() {
+		request := m.NewAdsDeleteDeviceNotificationRequest(notificationHandle)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsDeleteDeviceNotificationResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error writing: %v", err)
+	}
+	return response, nil
+}
+
+func ReadWithTimeout[T spi.Message](ch <-chan T) (T, error) {

Review Comment:
   this should get the ctx passed in an create a new ctx from that with timeout. Other than that time.After is leaking timers.



##########
plc4go/spi/default/DefaultBrowser.go:
##########
@@ -21,22 +21,25 @@ package _default
 
 import (
 	"context"
+
 	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
 	"github.com/apache/plc4x/plc4go/spi"
 	"github.com/apache/plc4x/plc4go/spi/model"
 )
 
 // DefaultBrowserRequirements adds required methods to Browser that are needed when using DefaultBrowser
 type DefaultBrowserRequirements interface {
-	BrowseField(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseEvent) bool, fieldName string, field apiModel.PlcField) (apiModel.PlcResponseCode, []apiModel.PlcBrowseFoundField)
+	InternalBrowse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem)

Review Comment:
   what was the reason to prefix that internal?



##########
plc4go/spi/model/render_test.go:
##########
@@ -44,11 +45,11 @@ func TestNonPanickingStrings(t *testing.T) {
 		&DefaultPlcWriteRequest{},
 		&DefaultPlcWriteRequestResult{},
 		&DefaultPlcWriteResponse{},
-		&DefaultRequest{},
+		&DefaultPlcRequest{},
 		&DefaultResponse{},
 		&DefaultPlcBrowseRequestResult{},
 		&DefaultPlcBrowseRequest{},
-		&DefaultPlcBrowseQueryResult{},
+		//&DefaultPlcBrowseQueryResult{},

Review Comment:
   out commented code



##########
plc4go/internal/cbus/Field.go:
##########
@@ -44,9 +47,16 @@ func (s StatusRequestType) String() string {
 	return ""
 }
 
+type CbusField interface {

Review Comment:
   I would just name that Field here...



##########
plc4go/internal/ads/Discoverer.go:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"net"
+	"net/url"
+	"strconv"
+	"time"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/pkg/api/values"
+	"github.com/apache/plc4x/plc4go/protocols/ads/discovery/readwrite/model"
+	driverModel "github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+	internalModel "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/options"
+	values2 "github.com/apache/plc4x/plc4go/spi/values"
+	"github.com/rs/zerolog/log"
+)
+
+type Discoverer struct {
+	messageCodec spi.MessageCodec
+}
+
+func NewDiscoverer() *Discoverer {
+	return &Discoverer{}
+}
+
+func (d *Discoverer) Discover(ctx context.Context, callback func(event apiModel.PlcDiscoveryItem), discoveryOptions ...options.WithDiscoveryOption) error {
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Set up a listening socket on all devices for processing the responses to any search requests
+
+	// Open a listening udp socket for the incoming responses
+	responseAddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf(":%d", model.AdsDiscoveryConstants_ADSDISCOVERYUDPDEFAULTPORT))
+	if err != nil {
+		panic(err)
+	}
+	socket, err := net.ListenUDP("udp4", responseAddr)
+	if err != nil {
+		panic(err)
+	}
+	defer socket.Close()
+
+	// Start a worker to receive responses
+	go func() {
+		buf := make([]byte, 1024)
+		for {
+			length, fromAddr, err := socket.ReadFromUDP(buf)
+			if length == 0 {
+				continue
+			}
+			discoveryResponse, err := model.AdsDiscoveryParse(buf[0:length])
+			if err != nil {
+				log.Error().Err(err).Str("src-ip", fromAddr.String()).Msg("error decoding response")
+				continue
+			}
+
+			if (discoveryResponse.GetRequestId() == 0) &&
+				(discoveryResponse.GetPortNumber() == model.AdsPortNumbers_SYSTEM_SERVICE) &&
+				(discoveryResponse.GetOperation() == model.Operation_DISCOVERY_RESPONSE) {
+				remoteAmsNetId := discoveryResponse.GetAmsNetId()
+				var hostNameBlock model.AdsDiscoveryBlockHostName
+				//var osDataBlock model.AdsDiscoveryBlockOsData
+				var versionBlock model.AdsDiscoveryBlockVersion
+				var fingerprintBlock model.AdsDiscoveryBlockFingerprint
+				for _, block := range discoveryResponse.GetBlocks() {
+					switch block.GetBlockType() {
+					case model.AdsDiscoveryBlockType_HOST_NAME:
+						hostNameBlock = block.(model.AdsDiscoveryBlockHostName)
+						/*									case model.AdsDiscoveryBlockType_OS_DATA:
+															osDataBlock = block.(model.AdsDiscoveryBlockOsData)*/
+					case model.AdsDiscoveryBlockType_VERSION:
+						versionBlock = block.(model.AdsDiscoveryBlockVersion)
+					case model.AdsDiscoveryBlockType_FINGERPRINT:
+						fingerprintBlock = block.(model.AdsDiscoveryBlockFingerprint)
+					}
+				}
+
+				if hostNameBlock != nil {
+					opts := make(map[string][]string)
+					//					opts["sourceAmsNetId"] = []string{localIpV4Address.String() + ".1.1"}
+					opts["sourceAmsPort"] = []string{"65534"}
+					opts["targetAmsNetId"] = []string{strconv.Itoa(int(remoteAmsNetId.GetOctet1())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet2())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet3())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet4())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet5())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet6()))}
+					// TODO: Check if this is legit, or if we can get the information from somewhere.
+					opts["targetAmsPort"] = []string{"851"}
+
+					attributes := make(map[string]values.PlcValue)
+					attributes["hostName"] = values2.NewPlcSTRING(hostNameBlock.GetHostName().GetText())
+					if versionBlock != nil {
+						versionData := versionBlock.GetVersionData()
+						patchVersion := (int(versionData[3])&0xFF)<<8 | (int(versionData[2]) & 0xFF)
+						attributes["twinCatVersion"] = values2.NewPlcSTRING(fmt.Sprintf("%d.%d.%d", int(versionData[0])&0xFF, int(versionData[1])&0xFF, patchVersion))
+					}
+					if fingerprintBlock != nil {
+						attributes["fingerprint"] = values2.NewPlcSTRING(string(fingerprintBlock.GetData()))
+					}
+					// TODO: Find out how to handle the OS Data
+
+					// Add an entry to the results.
+					remoteAddress, err2 := url.Parse("udp://" + strconv.Itoa(int(remoteAmsNetId.GetOctet1())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet2())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet3())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet4())) + ":" +
+						strconv.Itoa(int(driverModel.AdsConstants_ADSTCPDEFAULTPORT)))
+					if err2 == nil {
+						plcDiscoveryItem := &internalModel.DefaultPlcDiscoveryItem{
+							ProtocolCode:  "ads",
+							TransportCode: "tcp",
+							TransportUrl:  *remoteAddress,
+							Options:       opts,
+							Name:          hostNameBlock.GetHostName().GetText(),
+							Attributes:    attributes,
+						}
+
+						// Pass the event back to the callback
+						callback(plcDiscoveryItem)
+					}
+				}
+			}
+		}
+	}()
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Find out which interfaces to use for sending out search requests
+
+	allInterfaces, err := net.Interfaces()
+	if err != nil {
+		return err
+	}
+
+	// If no device is explicitly selected via option, simply use all of them
+	// However if a discovery option is present to select a device by name, only
+	// add those devices matching any of the given names.
+	var interfaces []net.Interface
+	deviceNames := options.FilterDiscoveryOptionsDeviceName(discoveryOptions)
+	if len(deviceNames) > 0 {
+		for _, curInterface := range allInterfaces {
+			for _, deviceNameOption := range deviceNames {
+				if curInterface.Name == deviceNameOption.GetDeviceName() {
+					interfaces = append(interfaces, curInterface)
+					break
+				}
+			}
+		}
+	} else {
+		interfaces = allInterfaces
+	}
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Send out search requests on all selected interfaces
+
+	// Iterate over all network devices of this system.
+	for _, interf := range interfaces {
+		addrs, err := interf.Addrs()
+		if err != nil {
+			return err
+		}
+		// Iterate over all addresses the current interface has configured
+		// For ADS we're only interested in IPv4 addresses, as it doesn't
+		// seem to work with IPv6.
+		for _, addr := range addrs {

Review Comment:
   In C-Bus discovery I moved those inner loops in functions, maybe that would help a bit here



##########
plc4go/spi/values/value_combination_test.go:
##########
@@ -127,7 +127,7 @@ func TestCombinations(t *testing.T) {
 			},
 		},
 		{
-			name: apiValues.LIST,
+			name: apiValues.List,

Review Comment:
   Why is the constant suddenly lower case?



##########
plc4go/internal/ads/Discoverer_test.go:
##########
@@ -0,0 +1,36 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+)
+
+func TestDiscovererManual(t *testing.T) {
+	discoverer := NewDiscoverer()

Review Comment:
   skip() should be added here



##########
plc4go/internal/ads/Browser.go:
##########
@@ -0,0 +1,270 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"strings"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	model2 "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/utils"
+)
+
+func (m *Connection) Browse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest) <-chan apiModel.PlcBrowseRequestResult {
+	return m.BrowseWithInterceptor(ctx, browseRequest, func(result apiModel.PlcBrowseItem) bool {
+		return true
+	})
+}
+
+func (m *Connection) BrowseWithInterceptor(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool) <-chan apiModel.PlcBrowseRequestResult {
+	result := make(chan apiModel.PlcBrowseRequestResult)
+	go func() {
+		responseCodes := map[string]apiModel.PlcResponseCode{}
+		results := map[string][]apiModel.PlcBrowseItem{}
+		for _, queryName := range browseRequest.GetQueryNames() {
+			query := browseRequest.GetQuery(queryName)
+			responseCodes[queryName], results[queryName] = m.BrowseQuery(ctx, browseRequest, interceptor, queryName, query)
+		}
+		browseResponse := model2.NewDefaultPlcBrowseResponse(browseRequest, results, responseCodes)
+		result <- &model2.DefaultPlcBrowseRequestResult{
+			Request:  browseRequest,
+			Response: &browseResponse,
+			Err:      nil,
+		}
+	}()
+	return result
+}
+
+func (m *Connection) BrowseQuery(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	switch query.(type) {
+	case SymbolicPlcQuery:
+		return m.executeSymbolicAddressQuery(ctx, query.(SymbolicPlcQuery))
+	default:
+		return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+	}
+}
+
+func (m *Connection) executeSymbolicAddressQuery(ctx context.Context, query SymbolicPlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	var err error
+
+	// First read the sizes of the data type and symbol table, if needed.
+	var tableSizes model.AdsTableSizes
+	if m.dataTypeTable == nil || m.symbolTable == nil {
+		tableSizes, err = m.readDataTypeTableAndSymbolTableSizes(ctx)
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the data type table, if needed.
+	if m.dataTypeTable == nil {
+		m.dataTypeTable, err = m.readDataTypeTable(ctx, tableSizes.GetDataTypeLength(), tableSizes.GetDataTypeCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the symbol table, if needed.
+	if m.symbolTable == nil {
+		m.symbolTable, err = m.readSymbolTable(ctx, tableSizes.GetSymbolLength(), tableSizes.GetSymbolCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Process the data type and symbol tables to produce the response.
+	fields := m.filterSymbols(query.GetSymbolicAddressPattern())
+	return apiModel.PlcResponseCode_OK, fields
+}
+
+func (m *Connection) filterSymbols(filterExpression string) []apiModel.PlcBrowseItem {
+	if len(filterExpression) == 0 {
+		return nil
+	}
+	addressSegments := strings.Split(filterExpression, ".")
+
+	// The symbol name consists of the first two segments of the address
+	// Some addresses only have one segment, so in that case we'll simply use that.
+	symbolName := addressSegments[0]
+	remainingSegments := addressSegments[1:]
+	if len(addressSegments) > 0 {
+		symbolName = symbolName + "." + remainingSegments[0]
+		remainingSegments = remainingSegments[1:]
+	}
+
+	if symbol, ok := m.symbolTable[symbolName]; !ok {
+		// Couldn't find the base symbol
+		return nil
+	} else if len(remainingSegments) == 0 {
+		// TODO: Convert the symbol itself into a PlcBrowseField
+		return nil
+	} else {
+		symbolDataTypeName := symbol.GetDataTypeName()
+		if symbolDataType, ok := m.dataTypeTable[symbolDataTypeName]; !ok {
+			// Couldn't find data type
+			return nil
+		} else {

Review Comment:
   else is unnecessary here as the if above has a return



##########
plc4go/internal/ads/Browser.go:
##########
@@ -0,0 +1,270 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"strings"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	model2 "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/utils"
+)
+
+func (m *Connection) Browse(ctx context.Context, browseRequest apiModel.PlcBrowseRequest) <-chan apiModel.PlcBrowseRequestResult {
+	return m.BrowseWithInterceptor(ctx, browseRequest, func(result apiModel.PlcBrowseItem) bool {
+		return true
+	})
+}
+
+func (m *Connection) BrowseWithInterceptor(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool) <-chan apiModel.PlcBrowseRequestResult {
+	result := make(chan apiModel.PlcBrowseRequestResult)
+	go func() {
+		responseCodes := map[string]apiModel.PlcResponseCode{}
+		results := map[string][]apiModel.PlcBrowseItem{}
+		for _, queryName := range browseRequest.GetQueryNames() {
+			query := browseRequest.GetQuery(queryName)
+			responseCodes[queryName], results[queryName] = m.BrowseQuery(ctx, browseRequest, interceptor, queryName, query)
+		}
+		browseResponse := model2.NewDefaultPlcBrowseResponse(browseRequest, results, responseCodes)
+		result <- &model2.DefaultPlcBrowseRequestResult{
+			Request:  browseRequest,
+			Response: &browseResponse,
+			Err:      nil,
+		}
+	}()
+	return result
+}
+
+func (m *Connection) BrowseQuery(ctx context.Context, browseRequest apiModel.PlcBrowseRequest, interceptor func(result apiModel.PlcBrowseItem) bool, queryName string, query apiModel.PlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	switch query.(type) {
+	case SymbolicPlcQuery:
+		return m.executeSymbolicAddressQuery(ctx, query.(SymbolicPlcQuery))
+	default:
+		return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+	}
+}
+
+func (m *Connection) executeSymbolicAddressQuery(ctx context.Context, query SymbolicPlcQuery) (apiModel.PlcResponseCode, []apiModel.PlcBrowseItem) {
+	var err error
+
+	// First read the sizes of the data type and symbol table, if needed.
+	var tableSizes model.AdsTableSizes
+	if m.dataTypeTable == nil || m.symbolTable == nil {
+		tableSizes, err = m.readDataTypeTableAndSymbolTableSizes(ctx)
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the data type table, if needed.
+	if m.dataTypeTable == nil {
+		m.dataTypeTable, err = m.readDataTypeTable(ctx, tableSizes.GetDataTypeLength(), tableSizes.GetDataTypeCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Then read the symbol table, if needed.
+	if m.symbolTable == nil {
+		m.symbolTable, err = m.readSymbolTable(ctx, tableSizes.GetSymbolLength(), tableSizes.GetSymbolCount())
+		if err != nil {
+			return apiModel.PlcResponseCode_INTERNAL_ERROR, nil
+		}
+	}
+
+	// Process the data type and symbol tables to produce the response.
+	fields := m.filterSymbols(query.GetSymbolicAddressPattern())
+	return apiModel.PlcResponseCode_OK, fields
+}
+
+func (m *Connection) filterSymbols(filterExpression string) []apiModel.PlcBrowseItem {
+	if len(filterExpression) == 0 {
+		return nil
+	}
+	addressSegments := strings.Split(filterExpression, ".")
+
+	// The symbol name consists of the first two segments of the address
+	// Some addresses only have one segment, so in that case we'll simply use that.
+	symbolName := addressSegments[0]
+	remainingSegments := addressSegments[1:]
+	if len(addressSegments) > 0 {
+		symbolName = symbolName + "." + remainingSegments[0]
+		remainingSegments = remainingSegments[1:]
+	}
+
+	if symbol, ok := m.symbolTable[symbolName]; !ok {
+		// Couldn't find the base symbol
+		return nil
+	} else if len(remainingSegments) == 0 {
+		// TODO: Convert the symbol itself into a PlcBrowseField
+		return nil
+	} else {
+		symbolDataTypeName := symbol.GetDataTypeName()
+		if symbolDataType, ok := m.dataTypeTable[symbolDataTypeName]; !ok {
+			// Couldn't find data type
+			return nil
+		} else {
+			return m.filterDataTypes(symbolName, symbolDataType, symbolDataTypeName, remainingSegments)
+		}
+	}
+}
+
+/*
+func LALALA(){

Review Comment:
   LALALA?



##########
plc4go/internal/ads/Discoverer.go:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"net"
+	"net/url"
+	"strconv"
+	"time"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/pkg/api/values"
+	"github.com/apache/plc4x/plc4go/protocols/ads/discovery/readwrite/model"
+	driverModel "github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+	internalModel "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/options"
+	values2 "github.com/apache/plc4x/plc4go/spi/values"
+	"github.com/rs/zerolog/log"
+)
+
+type Discoverer struct {
+	messageCodec spi.MessageCodec
+}
+
+func NewDiscoverer() *Discoverer {
+	return &Discoverer{}
+}
+
+func (d *Discoverer) Discover(ctx context.Context, callback func(event apiModel.PlcDiscoveryItem), discoveryOptions ...options.WithDiscoveryOption) error {
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Set up a listening socket on all devices for processing the responses to any search requests
+
+	// Open a listening udp socket for the incoming responses
+	responseAddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf(":%d", model.AdsDiscoveryConstants_ADSDISCOVERYUDPDEFAULTPORT))
+	if err != nil {
+		panic(err)
+	}
+	socket, err := net.ListenUDP("udp4", responseAddr)
+	if err != nil {
+		panic(err)
+	}
+	defer socket.Close()
+
+	// Start a worker to receive responses
+	go func() {
+		buf := make([]byte, 1024)
+		for {
+			length, fromAddr, err := socket.ReadFromUDP(buf)
+			if length == 0 {
+				continue
+			}
+			discoveryResponse, err := model.AdsDiscoveryParse(buf[0:length])
+			if err != nil {
+				log.Error().Err(err).Str("src-ip", fromAddr.String()).Msg("error decoding response")
+				continue
+			}
+
+			if (discoveryResponse.GetRequestId() == 0) &&
+				(discoveryResponse.GetPortNumber() == model.AdsPortNumbers_SYSTEM_SERVICE) &&
+				(discoveryResponse.GetOperation() == model.Operation_DISCOVERY_RESPONSE) {
+				remoteAmsNetId := discoveryResponse.GetAmsNetId()
+				var hostNameBlock model.AdsDiscoveryBlockHostName
+				//var osDataBlock model.AdsDiscoveryBlockOsData
+				var versionBlock model.AdsDiscoveryBlockVersion
+				var fingerprintBlock model.AdsDiscoveryBlockFingerprint
+				for _, block := range discoveryResponse.GetBlocks() {
+					switch block.GetBlockType() {
+					case model.AdsDiscoveryBlockType_HOST_NAME:
+						hostNameBlock = block.(model.AdsDiscoveryBlockHostName)
+						/*									case model.AdsDiscoveryBlockType_OS_DATA:
+															osDataBlock = block.(model.AdsDiscoveryBlockOsData)*/
+					case model.AdsDiscoveryBlockType_VERSION:
+						versionBlock = block.(model.AdsDiscoveryBlockVersion)
+					case model.AdsDiscoveryBlockType_FINGERPRINT:
+						fingerprintBlock = block.(model.AdsDiscoveryBlockFingerprint)
+					}
+				}
+
+				if hostNameBlock != nil {

Review Comment:
   early exit would improve readability



##########
plc4go/internal/ads/Interactions.go:
##########
@@ -0,0 +1,261 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+)
+
+func (m *Connection) ExecuteAdsReadDeviceInfoRequest(ctx context.Context) (model.AdsReadDeviceInfoResponse, error) {
+	responseChannel := make(chan model.AdsReadDeviceInfoResponse)
+	go func() {
+		request := m.NewAdsReadDeviceInfoRequest()
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadDeviceInfoResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second); err != nil {
+			close(responseChannel)
+		} else {
+			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)
+	if err != nil {
+		return nil, fmt.Errorf("error reading device info: %v", err)
+	}
+	return response, nil
+}
+
+func (m *Connection) ExecuteAdsReadRequest(ctx context.Context, indexGroup uint32, indexOffset uint32, length uint32) (model.AdsReadResponse, error) {
+	responseChannel := make(chan model.AdsReadResponse)
+	go func() {
+		request := m.NewAdsReadRequest(indexGroup, indexOffset, length)
+		if err := m.messageCodec.SendRequest(
+			ctx,
+			request,
+			func(message spi.Message) bool {
+				amsTcpPacket, ok := message.(model.AmsTCPPacket)
+				if !ok {
+					return false
+				}
+				return amsTcpPacket.GetUserdata().GetInvokeId() == request.GetUserdata().GetInvokeId()
+			},
+			func(message spi.Message) error {
+				amsTcpPacket := message.(model.AmsTCPPacket)
+				response := amsTcpPacket.GetUserdata().(model.AdsReadResponse)
+				responseChannel <- response
+				close(responseChannel)
+				return nil
+			},
+			func(err error) error {
+				return nil
+			},
+			time.Second*5); err != nil {
+			close(responseChannel)
+		} else {
+			//			close(responseChannel)
+		}
+	}()
+	response, err := ReadWithTimeout(responseChannel)

Review Comment:
   shouldn't read with timeout respect the ctx too?



##########
plc4go/internal/ads/Discoverer.go:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"net"
+	"net/url"
+	"strconv"
+	"time"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/pkg/api/values"
+	"github.com/apache/plc4x/plc4go/protocols/ads/discovery/readwrite/model"
+	driverModel "github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+	internalModel "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/options"
+	values2 "github.com/apache/plc4x/plc4go/spi/values"
+	"github.com/rs/zerolog/log"
+)
+
+type Discoverer struct {
+	messageCodec spi.MessageCodec
+}
+
+func NewDiscoverer() *Discoverer {
+	return &Discoverer{}
+}
+
+func (d *Discoverer) Discover(ctx context.Context, callback func(event apiModel.PlcDiscoveryItem), discoveryOptions ...options.WithDiscoveryOption) error {
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Set up a listening socket on all devices for processing the responses to any search requests
+
+	// Open a listening udp socket for the incoming responses
+	responseAddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf(":%d", model.AdsDiscoveryConstants_ADSDISCOVERYUDPDEFAULTPORT))
+	if err != nil {
+		panic(err)
+	}
+	socket, err := net.ListenUDP("udp4", responseAddr)
+	if err != nil {
+		panic(err)
+	}
+	defer socket.Close()
+
+	// Start a worker to receive responses
+	go func() {
+		buf := make([]byte, 1024)
+		for {
+			length, fromAddr, err := socket.ReadFromUDP(buf)
+			if length == 0 {
+				continue
+			}
+			discoveryResponse, err := model.AdsDiscoveryParse(buf[0:length])
+			if err != nil {
+				log.Error().Err(err).Str("src-ip", fromAddr.String()).Msg("error decoding response")
+				continue
+			}
+
+			if (discoveryResponse.GetRequestId() == 0) &&
+				(discoveryResponse.GetPortNumber() == model.AdsPortNumbers_SYSTEM_SERVICE) &&
+				(discoveryResponse.GetOperation() == model.Operation_DISCOVERY_RESPONSE) {
+				remoteAmsNetId := discoveryResponse.GetAmsNetId()
+				var hostNameBlock model.AdsDiscoveryBlockHostName
+				//var osDataBlock model.AdsDiscoveryBlockOsData
+				var versionBlock model.AdsDiscoveryBlockVersion
+				var fingerprintBlock model.AdsDiscoveryBlockFingerprint
+				for _, block := range discoveryResponse.GetBlocks() {
+					switch block.GetBlockType() {
+					case model.AdsDiscoveryBlockType_HOST_NAME:
+						hostNameBlock = block.(model.AdsDiscoveryBlockHostName)
+						/*									case model.AdsDiscoveryBlockType_OS_DATA:
+															osDataBlock = block.(model.AdsDiscoveryBlockOsData)*/
+					case model.AdsDiscoveryBlockType_VERSION:
+						versionBlock = block.(model.AdsDiscoveryBlockVersion)
+					case model.AdsDiscoveryBlockType_FINGERPRINT:
+						fingerprintBlock = block.(model.AdsDiscoveryBlockFingerprint)
+					}
+				}
+
+				if hostNameBlock != nil {
+					opts := make(map[string][]string)
+					//					opts["sourceAmsNetId"] = []string{localIpV4Address.String() + ".1.1"}
+					opts["sourceAmsPort"] = []string{"65534"}
+					opts["targetAmsNetId"] = []string{strconv.Itoa(int(remoteAmsNetId.GetOctet1())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet2())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet3())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet4())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet5())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet6()))}
+					// TODO: Check if this is legit, or if we can get the information from somewhere.
+					opts["targetAmsPort"] = []string{"851"}
+
+					attributes := make(map[string]values.PlcValue)
+					attributes["hostName"] = values2.NewPlcSTRING(hostNameBlock.GetHostName().GetText())
+					if versionBlock != nil {
+						versionData := versionBlock.GetVersionData()
+						patchVersion := (int(versionData[3])&0xFF)<<8 | (int(versionData[2]) & 0xFF)
+						attributes["twinCatVersion"] = values2.NewPlcSTRING(fmt.Sprintf("%d.%d.%d", int(versionData[0])&0xFF, int(versionData[1])&0xFF, patchVersion))
+					}
+					if fingerprintBlock != nil {
+						attributes["fingerprint"] = values2.NewPlcSTRING(string(fingerprintBlock.GetData()))
+					}
+					// TODO: Find out how to handle the OS Data
+
+					// Add an entry to the results.
+					remoteAddress, err2 := url.Parse("udp://" + strconv.Itoa(int(remoteAmsNetId.GetOctet1())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet2())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet3())) + "." +
+						strconv.Itoa(int(remoteAmsNetId.GetOctet4())) + ":" +
+						strconv.Itoa(int(driverModel.AdsConstants_ADSTCPDEFAULTPORT)))
+					if err2 == nil {
+						plcDiscoveryItem := &internalModel.DefaultPlcDiscoveryItem{
+							ProtocolCode:  "ads",
+							TransportCode: "tcp",
+							TransportUrl:  *remoteAddress,
+							Options:       opts,
+							Name:          hostNameBlock.GetHostName().GetText(),
+							Attributes:    attributes,
+						}
+
+						// Pass the event back to the callback
+						callback(plcDiscoveryItem)
+					}
+				}
+			}
+		}
+	}()
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Find out which interfaces to use for sending out search requests
+
+	allInterfaces, err := net.Interfaces()
+	if err != nil {
+		return err
+	}
+
+	// If no device is explicitly selected via option, simply use all of them
+	// However if a discovery option is present to select a device by name, only
+	// add those devices matching any of the given names.
+	var interfaces []net.Interface
+	deviceNames := options.FilterDiscoveryOptionsDeviceName(discoveryOptions)
+	if len(deviceNames) > 0 {
+		for _, curInterface := range allInterfaces {
+			for _, deviceNameOption := range deviceNames {
+				if curInterface.Name == deviceNameOption.GetDeviceName() {
+					interfaces = append(interfaces, curInterface)
+					break
+				}
+			}
+		}
+	} else {
+		interfaces = allInterfaces
+	}
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Send out search requests on all selected interfaces
+
+	// Iterate over all network devices of this system.
+	for _, interf := range interfaces {
+		addrs, err := interf.Addrs()
+		if err != nil {
+			return err
+		}
+		// Iterate over all addresses the current interface has configured
+		// For ADS we're only interested in IPv4 addresses, as it doesn't
+		// seem to work with IPv6.
+		for _, addr := range addrs {
+			var ipv4Addr net.IP
+			switch addr.(type) {
+			// If the device is configured to communicate with a subnet
+			case *net.IPNet:
+				ipv4Addr = addr.(*net.IPNet).IP.To4()
+
+			// If the device is configured for a point-to-point connection
+			case *net.IPAddr:
+				ipv4Addr = addr.(*net.IPAddr).IP.To4()
+			}
+
+			// If we found an IPv4 address and this is not a loopback address,
+			// add it to the list of devices we will open ports and send discovery
+			// messages from.
+			if ipv4Addr != nil && !ipv4Addr.IsLoopback() {

Review Comment:
   early exit would improve readability



##########
plc4go/internal/ads/DiscoveryMessageCodec.go:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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 ads
+
+import (
+	"github.com/apache/plc4x/plc4go/protocols/ads/discovery/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+	"github.com/apache/plc4x/plc4go/spi/default"
+	"github.com/apache/plc4x/plc4go/spi/transports"
+	"github.com/pkg/errors"
+	"github.com/rs/zerolog/log"
+)
+
+type DiscoveryMessageCodec struct {
+	_default.DefaultCodec
+}
+
+func NewDiscoveryMessageCodec(transportInstance transports.TransportInstance) *DiscoveryMessageCodec {
+	codec := &DiscoveryMessageCodec{}
+	codec.DefaultCodec = _default.NewDefaultCodec(codec, transportInstance)
+	return codec
+}
+
+func (m *DiscoveryMessageCodec) GetCodec() spi.MessageCodec {
+	return m
+}
+
+func (m *DiscoveryMessageCodec) Send(message spi.Message) error {
+	log.Trace().Msg("Sending message")
+	// Cast the message to the correct type of struct
+	tcpPaket := message.(model.AdsDiscovery)
+	// Serialize the request
+	bytes, err := tcpPaket.Serialize()
+	if err != nil {
+		return errors.Wrap(err, "error serializing request")
+	}
+
+	// Send it to the PLC
+	err = m.GetTransportInstance().Write(bytes)
+	if err != nil {
+		return errors.Wrap(err, "error sending request")
+	}
+	return nil
+}
+
+func (m *DiscoveryMessageCodec) Receive() (spi.Message, error) {
+	// We need at least 6 bytes in order to know how big the packet is in total
+	if num, err := m.GetTransportInstance().GetNumBytesAvailableInBuffer(); (err == nil) && (num >= 6) {
+		log.Debug().Msgf("we got %d readable bytes", num)
+		data, err := m.GetTransportInstance().PeekReadableBytes(6)
+		if err != nil {
+			log.Warn().Err(err).Msg("error peeking")
+			// TODO: Possibly clean up ...
+			return nil, nil
+		}
+		// Get the size of the entire packet little endian plus size of header
+		packetSize := (uint32(data[5]) << 24) + (uint32(data[4]) << 16) + (uint32(data[3]) << 8) + (uint32(data[2])) + 6
+		if num < packetSize {
+			log.Debug().Msgf("Not enough bytes. Got: %d Need: %d\n", num, packetSize)
+			return nil, nil
+		}
+		data, err = m.GetTransportInstance().Read(packetSize)
+		if err != nil {
+			// TODO: Possibly clean up ...
+			return nil, nil
+		}
+		tcpPacket, err := model.AdsDiscoveryParse(data)
+		if err != nil {
+			log.Warn().Err(err).Msg("error parsing")
+			// TODO: Possibly clean up ...
+			return nil, nil
+		}
+		return tcpPacket, nil
+	} else if err != nil {
+		log.Warn().Err(err).Msg("Got error reading")
+		return nil, nil
+	}
+	// TODO: maybe we return here a not enough error error

Review Comment:
   can this TODO be fixed?



##########
plc4go/internal/ads/Connection.go:
##########
@@ -38,9 +42,53 @@ type Connection struct {
 	reader             *Reader
 	writer             *Writer
 	connectionId       string
+	invokeId           uint32
+	dataTypeTable      map[string]model.AdsDataTypeTableEntry
+	symbolTable        map[string]model.AdsSymbolTableEntry
 	tracer             *spi.Tracer
 }
 
+/*

Review Comment:
   code outcommended here



##########
plc4go/internal/ads/Writer.go:
##########
@@ -56,133 +55,135 @@ func NewWriter(messageCodec spi.MessageCodec, targetAmsNetId readWriteModel.AmsN
 }
 
 func (m *Writer) Write(ctx context.Context, writeRequest model.PlcWriteRequest) <-chan model.PlcWriteRequestResult {
-	// TODO: handle context
-	result := make(chan model.PlcWriteRequestResult)
-	go func() {
-		// If we are requesting only one field, use a
-		if len(writeRequest.GetFieldNames()) != 1 {
-			result <- &plc4goModel.DefaultPlcWriteRequestResult{
-				Request:  writeRequest,
-				Response: nil,
-				Err:      errors.New("ads only supports single-item requests"),
-			}
-			return
-		}
-		fieldName := writeRequest.GetFieldNames()[0]
-
-		// Get the ads field instance from the request
-		field := writeRequest.GetField(fieldName)
-		if needsResolving(field) {
-			adsField, err := castToSymbolicPlcFieldFromPlcField(field)
-			if err != nil {
+	/*	// TODO: handle context

Review Comment:
   outcommented code here



##########
plc4go/internal/ads/Discoverer.go:
##########
@@ -0,0 +1,250 @@
+/*
+ * 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 ads
+
+import (
+	"context"
+	"encoding/binary"
+	"fmt"
+	"net"
+	"net/url"
+	"strconv"
+	"time"
+
+	apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+	"github.com/apache/plc4x/plc4go/pkg/api/values"
+	"github.com/apache/plc4x/plc4go/protocols/ads/discovery/readwrite/model"
+	driverModel "github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+	"github.com/apache/plc4x/plc4go/spi"
+	internalModel "github.com/apache/plc4x/plc4go/spi/model"
+	"github.com/apache/plc4x/plc4go/spi/options"
+	values2 "github.com/apache/plc4x/plc4go/spi/values"
+	"github.com/rs/zerolog/log"
+)
+
+type Discoverer struct {
+	messageCodec spi.MessageCodec
+}
+
+func NewDiscoverer() *Discoverer {
+	return &Discoverer{}
+}
+
+func (d *Discoverer) Discover(ctx context.Context, callback func(event apiModel.PlcDiscoveryItem), discoveryOptions ...options.WithDiscoveryOption) error {
+
+	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+	// Set up a listening socket on all devices for processing the responses to any search requests
+
+	// Open a listening udp socket for the incoming responses
+	responseAddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf(":%d", model.AdsDiscoveryConstants_ADSDISCOVERYUDPDEFAULTPORT))
+	if err != nil {
+		panic(err)
+	}
+	socket, err := net.ListenUDP("udp4", responseAddr)
+	if err != nil {
+		panic(err)
+	}
+	defer socket.Close()
+
+	// Start a worker to receive responses
+	go func() {
+		buf := make([]byte, 1024)
+		for {
+			length, fromAddr, err := socket.ReadFromUDP(buf)
+			if length == 0 {
+				continue
+			}
+			discoveryResponse, err := model.AdsDiscoveryParse(buf[0:length])
+			if err != nil {
+				log.Error().Err(err).Str("src-ip", fromAddr.String()).Msg("error decoding response")
+				continue
+			}
+
+			if (discoveryResponse.GetRequestId() == 0) &&

Review Comment:
   early exit would improve readability



##########
plc4go/internal/ads/MessageTemplates.go:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 ads
+
+import adsModel "github.com/apache/plc4x/plc4go/protocols/ads/readwrite/model"
+
+func (m *Connection) NewAdsReadDeviceInfoRequest() adsModel.AmsTCPPacket {
+	return adsModel.NewAmsTCPPacket(
+		adsModel.NewAdsReadDeviceInfoRequest(m.configuration.targetAmsNetId, uint16(adsModel.DefaultAmsPorts_RUNTIME_SYSTEM_01),
+			m.configuration.sourceAmsNetId, 800, 0, m.getInvokeId()))

Review Comment:
   what is the magic number 800 here?



-- 
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: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [plc4x] chrisdutz merged pull request #576: Feature/cdutz/go ads ng (Streamlining of PLC4X API in PLC4Go and PLC4J)

Posted by GitBox <gi...@apache.org>.
chrisdutz merged PR #576:
URL: https://github.com/apache/plc4x/pull/576


-- 
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: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org