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 d1895d6  fix(plc4go): fixed leaking spi interfaces in driverManager.go
d1895d6 is described below

commit d1895d660e69b28ba8b77378973eb55cd3e365a0
Author: Sebastian Rühl <[email protected]>
AuthorDate: Thu Nov 4 17:00:41 2021 +0100

    fix(plc4go): fixed leaking spi interfaces in driverManager.go
    
    + Added todos to driver.go
---
 plc4go/internal/plc4go/spi/transports/Transport.go |  6 +--
 plc4go/pkg/plc4go/driver.go                        |  2 +
 plc4go/pkg/plc4go/driverManager.go                 | 55 ++++++++++++++++------
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/plc4go/internal/plc4go/spi/transports/Transport.go 
b/plc4go/internal/plc4go/spi/transports/Transport.go
index f4cb5bc..674f363 100644
--- a/plc4go/internal/plc4go/spi/transports/Transport.go
+++ b/plc4go/internal/plc4go/spi/transports/Transport.go
@@ -22,10 +22,10 @@ package transports
 import "net/url"
 
 type Transport interface {
-       // Get the short code used to identify this transport (As used in the 
connection string)
+       // GetTransportCode Get the short code used to identify this transport 
(As used in the connection string)
        GetTransportCode() string
-       // Get a human readable name for this transport
+       // GetTransportName Get a human-readable name for this transport
        GetTransportName() string
-
+       // CreateTransportInstance creates transport instance
        CreateTransportInstance(transportUrl url.URL, options 
map[string][]string) (TransportInstance, error)
 }
diff --git a/plc4go/pkg/plc4go/driver.go b/plc4go/pkg/plc4go/driver.go
index c2d0687..d32918f 100644
--- a/plc4go/pkg/plc4go/driver.go
+++ b/plc4go/pkg/plc4go/driver.go
@@ -40,11 +40,13 @@ type PlcDriver interface {
        CheckQuery(query string) error
 
        // GetConnection Establishes a connection to a given PLC using the 
information in the connectionString
+       // FIXME: this leaks spi in the signature move to spi driver or create 
interfaces. Can also be done by moving spi in a proper module
        GetConnection(transportUrl url.URL, transports 
map[string]transports.Transport, options map[string][]string) <-chan 
PlcConnectionConnectResult
 
        // SupportsDiscovery returns true if this driver supports discovery
        SupportsDiscovery() bool
 
        // Discover TODO: document me
+       // FIXME: this leaks spi in the signature move to spi driver or create 
interfaces. Can also be done by moving spi in a proper module
        Discover(callback func(event model.PlcDiscoveryEvent), discoveryOptions 
...options.WithDiscoveryOption) error
 }
diff --git a/plc4go/pkg/plc4go/driverManager.go 
b/plc4go/pkg/plc4go/driverManager.go
index 917c0d9..f629e4e 100644
--- a/plc4go/pkg/plc4go/driverManager.go
+++ b/plc4go/pkg/plc4go/driverManager.go
@@ -41,7 +41,7 @@ type PlcDriverManager interface {
        GetConnection(connectionString string) <-chan PlcConnectionConnectResult
 
        // Discover Execute all available discovery methods on all available 
drivers using all transports
-       Discover(callback func(event model.PlcDiscoveryEvent), discoveryOptions 
...options.WithDiscoveryOption) error
+       Discover(callback func(event model.PlcDiscoveryEvent), discoveryOptions 
...WithDiscoveryOption) error
 }
 
 func NewPlcDriverManager() PlcDriverManager {
@@ -52,24 +52,35 @@ func NewPlcDriverManager() PlcDriverManager {
        }
 }
 
-func WithDiscoveryOptionProtocol(protocolName string) 
options.WithDiscoveryOption {
-       return options.WithDiscoveryOptionProtocol(protocolName)
+// WithDiscoveryOptionProtocol sets an option for a protocol
+func WithDiscoveryOptionProtocol(protocolName string) WithDiscoveryOption {
+       return 
withDiscoveryOption{options.WithDiscoveryOptionProtocol(protocolName)}
 }
 
-func WithDiscoveryOptionTransport(transportName string) 
options.WithDiscoveryOption {
-       return options.WithDiscoveryOptionTransport(transportName)
+// WithDiscoveryOptionTransport sets an option for a transportName
+func WithDiscoveryOptionTransport(transportName string) WithDiscoveryOption {
+       return 
withDiscoveryOption{options.WithDiscoveryOptionTransport(transportName)}
 }
 
-func WithDiscoveryOptionDeviceName(deviceName string) 
options.WithDiscoveryOption {
-       return options.WithDiscoveryOptionDeviceName(deviceName)
+// WithDiscoveryOptionDeviceName sets an option for a deviceName
+func WithDiscoveryOptionDeviceName(deviceName string) WithDiscoveryOption {
+       return 
withDiscoveryOption{options.WithDiscoveryOptionDeviceName(deviceName)}
 }
 
-func WithDiscoveryOptionLocalAddress(localAddress string) 
options.WithDiscoveryOption {
-       return options.WithDiscoveryOptionLocalAddress(localAddress)
+// WithDiscoveryOptionLocalAddress sets an option for a localAddress
+func WithDiscoveryOptionLocalAddress(localAddress string) WithDiscoveryOption {
+       return 
withDiscoveryOption{options.WithDiscoveryOptionLocalAddress(localAddress)}
 }
 
-func WithDiscoveryOptionRemoteAddress(remoteAddress string) 
options.WithDiscoveryOption {
-       return options.WithDiscoveryOptionRemoteAddress(remoteAddress)
+// WithDiscoveryOptionRemoteAddress sets an option for a remoteAddress
+func WithDiscoveryOptionRemoteAddress(remoteAddress string) 
WithDiscoveryOption {
+       return 
withDiscoveryOption{options.WithDiscoveryOptionRemoteAddress(remoteAddress)}
+}
+
+// WithDiscoveryOption is a marker interface for options regarding discovery
+// FIXME: this is to avoid leaks spi in the signature move to spi driver or 
create interfaces. Can also be done by moving spi in a proper module
+type WithDiscoveryOption interface {
+       isDiscoveryOption() bool
 }
 
 ///////////////////////////////////////
@@ -96,6 +107,22 @@ func (d *plcConnectionConnectResult) GetErr() error {
        return d.err
 }
 
+type withDiscoveryOption struct {
+       options.WithDiscoveryOption
+}
+
+func (w withDiscoveryOption) isDiscoveryOption() bool {
+       return true
+}
+
+func convertToInternalOptions(withDiscoveryOptions ...WithDiscoveryOption) 
[]options.WithDiscoveryOption {
+       result := make([]options.WithDiscoveryOption, len(withDiscoveryOptions))
+       for i, discoveryOption := range withDiscoveryOptions {
+               result[i] = 
discoveryOption.(withDiscoveryOption).WithDiscoveryOption
+       }
+       return result
+}
+
 //
 // Internal section
 //
@@ -251,11 +278,11 @@ func (m *plcDriverManger) GetConnection(connectionString 
string) <-chan PlcConne
        return driver.GetConnection(transportUrl, m.transports, configOptions)
 }
 
-func (m *plcDriverManger) Discover(callback func(event 
model.PlcDiscoveryEvent), discoveryOptions ...options.WithDiscoveryOption) 
error {
+func (m *plcDriverManger) Discover(callback func(event 
model.PlcDiscoveryEvent), discoveryOptions ...WithDiscoveryOption) error {
        // Check if we've got at least one option to restrict to certain 
protocols only.
        // If there is at least one, we only check that protocol, if there are 
none, all
        // available protocols are checked.
-       protocolOptions := 
options.FilterDiscoveryOptionsProtocol(discoveryOptions)
+       protocolOptions := 
options.FilterDiscoveryOptionsProtocol(convertToInternalOptions(discoveryOptions...))
        discoveryDrivers := map[string]PlcDriver{}
        if len(protocolOptions) > 0 {
                for _, protocolOption := range protocolOptions {
@@ -270,7 +297,7 @@ func (m *plcDriverManger) Discover(callback func(event 
model.PlcDiscoveryEvent),
        // Execute discovery on all selected drivers
        for _, driver := range discoveryDrivers {
                if driver.SupportsDiscovery() {
-                       err := driver.Discover(callback, discoveryOptions...)
+                       err := driver.Discover(callback, 
convertToInternalOptions(discoveryOptions...)...)
                        if err != nil {
                                return errors.Wrapf(err, "Error running 
Discover on driver %s", driver.GetProtocolName())
                        }

Reply via email to