Skip to content

Commit

Permalink
fix(plc4go): fixed leaking spi interfaces in driverManager.go
Browse files Browse the repository at this point in the history
+ Added todos to driver.go
  • Loading branch information
sruehl committed Nov 4, 2021
1 parent 3154158 commit d1895d6
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 17 deletions.
6 changes: 3 additions & 3 deletions plc4go/internal/plc4go/spi/transports/Transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions plc4go/pkg/plc4go/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
55 changes: 41 additions & 14 deletions plc4go/pkg/plc4go/driverManager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

///////////////////////////////////////
Expand All @@ -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
//
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
}
Expand Down

0 comments on commit d1895d6

Please sign in to comment.