search

edgexfoundry / device-sdk-go

Owner: Device WG
Apache License 2.0
90 stars 125 forks source link

Implement support for specifying a ProvisionWatchersDir, similar to ProfilesDir and DevicesDir #1100

Closed siggiskulason closed 1 year ago

siggiskulason commented 2 years ago

🚀 Feature Request

Description [REQUIRED]

We should be able to specify a `ProvisionWatchersDir`, similar to `ProfilesDir` and `DevicesDir`. ### Describe the solution you'd like 1. add a new file, `internal/provision/provisionwatchers.go` with a `LoadProvisonWatchers` method. See the corresponding profile and device files and also implementation in https://github.com/edgexfoundry/device-rfid-llrp-go/blob/fdb8dc9e065565c3e0b08ed77eb96ef70046d97a/internal/driver/driver.go#L715 2. Add a call to `provision.LoadProvisionWatchers` in`service/init.go` 3. Add `ProvisionWatchersDir` as a configuration file option, in `internal/config/types.go/` and `configuration.toml` ### Describe alternatives you've considered The workaround is for the device service to do this, but that means that the implementation is inconsistent across device services with paths being defined in inconsistent ways. See https://github.com/edgexfoundry/device-rfid-llrp-go/pull/61 with comments discussing the workaround for this issue and the need for an update to the SDK
cloudxxx8 commented 2 years ago

@iain-anderson we discussed about this requirement, but we decided not to implement this feature. please help confirm whether we want to implement this feature in the new release.

iain-anderson commented 2 years ago

I think the question is, are Provision Watchers as currently implemented a good and useful way of determining whether discovered devices should be added and if so what profile they should be used with. If so, then this would seem to be sensible feature - if on the other hand we need to rethink discovery/PWs I'd rather not be enhancing legacy features.

siggiskulason commented 2 years ago

This is mainly an issue because in device-rfid-llrp-go, this feature is being used, but because of a lack of a standard way of defining this, the path was hardcoded to res/provision_watchers (see https://github.com/edgexfoundry/device-rfid-llrp-go/blob/fdb8dc9e065565c3e0b08ed77eb96ef70046d97a/internal/driver/driver.go#L56 ). Now, that doesn´t work with snaps, so I needed to update it to make it work in a PR that has been merged (see https://github.com/edgexfoundry/device-rfid-llrp-go/blob/4e518eebde62524943ef4fba54b9c3a8d84db30b/internal/driver/driver.go#L719 ), where we now read the AppCustom.ProvisionWatcherDir value and use that if it's present. That works, but it means that we are using a non-standard AppCustom.ProvisionWatcherDir setting rather than something which would be standardized across all services.

lenny-goodell commented 2 years ago

@iain-anderson , is there any plan to deprecate the Provision Watchers capability? if not, then I would not consider them legacy features

iain-anderson commented 2 years ago

Thinking about this some more - why are PWs stored in metadata anyway? They don't have a life outside of the particular device service that they apply to. I'm happy to add the ProvisionWatchersDir feature if we say that in v3 we'll remove them from core-metadata and make them local data for the device service.

iain-anderson commented 2 years ago

In answer to the above, as noted in today's WG meeting, storing the PWs in core-metadata allows for dynamic updates

lenny-goodell commented 2 years ago

@iain-anderson, we currently have 3 device service LLRP, USB Camera and ONVIF camera that have the same code to load the PWs. This common code needs to be put in the SDKs.