diff --git a/internal/driver/discover.go b/internal/driver/discover.go index 24749778..8cf93d4b 100644 --- a/internal/driver/discover.go +++ b/internal/driver/discover.go @@ -24,9 +24,10 @@ import ( ) const ( - DefaultDevicePrefix = "LLRP" - UnknownVendorID = 0 - UnknownModelID = 0 + defaultDevicePrefix = "LLRP" + unknownVendorID = 0 + unknownModelID = 0 + unknownFirmwareVersion = "" ) // discoveryInfo holds information about a discovered device @@ -36,6 +37,7 @@ type discoveryInfo struct { port string vendor uint32 model uint32 + fwVersion string } // newDiscoveryInfo creates a new discoveryInfo with just a host and port pre-filled @@ -236,7 +238,6 @@ func (info *discoveryInfo) updateExistingDevice(device contract.Device) error { "oldInfo", fmt.Sprintf("%+v", tcpInfo), "discoveredInfo", fmt.Sprintf("%+v", info)) - // todo: double check to make sure EdgeX calls driver.UpdateDevice() device.Protocols["tcp"] = map[string]string{ "host": info.host, "port": info.port, @@ -397,11 +398,13 @@ func probe(host, port string, timeout time.Duration) (*discoveryInfo, error) { info := newDiscoveryInfo(host, port) if readerCaps.GeneralDeviceCapabilities == nil { driver.lc.Warn("ReaderCapabilities.GeneralDeviceCapabilities was nil, unable to determine vendor and model info") - info.vendor = UnknownVendorID - info.model = UnknownModelID + info.vendor = unknownVendorID + info.model = unknownModelID + info.fwVersion = unknownFirmwareVersion } else { info.vendor = readerCaps.GeneralDeviceCapabilities.DeviceManufacturer info.model = readerCaps.GeneralDeviceCapabilities.Model + info.fwVersion = readerCaps.GeneralDeviceCapabilities.FirmwareVersion } if readerConfig.Identification == nil { @@ -409,7 +412,7 @@ func probe(host, port string, timeout time.Duration) (*discoveryInfo, error) { return nil, fmt.Errorf("unable to retrieve device identification") } - prefix := DefaultDevicePrefix + prefix := defaultDevicePrefix if VendorIDType(info.vendor) == Impinj { prefix = ImpinjModelType(info.model).HostnamePrefix() } @@ -480,31 +483,44 @@ func newDiscoveredDevice(info *discoveryInfo) dsModels.DiscoveredDevice { "RFID", "LLRP", } + + // Add vendor string only if it is a known vendor + vendorStr := VendorIDType(info.vendor).String() + if !strings.HasPrefix(vendorStr, "VendorIDType(") { + labels = append(labels, vendorStr) + } else { + vendorStr = "" + } + + // Add model string if it is available. Currently only supported for a select few Impinj devices. + var modelStr string if VendorIDType(info.vendor) == Impinj { - labels = append(labels, Impinj.String()) - modelStr := ImpinjModelType(info.model).String() - // only add the label if we know the model + ipjModelStr := ImpinjModelType(info.model).String() if !strings.HasPrefix(modelStr, "ImpinjModelType(") { labels = append(labels, modelStr) + modelStr = ipjModelStr } } // Note that we are adding vendorPEN to the protocol properties in order to // allow the provision watchers to be able to match against that info. Currently // that is the only thing the provision watchers use for matching against. - // - // We would prefer to put the vendorPEN, and possibly model and fw version - // in a separate "protocol", possibly named "metadata", however there is a bug in the - // current logic that does not allow this. - // - // see: https://github.com/edgexfoundry/device-sdk-go/issues/598 return dsModels.DiscoveredDevice{ Name: info.deviceName, Protocols: map[string]contract.ProtocolProperties{ "tcp": { - "host": info.host, - "port": info.port, + "host": info.host, + "port": info.port, + }, + // we are adding a "metadata" protocol as a place to store general properties for + // matching against provision watchers. It is not an actual protocol, but + // the recommended way of adding generic key/value pairs to a device. + "metadata": { "vendorPEN": strconv.FormatUint(uint64(info.vendor), 10), + "vendorStr": vendorStr, + "model": strconv.FormatUint(uint64(info.model), 10), + "modelStr": modelStr, + "fwVersion": info.fwVersion, }, }, Description: "LLRP RFID Reader", diff --git a/internal/driver/discover_test.go b/internal/driver/discover_test.go index 42c23a0a..07362876 100644 --- a/internal/driver/discover_test.go +++ b/internal/driver/discover_test.go @@ -194,13 +194,22 @@ func TestAutoDiscoverNaming(t *testing.T) { if discovered[0].Name != test.name { t.Errorf("expected discovered device's name to be %s, but was: %s", test.name, discovered[0].Name) } - pen, ok := discovered[0].Protocols["tcp"]["vendorPEN"] + + pen, ok := discovered[0].Protocols["metadata"]["vendorPEN"] if !ok { - t.Fatalf("Missing vendorPEN field in tcp protocol. ProtocolMap: %+v", discovered[0].Protocols) + t.Fatalf("Missing vendorPEN field in metadata protocol. ProtocolMap: %+v", discovered[0].Protocols) } if pen != strconv.FormatUint(uint64(test.caps.DeviceManufacturer), 10) { t.Errorf("expected vendorPEN to be %v, but was: %s", test.caps.DeviceManufacturer, pen) } + + fw, ok := discovered[0].Protocols["metadata"]["fwVersion"] + if !ok { + t.Fatalf("Missing fwVersion field in metadata protocol. ProtocolMap: %+v", discovered[0].Protocols) + } + if fw != test.caps.FirmwareVersion { + t.Errorf("expected fwVersion to be %v, but was: %s", test.caps.FirmwareVersion, fw) + } }) } } diff --git a/internal/driver/driver.go b/internal/driver/driver.go index 20252bbd..9a8b0733 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -54,13 +54,10 @@ const ( AttribVendor = "vendor" AttribSubtype = "subtype" - // Note: For now disable the registration of provision watchers since we are not using them - registerProvisionWatchers = false + // enable this by default, otherwise discovery will not work. + registerProvisionWatchers = true provisionWatcherFolder = "res/provision_watchers" - GenericDeviceProfile = "Device.LLRP.Profile" - ImpinjDeviceProfile = "Impinj.LLRP.Profile" - // discoverDebounceDuration is the amount of time to wait for additional changes to discover // configuration before auto-triggering a discovery discoverDebounceDuration = 10 * time.Second @@ -892,50 +889,7 @@ func (d *Driver) discover(ctx context.Context) { d.lc.Warn("Discover process has been cancelled!", "ctxErr", ctx.Err()) } - // Note: We have to send data over this channel to let the SDK know we are done discovering. - // see: https://github.com/edgexfoundry/device-sdk-go/issues/609 - d.deviceCh <- nil d.lc.Info(fmt.Sprintf("Discovered %d new devices in %v.", len(result), time.Now().Sub(t1))) - - // Note: For now we have to resort to adding our discovered devices ourselves due to multiple bugs in the - // provision watcher code, as well as no clear way to tell if a device was matched by a PW or not. - // see: https://github.com/edgexfoundry/device-sdk-go/issues/598 - // see also: https://github.com/edgexfoundry/device-sdk-go/issues/606 - for _, discovered := range result { - if _, err := d.registerDevice(discovered); err != nil { - d.lc.Error("Error adding device.", "name", discovered.Name, "error", err) - } - } -} - -func (d *Driver) registerDevice(discovered dsModels.DiscoveredDevice) (id string, err error) { - profile := GenericDeviceProfile - // Note: This is a bit of a workaround based on provision watcher logic. It was only left - // this way (as opposed to a total refactor) in order to allow a smooth transition - // back to provision watchers once the assortment of bugs have been fixed. - if discovered.Protocols["tcp"]["vendorPEN"] == strconv.FormatUint(uint64(Impinj), 10) { - profile = ImpinjDeviceProfile - } - // remove the field as it is no longer needed/useful - delete(discovered.Protocols["tcp"], "vendorPEN") - - return d.svc.AddDevice(contract.Device{ - DescribedObject: contract.DescribedObject{ - Timestamps: contract.Timestamps{ - Origin: time.Now().UnixNano(), - }, - Description: discovered.Description, - }, - Name: discovered.Name, - Profile: contract.DeviceProfile{ - Name: profile, - }, - Protocols: discovered.Protocols, - Labels: discovered.Labels, - Service: contract.DeviceService{ - Name: ServiceName, - }, - AdminState: contract.Unlocked, - OperatingState: contract.Enabled, - }) + // pass the discovered devices to the EdgeX SDK to be passed through to the provision watchers + d.deviceCh <- result } diff --git a/internal/driver/types.go b/internal/driver/types.go index 43b9942e..aef063d9 100644 --- a/internal/driver/types.go +++ b/internal/driver/types.go @@ -39,6 +39,6 @@ func (imt ImpinjModelType) HostnamePrefix() string { case XArray, XArrayEAP, XArrayWM: return "xArray" default: - return DefaultDevicePrefix + return defaultDevicePrefix } }