You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2021/03/31 21:02:12 UTC

[plc4x] branch develop updated: plc4go: fixes on ads symbolic resolving

This is an automated email from the ASF dual-hosted git repository.

sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git


The following commit(s) were added to refs/heads/develop by this push:
     new 59fba8a  plc4go: fixes on ads symbolic resolving
59fba8a is described below

commit 59fba8ab8a09b84184a31daa3063c8ca2889817f
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Wed Mar 31 23:01:56 2021 +0200

    plc4go: fixes on ads symbolic resolving
    
    + Enabled test "Single element symbolic read request (Address previously resolved)"
    + Writer.go now uses Reader.go for symbolic resolving
    + Fixed DriverTestRunner.go no freeing channels after use
---
 plc4go/cmd/main/drivers/tests/ads_driver_test.go   |  2 --
 plc4go/internal/plc4go/ads/Connection.go           | 16 +++++++----
 plc4go/internal/plc4go/ads/Reader.go               |  1 -
 plc4go/internal/plc4go/ads/Writer.go               | 32 +++++++++++++++++++---
 .../knxnetip/readwrite/model/KnxManufacturer.go    | 25 +++++++++++++----
 .../plc4go/spi/testutils/DriverTestRunner.go       |  9 ++++++
 6 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/plc4go/cmd/main/drivers/tests/ads_driver_test.go b/plc4go/cmd/main/drivers/tests/ads_driver_test.go
index a3b2749..f4ceafa 100644
--- a/plc4go/cmd/main/drivers/tests/ads_driver_test.go
+++ b/plc4go/cmd/main/drivers/tests/ads_driver_test.go
@@ -30,7 +30,5 @@ func TestAdsDriver(t *testing.T) {
 	testutils.RunDriverTestsuite(t, ads.NewDriver(), "assets/testing/protocols/ads/DriverTestsuite.xml",
 		// TODO: tests assumes proper multi requests which is currently not implemented yet
 		"Multi-element direct read request",
-		// TODO: test fails with "testcase read-request result channel already occupied". Probably a bug in the test runner
-		"Single element symbolic read request (Address previously resolved)",
 	)
 }
diff --git a/plc4go/internal/plc4go/ads/Connection.go b/plc4go/internal/plc4go/ads/Connection.go
index 6d3e4d5..f392764 100644
--- a/plc4go/internal/plc4go/ads/Connection.go
+++ b/plc4go/internal/plc4go/ads/Connection.go
@@ -69,6 +69,8 @@ type Connection struct {
 	sourceAmsPort  uint16
 	targetAmsNetId readWriteModel.AmsNetId
 	targetAmsPort  uint16
+	reader         *Reader
+	writer         *Writer
 }
 
 func NewConnection(messageCodec spi.MessageCodec, options map[string][]string, fieldHandler spi.PlcFieldHandler) (*Connection, error) {
@@ -153,16 +155,20 @@ func NewConnection(messageCodec spi.MessageCodec, options map[string][]string, f
 	if err != nil {
 		return nil, errors.Wrap(err, "error prasing targetAmsPort")
 	}
+	reader := *NewReader(messageCodec, targetAmsNetId, uint16(targetAmsPort), sourceAmsNetId, uint16(sourceAmsPort))
+	writer := *NewWriter(messageCodec, targetAmsNetId, uint16(targetAmsPort), sourceAmsNetId, uint16(sourceAmsPort), &reader)
 	return &Connection{
 		messageCodec:       messageCodec,
 		options:            options,
 		fieldHandler:       fieldHandler,
 		valueHandler:       NewValueHandler(),
 		requestInterceptor: interceptors.NewSingleItemRequestInterceptor(),
-		sourceAmsNetId:     sourceAmsNetId,
-		sourceAmsPort:      uint16(sourceAmsPort),
 		targetAmsNetId:     targetAmsNetId,
 		targetAmsPort:      uint16(targetAmsPort),
+		sourceAmsNetId:     sourceAmsNetId,
+		sourceAmsPort:      uint16(sourceAmsPort),
+		reader:             &reader,
+		writer:             &writer,
 	}, nil
 }
 
@@ -220,13 +226,11 @@ func (m Connection) GetMetadata() apiModel.PlcConnectionMetadata {
 }
 
 func (m Connection) ReadRequestBuilder() apiModel.PlcReadRequestBuilder {
-	return internalModel.NewDefaultPlcReadRequestBuilderWithInterceptor(m.fieldHandler,
-		NewReader(m.messageCodec, m.targetAmsNetId, m.targetAmsPort, m.sourceAmsNetId, m.sourceAmsPort), m.requestInterceptor)
+	return internalModel.NewDefaultPlcReadRequestBuilderWithInterceptor(m.fieldHandler, m.reader, m.requestInterceptor)
 }
 
 func (m Connection) WriteRequestBuilder() apiModel.PlcWriteRequestBuilder {
-	return internalModel.NewDefaultPlcWriteRequestBuilder(
-		m.fieldHandler, m.valueHandler, NewWriter(m.messageCodec, m.targetAmsNetId, m.targetAmsPort, m.sourceAmsNetId, m.sourceAmsPort))
+	return internalModel.NewDefaultPlcWriteRequestBuilder(m.fieldHandler, m.valueHandler, m.writer)
 }
 
 func (m Connection) SubscriptionRequestBuilder() apiModel.PlcSubscriptionRequestBuilder {
diff --git a/plc4go/internal/plc4go/ads/Reader.go b/plc4go/internal/plc4go/ads/Reader.go
index 0c8c627..1959dc4 100644
--- a/plc4go/internal/plc4go/ads/Reader.go
+++ b/plc4go/internal/plc4go/ads/Reader.go
@@ -73,7 +73,6 @@ func (m *Reader) Read(readRequest model.PlcReadRequest) <-chan model.PlcReadRequ
 		fieldName := readRequest.GetFieldNames()[0]
 		field := readRequest.GetField(fieldName)
 		if needsResolving(field) {
-			// TODO: resolve field
 			adsField, err := castToSymbolicPlcFieldFromPlcField(field)
 			if err != nil {
 				result <- model.PlcReadRequestResult{
diff --git a/plc4go/internal/plc4go/ads/Writer.go b/plc4go/internal/plc4go/ads/Writer.go
index 088855a..5fdd18b 100644
--- a/plc4go/internal/plc4go/ads/Writer.go
+++ b/plc4go/internal/plc4go/ads/Writer.go
@@ -38,20 +38,22 @@ type Writer struct {
 	sourceAmsNetId        readWriteModel.AmsNetId
 	sourceAmsPort         uint16
 	messageCodec          spi.MessageCodec
+	reader                *Reader
 }
 
-func NewWriter(messageCodec spi.MessageCodec, targetAmsNetId readWriteModel.AmsNetId, targetAmsPort uint16, sourceAmsNetId readWriteModel.AmsNetId, sourceAmsPort uint16) Writer {
-	return Writer{
+func NewWriter(messageCodec spi.MessageCodec, targetAmsNetId readWriteModel.AmsNetId, targetAmsPort uint16, sourceAmsNetId readWriteModel.AmsNetId, sourceAmsPort uint16, reader *Reader) *Writer {
+	return &Writer{
 		transactionIdentifier: 0,
 		targetAmsNetId:        targetAmsNetId,
 		targetAmsPort:         targetAmsPort,
 		sourceAmsNetId:        sourceAmsNetId,
 		sourceAmsPort:         sourceAmsPort,
 		messageCodec:          messageCodec,
+		reader:                reader,
 	}
 }
 
-func (m Writer) Write(writeRequest model.PlcWriteRequest) <-chan model.PlcWriteRequestResult {
+func (m *Writer) Write(writeRequest model.PlcWriteRequest) <-chan model.PlcWriteRequestResult {
 	result := make(chan model.PlcWriteRequestResult)
 	go func() {
 		// If we are requesting only one field, use a
@@ -67,6 +69,28 @@ func (m Writer) Write(writeRequest model.PlcWriteRequest) <-chan model.PlcWriteR
 
 		// Get the ads field instance from the request
 		field := writeRequest.GetField(fieldName)
+		if needsResolving(field) {
+			adsField, err := castToSymbolicPlcFieldFromPlcField(field)
+			if err != nil {
+				result <- model.PlcWriteRequestResult{
+					Request:  writeRequest,
+					Response: nil,
+					Err:      errors.Wrap(err, "invalid field item type"),
+				}
+				log.Debug().Msgf("Invalid field item type %T", field)
+				return
+			}
+			field, err = m.reader.resolveField(adsField)
+			if err != nil {
+				result <- model.PlcWriteRequestResult{
+					Request:  writeRequest,
+					Response: nil,
+					Err:      errors.Wrap(err, "invalid field item type"),
+				}
+				log.Debug().Msgf("Invalid field item type %T", field)
+				return
+			}
+		}
 		adsField, err := castToDirectAdsFieldFromPlcField(field)
 		if err != nil {
 			result <- model.PlcWriteRequestResult{
@@ -170,7 +194,7 @@ func (m Writer) Write(writeRequest model.PlcWriteRequest) <-chan model.PlcWriteR
 	return result
 }
 
-func (m Writer) ToPlc4xWriteResponse(requestTcpPaket readWriteModel.AmsTCPPacket, responseTcpPaket readWriteModel.AmsTCPPacket, writeRequest model.PlcWriteRequest) (model.PlcWriteResponse, error) {
+func (m *Writer) ToPlc4xWriteResponse(requestTcpPaket readWriteModel.AmsTCPPacket, responseTcpPaket readWriteModel.AmsTCPPacket, writeRequest model.PlcWriteRequest) (model.PlcWriteResponse, error) {
 	responseCodes := map[string]model.PlcResponseCode{}
 	fieldName := writeRequest.GetFieldNames()[0]
 
diff --git a/plc4go/internal/plc4go/knxnetip/readwrite/model/KnxManufacturer.go b/plc4go/internal/plc4go/knxnetip/readwrite/model/KnxManufacturer.go
index d931476..ed0d11d 100644
--- a/plc4go/internal/plc4go/knxnetip/readwrite/model/KnxManufacturer.go
+++ b/plc4go/internal/plc4go/knxnetip/readwrite/model/KnxManufacturer.go
@@ -578,8 +578,9 @@ const (
 	KnxManufacturer_M_SOLOMIO_SRL                                        KnxManufacturer = 540
 	KnxManufacturer_M_DOMOTICA_LABS                                      KnxManufacturer = 541
 	KnxManufacturer_M_NVC_INTERNATIONAL                                  KnxManufacturer = 542
-	KnxManufacturer_M_ABB___RESERVED                                     KnxManufacturer = 543
-	KnxManufacturer_M_BUSCH_JAEGER_ELEKTRO___RESERVED                    KnxManufacturer = 544
+	KnxManufacturer_M_BA                                                 KnxManufacturer = 543
+	KnxManufacturer_M_ABB___RESERVED                                     KnxManufacturer = 544
+	KnxManufacturer_M_BUSCH_JAEGER_ELEKTRO___RESERVED                    KnxManufacturer = 545
 )
 
 func (e KnxManufacturer) Number() uint16 {
@@ -2562,10 +2563,14 @@ func (e KnxManufacturer) Number() uint16 {
 		}
 	case 543:
 		{ /* '543' */
-			return 43954
+			return 600
 		}
 	case 544:
 		{ /* '544' */
+			return 43954
+		}
+	case 545:
+		{ /* '545' */
 			return 43959
 		}
 	case 55:
@@ -4751,10 +4756,14 @@ func (e KnxManufacturer) Name() string {
 		}
 	case 543:
 		{ /* '543' */
-			return "ABB - reserved"
+			return "BA"
 		}
 	case 544:
 		{ /* '544' */
+			return "ABB - reserved"
+		}
+	case 545:
+		{ /* '545' */
 			return "Busch-Jaeger Elektro - reserved"
 		}
 	case 55:
@@ -5950,8 +5959,10 @@ func KnxManufacturerByValue(value uint16) KnxManufacturer {
 	case 542:
 		return KnxManufacturer_M_NVC_INTERNATIONAL
 	case 543:
-		return KnxManufacturer_M_ABB___RESERVED
+		return KnxManufacturer_M_BA
 	case 544:
+		return KnxManufacturer_M_ABB___RESERVED
+	case 545:
 		return KnxManufacturer_M_BUSCH_JAEGER_ELEKTRO___RESERVED
 	case 55:
 		return KnxManufacturer_M_WINDOWMASTER_AS
@@ -7045,6 +7056,8 @@ func KnxManufacturerByName(value string) KnxManufacturer {
 		return KnxManufacturer_M_DOMOTICA_LABS
 	case "M_NVC_INTERNATIONAL":
 		return KnxManufacturer_M_NVC_INTERNATIONAL
+	case "M_BA":
+		return KnxManufacturer_M_BA
 	case "M_ABB___RESERVED":
 		return KnxManufacturer_M_ABB___RESERVED
 	case "M_BUSCH_JAEGER_ELEKTRO___RESERVED":
@@ -8191,6 +8204,8 @@ func (e KnxManufacturer) String() string {
 		return "M_DOMOTICA_LABS"
 	case KnxManufacturer_M_NVC_INTERNATIONAL:
 		return "M_NVC_INTERNATIONAL"
+	case KnxManufacturer_M_BA:
+		return "M_BA"
 	case KnxManufacturer_M_ABB___RESERVED:
 		return "M_ABB___RESERVED"
 	case KnxManufacturer_M_BUSCH_JAEGER_ELEKTRO___RESERVED:
diff --git a/plc4go/internal/plc4go/spi/testutils/DriverTestRunner.go b/plc4go/internal/plc4go/spi/testutils/DriverTestRunner.go
index bada58e..a7b037c 100644
--- a/plc4go/internal/plc4go/spi/testutils/DriverTestRunner.go
+++ b/plc4go/internal/plc4go/spi/testutils/DriverTestRunner.go
@@ -23,6 +23,7 @@ import (
 	"encoding/xml"
 	"fmt"
 	adsModel "github.com/apache/plc4x/plc4go/internal/plc4go/ads/readwrite"
+	readWriteModel "github.com/apache/plc4x/plc4go/internal/plc4go/ads/readwrite/model"
 	modbusModel "github.com/apache/plc4x/plc4go/internal/plc4go/modbus/readwrite"
 	"github.com/apache/plc4x/plc4go/internal/plc4go/spi"
 	"github.com/apache/plc4x/plc4go/internal/plc4go/spi/transports"
@@ -192,6 +193,8 @@ func (m DriverTestsuite) ExecuteStep(connection plc4go.PlcConnection, testcase *
 			if err != nil {
 				return errors.Wrap(err, "Error comparing the results")
 			}
+			// Reset read channel
+			testcase.readRequestResultChannel = nil
 		case "PlcWriteResponse":
 			if testcase.writeRequestResultChannel == nil {
 				return errors.New("no write response expected")
@@ -213,6 +216,8 @@ func (m DriverTestsuite) ExecuteStep(connection plc4go.PlcConnection, testcase *
 			if err != nil {
 				return errors.Wrap(err, "Error comparing the results")
 			}
+			// Reset write channel
+			testcase.writeRequestResultChannel = nil
 		}
 	case StepTypeOutgoingPlcMessage:
 		typeName := step.payload.Name
@@ -269,6 +274,10 @@ func (m DriverTestsuite) ExecuteStep(connection plc4go.PlcConnection, testcase *
 		log.Trace().Msg("Comparing outputs")
 		for i := range expectedRawOutput {
 			if expectedRawOutput[i] != rawOutput[i] {
+				dasHierWollenWir := ser
+				dasKommt, err := readWriteModel.AmsTCPPacketParse(utils.NewReadBuffer(rawOutput))
+				println(dasHierWollenWir, dasKommt, err)
+				log.Error().Err(err).Msg("Omg")
 				return errors.Errorf("actual output doesn't match expected output:\nactual:   0x%X\nexpected: 0x%X", rawOutput, expectedRawOutput)
 			}
 		}