feat!: Make All ProtocolDriver interface API implementations required (…

…#1391)

* feat!: Make All ProtocolDriver interface API implementations required

Move Discover(), ValidateDevice() to the ProtocolDriver interface to that they are required like the existing interfaces. This forces at a minimum an empty implementation which gives exposure to the developer that they exist.

Close #1374

Signed-off-by: bruce <weichou1229@gmail.com>

* feat!: Refactor Bootstrap and NewDeviceService parameter

Use interfaces.ProtocolDriver instead of any as parameter.

Close #1374

Signed-off-by: bruce <weichou1229@gmail.com>

---------

Signed-off-by: bruce <weichou1229@gmail.com>

example/driver/simpledriver.go

@@ -291,7 +291,7 @@ func (s *SimpleDriver) RemoveDevice(deviceName string, protocols map[string]mode
 
 // Discover triggers protocol specific device discovery, which is an asynchronous operation.
 // Devices found as part of this discovery operation are written to the channel devices.
-func (s *SimpleDriver) Discover() {
+func (s *SimpleDriver) Discover() error {
 	proto := make(map[string]models.ProtocolProperties)
 	proto["other"] = map[string]any{"Address": "simple02", "Port": 301}
 
@@ -316,6 +316,7 @@ func (s *SimpleDriver) Discover() {
 
 	time.Sleep(time.Duration(s.serviceConfig.SimpleCustom.Writable.DiscoverSleepDurationSecs) * time.Second)
 	s.deviceCh <- res
+	return nil
 }
 
 func (s *SimpleDriver) ValidateDevice(device models.Device) error {

internal/autodiscovery/autodiscovery.go

@@ -1,14 +1,13 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 //
-// Copyright (C) 2020 IOTech Ltd
+// Copyright (C) 2020-2023 IOTech Ltd
 //
 // SPDX-License-Identifier: Apache-2.0
 
 package autodiscovery
 
 import (
 	"context"
-	"fmt"
 	"sync"
 	"time"
 
@@ -24,7 +23,7 @@ func BootstrapHandler(
 	wg *sync.WaitGroup,
 	_ startup.Timer,
 	dic *di.Container) bool {
-	discovery := container.ProtocolDiscoveryFrom(dic.Get)
+	driver := container.ProtocolDriverFrom(dic.Get)
 	lc := bootstrapContainer.LoggingClientFrom(dic.Get)
 	configuration := container.ConfigurationFrom(dic.Get)
 	var runDiscovery bool = true
@@ -38,24 +37,20 @@ func BootstrapHandler(
 		lc.Info("AutoDiscovery stopped: interval error in configuration")
 		runDiscovery = false
 	}
-	if discovery == nil {
-		lc.Info("AutoDiscovery stopped: ProtocolDiscovery not implemented")
-		runDiscovery = false
-	}
 
 	if runDiscovery {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
 
-			lc.Info(fmt.Sprintf("Starting auto-discovery with duration %v", duration))
-			DiscoveryWrapper(discovery, lc)
+			lc.Infof("Starting auto-discovery with duration %v", duration)
+			DiscoveryWrapper(driver, lc)
 			for {
 				select {
 				case <-ctx.Done():
 					return
 				case <-time.After(duration):
-					DiscoveryWrapper(discovery, lc)
+					DiscoveryWrapper(driver, lc)
 				}
 			}
 		}()

internal/autodiscovery/discovery.go

@@ -1,6 +1,6 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 //
-// Copyright (C) 2020 IOTech Ltd
+// Copyright (C) 2020-2023 IOTech Ltd
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -21,7 +21,7 @@ type discoveryLocker struct {
 
 var locker discoveryLocker
 
-func DiscoveryWrapper(discovery interfaces.ProtocolDiscovery, lc logger.LoggingClient) {
+func DiscoveryWrapper(driver interfaces.ProtocolDriver, lc logger.LoggingClient) {
 	locker.mux.Lock()
 	if locker.busy {
 		lc.Info("another device discovery process is currently running")
@@ -32,7 +32,10 @@ func DiscoveryWrapper(discovery interfaces.ProtocolDiscovery, lc logger.LoggingC
 	locker.mux.Unlock()
 
 	lc.Debug("protocol discovery triggered")
-	discovery.Discover()
+	err := driver.Discover()
+	if err != nil {
+		lc.Error("failed to trigger protocol discovery", err.Error())
+	}
 
 	// ReleaseLock
 	locker.mux.Lock()

internal/container/deviceservice.go

@@ -1,6 +1,6 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 //
-// Copyright (C) 2020-2022 IOTech Ltd
+// Copyright (C) 2020-2023 IOTech Ltd
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -16,41 +16,17 @@ import (
 // DeviceServiceName contains the name of device service struct in the DIC.
 var DeviceServiceName = di.TypeInstanceToName(models.DeviceService{})
 
-// ProtocolDiscoveryName contains the name of protocol discovery implementation in the DIC.
-var ProtocolDiscoveryName = di.TypeInstanceToName((*interfaces.ProtocolDiscovery)(nil))
-
 // ProtocolDriverName contains the name of protocol driver implementation in the DIC.
 var ProtocolDriverName = di.TypeInstanceToName((*interfaces.ProtocolDriver)(nil))
 
 // AutoEventManagerName contains the name of autoevent manager implementation in the DIC
 var AutoEventManagerName = di.TypeInstanceToName((*interfaces.AutoEventManager)(nil))
 
-// DeviceValidatorName contains the name of device validator implementation in the DIC.
-var DeviceValidatorName = di.TypeInstanceToName((*interfaces.DeviceValidator)(nil))
-
 // DeviceServiceFrom helper function queries the DIC and returns device service struct.
 func DeviceServiceFrom(get di.Get) *models.DeviceService {
 	return get(DeviceServiceName).(*models.DeviceService)
 }
 
-// ProtocolDiscoveryFrom helper function queries the DIC and returns protocol discovery implementation.
-func ProtocolDiscoveryFrom(get di.Get) interfaces.ProtocolDiscovery {
-	casted, ok := get(ProtocolDiscoveryName).(interfaces.ProtocolDiscovery)
-	if ok {
-		return casted
-	}
-	return nil
-}
-
-// DeviceValidatorFrom helper function queries the DIC and returns device validator implementation.
-func DeviceValidatorFrom(get di.Get) interfaces.DeviceValidator {
-	casted, ok := get(DeviceValidatorName).(interfaces.DeviceValidator)
-	if ok {
-		return casted
-	}
-	return nil
-}
-
 // ProtocolDriverFrom helper function queries the DIC and returns protocol driver implementation.
 func ProtocolDriverFrom(get di.Get) interfaces.ProtocolDriver {
 	return get(ProtocolDriverName).(interfaces.ProtocolDriver)

internal/controller/http/discovery.go

@@ -1,6 +1,6 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 //
-// Copyright (C) 2020 IOTech Ltd
+// Copyright (C) 2020-2023 IOTech Ltd
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -32,13 +32,8 @@ func (c *RestController) Discovery(writer http.ResponseWriter, request *http.Req
 		return
 	}
 
-	discovery := container.ProtocolDiscoveryFrom(c.dic.Get)
-	if discovery == nil {
-		err := errors.NewCommonEdgeX(errors.KindNotImplemented, "protocolDiscovery not implemented", nil)
-		c.sendEdgexError(writer, request, err, common.ApiDiscoveryRoute)
-		return
-	}
+	driver := container.ProtocolDriverFrom(c.dic.Get)
 
-	go autodiscovery.DiscoveryWrapper(discovery, c.lc)
+	go autodiscovery.DiscoveryWrapper(driver, c.lc)
 	c.sendResponse(writer, request, common.ApiDiscoveryRoute, nil, http.StatusAccepted)
 }

internal/controller/messaging/validation.go

@@ -59,30 +59,29 @@ func SubscribeDeviceValidation(ctx context.Context, dic *di.Container) errors.Ed
 
 				responseTopic := common.BuildTopic(responseTopicPrefix, msgEnvelope.RequestID)
 
-				validator := container.DeviceValidatorFrom(dic.Get)
-				if validator != nil {
-					var deviceRequest requests.AddDeviceRequest
-					err = json.Unmarshal(msgEnvelope.Payload, &deviceRequest)
+				driver := container.ProtocolDriverFrom(dic.Get)
+
+				var deviceRequest requests.AddDeviceRequest
+				err = json.Unmarshal(msgEnvelope.Payload, &deviceRequest)
+				if err != nil {
+					lc.Errorf("Failed to JSON decoding AddDeviceRequest: %s", err.Error())
+					res := types.NewMessageEnvelopeWithError(msgEnvelope.RequestID, err.Error())
+					err = messageBus.Publish(res, responseTopic)
 					if err != nil {
-						lc.Errorf("Failed to JSON decoding AddDeviceRequest: %s", err.Error())
-						res := types.NewMessageEnvelopeWithError(msgEnvelope.RequestID, err.Error())
-						err = messageBus.Publish(res, responseTopic)
-						if err != nil {
-							lc.Errorf("Failed to publish device validation error response: %s", err.Error())
-						}
-						continue
+						lc.Errorf("Failed to publish device validation error response: %s", err.Error())
 					}
+					continue
+				}
 
-					err = validator.ValidateDevice(dtos.ToDeviceModel(deviceRequest.Device))
+				err = driver.ValidateDevice(dtos.ToDeviceModel(deviceRequest.Device))
+				if err != nil {
+					lc.Errorf("Device validation failed: %s", err.Error())
+					res := types.NewMessageEnvelopeWithError(msgEnvelope.RequestID, err.Error())
+					err = messageBus.Publish(res, responseTopic)
 					if err != nil {
-						lc.Errorf("Device validation failed: %s", err.Error())
-						res := types.NewMessageEnvelopeWithError(msgEnvelope.RequestID, err.Error())
-						err = messageBus.Publish(res, responseTopic)
-						if err != nil {
-							lc.Errorf("Failed to publish device validation error response: %s", err.Error())
-						}
-						continue
+						lc.Errorf("Failed to publish device validation error response: %s", err.Error())
 					}
+					continue
 				}
 
 				res, err := types.NewMessageEnvelopeForResponse(nil, msgEnvelope.RequestID, msgEnvelope.CorrelationID, common.ContentTypeJSON)

internal/controller/messaging/validation_test.go

@@ -67,16 +67,16 @@ func TestDeviceValidation(t *testing.T) {
 	mockLogger.On("Debugf", mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	mockLogger.On("Errorf", mock.Anything, mock.Anything).Return(nil)
 
-	mockValidator := &mocks.DeviceValidator{}
-	mockValidator.On("ValidateDevice", dtos.ToDeviceModel(expectedDevice)).Return(nil)
-	mockValidator.On("ValidateDevice", dtos.ToDeviceModel(validationFailedDevice)).Return(errors.New("validation failed"))
+	mockDriver := &mocks.ProtocolDriver{}
+	mockDriver.On("ValidateDevice", dtos.ToDeviceModel(expectedDevice)).Return(nil)
+	mockDriver.On("ValidateDevice", dtos.ToDeviceModel(validationFailedDevice)).Return(errors.New("validation failed"))
 
 	dic := di.NewContainer(di.ServiceConstructorMap{
 		container.ConfigurationName: func(get di.Get) any {
 			return &config.ConfigurationStruct{}
 		},
-		container.DeviceValidatorName: func(get di.Get) any {
-			return mockValidator
+		container.ProtocolDriverName: func(get di.Get) any {
+			return mockDriver
 		},
 		container.DeviceServiceName: func(get di.Get) any {
 			return &models.DeviceService{Name: testServiceName}

openapi/v3/device-sdk.yaml

@@ -571,12 +571,6 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/ErrorResponse'
-        '501':
-          description: The device driver does not implement discovery.
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/ErrorResponse'
         '503':
           description: Discovery is disabled by configuration.
           content:

pkg/interfaces/protocoldriver.go

@@ -54,4 +54,14 @@ type ProtocolDriver interface {
 	// RemoveDevice is a callback function that is invoked
 	// when a Device associated with this Device Service is removed
 	RemoveDevice(deviceName string, protocols map[string]models.ProtocolProperties) error
+
+	// Discover triggers protocol specific device discovery, asynchronously
+	// writes the results to the channel which is passed to the implementation
+	// via ProtocolDriver.Initialize(). The results may be added to the device service
+	// based on a set of acceptance criteria (i.e. Provision Watchers).
+	Discover() error
+
+	// ValidateDevice triggers device's protocol properties validation, returns error
+	// if validation failed and the incoming device will not be added into EdgeX.
+	ValidateDevice(device models.Device) error
 }