Discovery fails for unauthorized devices

ajcasagrande commented 2 years ago

Observation When discovering devices, if the device is password-protected, and the credentials do not match the default credentials, the device will not be added to EdgeX. This is because the GetDeviceInformation requires authentication.

Problem Without these devices being added to EdgeX there is no way to assign credentials to them. Thus, currently your solution requires all auto-discovered cameras to share the same username, password, and authentication type (which is bad practice).

Proposed Solution

If a device fails the GetDeviceInformation, but it is still a valid ONVIF device, it should be added to EdgeX, so that the credentials can be added after the fact.
Fix #2 and you will be able to add devices with unique names even without authentication.

ajcasagrande commented 2 years ago

@cloudxxx8 @weichou1229 Any updates on this?

weichou1229 commented 2 years ago

@ajcasagrande Not sure how to handle the device name and other information properly. If we can't query the GetDeviceInformation API, the device name is just a UUID and hard to identify by the label. https://github.com/edgexfoundry-holding/device-onvif-camera/blob/a683f0edb2f5badebbf413b4efd12f153319264c/internal/driver/driver.go#L332-L335

Or we might need to change the mechanism to provide the credential. For example, using secretPath list for matching the credential like below:


[Driver]
DefaultSecretPathes= ["credentials001", "credentials002"]

cloudxxx8 commented 2 years ago

@ajcasagrande let's adopt your proposal. We can change here https://github.com/edgexfoundry-holding/device-onvif-camera/blob/a683f0edb2f5badebbf413b4efd12f153319264c/internal/driver/driver.go#L321-L324

to:

        devInfo, edgexErr := d.getDeviceInformation(dev)
        endpointRef := onvifDevice.GetDeviceParams().EndpointRefAddress
        var discovered sdkModel.DiscoveredDevice
        if edgexErr != nil {
            d.lc.Warnf("failed to get the device information for the camera %s, %v", endpointRef , edgexErr)
            dev.Protocols[OnvifProtocol][SecretPath] = endpointRef 
            discovered = sdkModel.DiscoveredDevice{
                Name:        endpointRef,
                Protocols:   dev.Protocols,
                Description: "Auto discovered Onvif camera",
                Labels:      []string{"auto-discovery"},
            }
        } else {
            dev.Protocols[OnvifProtocol][Manufacturer] = devInfo.Manufacturer
            dev.Protocols[OnvifProtocol][Model] = devInfo.Model
            dev.Protocols[OnvifProtocol][FirmwareVersion] = devInfo.FirmwareVersion
            dev.Protocols[OnvifProtocol][SerialNumber] = devInfo.SerialNumber
            dev.Protocols[OnvifProtocol][HardwareId] = devInfo.HardwareId
            discovered = sdkModel.DiscoveredDevice{
                Name:        fmt.Sprintf("%s-%s-%s", devInfo.Manufacturer, devInfo.Model, endpointRef),
                Protocols:   dev.Protocols,
                Description: fmt.Sprintf("%s %s Camera", devInfo.Manufacturer, devInfo.Model),
                Labels:      []string{"auto-discovery", devInfo.Manufacturer, devInfo.Model},
            }
            d.lc.Debugf("Discovered camera from the address '%s'", onvifDevice.GetDeviceParams().Xaddr)
        }
        discoveredDevices = append(discoveredDevices, discovered)

Is it ok with you?

weichou1229 commented 2 years ago

@ajcasagrande Could you help review the PR? https://github.com/edgexfoundry-holding/device-onvif-camera/pull/10

edgexfoundry / device-onvif-camera

Discovery fails for unauthorized devices #3