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())
}