feat: Migrate service to V2 #52
Conversation
lenny-goodell
requested review from
ajcasagrande,
sicoyle,
brian-intel,
jim-wang-intel and
Walt-Intel
|
@marcpfuller thanks for the changes, but you changed more than I suggested which has introduced new issues. Please see my inline comment: #52 (comment) |
|
@farshidtz could you take a look at the snap. It looks to be failing. I am not sure what to do fix this |
|
recheck |
I had already explained why it's failing in my comment above. Please merge this to fix it: marcpfuller#1 |
Codecov Report
@@ Coverage Diff @@
## main #52 +/- ##
==========================================
+ Coverage 40.99% 41.14% +0.14%
==========================================
Files 21 21
Lines 9081 9012 -69
==========================================
- Hits 3723 3708 -15
+ Misses 4184 4136 -48
+ Partials 1174 1168 -6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This generally looks good. I was mainly focusing on the snap functionality - we added some extra functionality at Canonial for the earlier pre-release snap for this device, but that's something which we can add in again separately in a new PR, so we don't need to add it now and I think the snap part is overall fine.
However, I was not able to add a device, even though the device discovery found it.
After setting the subnet to 192.168.1.0/24, starting up the device service (as a snap) and doing
curl -X POST http://localhost:59989/api/v2/discovery
to hit the endpoint to trigger discovery, I get the following log messages:
Nov 16 17:28:16 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:16.83168951Z app=device-rfid-llrp source=driver.go:805 msg="Discover was called."
Nov 16 17:28:16 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=ERROR ts=2021-11-16T17:28:16.831782894Z app=device-rfid-llrp source=driver.go:815 error="open res/provision_watchers: no such file or directory" msg="Error adding provision watchers. Newly discovered devices may fail to register with EdgeX."
Nov 16 17:28:17 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:17.845728414Z app=device-rfid-llrp source=discover.go:343 host=192.168.1.124 port=5084 msg="Connection dialed"
Nov 16 17:28:17 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:17.84586393Z app=device-rfid-llrp source=discover.go:391 host=192.168.1.124 port=5084 msg="Attempting to connect to potential LLRP device..."
Nov 16 17:28:17 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:17.854658043Z app=device-rfid-llrp source=logging.go:34 type=MsgReaderEventNotification device=probe-192.168.1.124 msg="Incoming LLRP message"
Nov 16 17:28:17 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=WARN ts=2021-11-16T17:28:17.854748892Z app=device-rfid-llrp source=logging.go:38 message-version=Version1_0_1 client-version=Version1_1 msg="LLRP incoming message version mismatch"
Nov 16 17:28:17 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:17.854860087Z app=device-rfid-llrp source=logging.go:25 type=MsgGetSupportedVersion device=probe-192.168.1.124 msg="Sending LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.512916517Z app=device-rfid-llrp source=logging.go:34 type=MsgErrorMessage device=probe-192.168.1.124 msg="Incoming LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.513064384Z app=device-rfid-llrp source=logging.go:25 type=MsgGetReaderConfig device=probe-192.168.1.124 msg="Sending LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.51686681Z app=device-rfid-llrp source=logging.go:34 type=MsgGetReaderConfigResponse device=probe-192.168.1.124 msg="Incoming LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.517026529Z app=device-rfid-llrp source=logging.go:25 type=MsgGetReaderCapabilities device=probe-192.168.1.124 msg="Sending LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.519738967Z app=device-rfid-llrp source=logging.go:34 type=MsgGetReaderCapabilitiesResponse device=probe-192.168.1.124 msg="Incoming LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.519891589Z app=device-rfid-llrp source=logging.go:25 type=MsgCloseConnection device=probe-192.168.1.124 msg="Sending LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.52119979Z app=device-rfid-llrp source=logging.go:34 type=MsgCloseConnectionResponse device=probe-192.168.1.124 msg="Incoming LLRP message"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.521349274Z app=device-rfid-llrp source=discover.go:397 host=192.168.1.124 port=5084 msg="Connection initiated successfully."
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.521410022Z app=device-rfid-llrp source=discover.go:431 msg="Discovered device: &{deviceName:SpeedwayR-10-EE-EF host:192.168.1.124 port:5084 vendor:25882 model:2001002 fwVersion:4.8.3.240}"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=ERROR ts=2021-11-16T17:28:18.521558433Z app=device-rfid-llrp source=manageddevices.go:69 msg="failed to find Device SpeedwayR-10-EE-EF in cache"
Nov 16 17:28:18 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[405981]: level=INFO ts=2021-11-16T17:28:18.834550047Z app=device-rfid-llrp source=driver.go:855 msg="Discovered 1 new devices in 2.002732959s."
So it finds my Speedway RFID reader at 192.168.1.124, but doesn't then add it as a known device - and I got this error message about the device not being found in the cache.
Using the edgex-cli client, I get:
$ edgex-cli device list
No devices available
$ edgex-cli deviceprofile list
Name Description Manufacturer Model Name
LLRP-Impinj-Profile A device profile with added Impinj support. Impinj LLRP-Impinj-Profile
LLRP-Device-Profile A generic device profile for readers that communicate over LLRP Intel RFID Reader LLRP-Device-Profile
$ edgex-cli deviceservice list
Name BaseAddress Description
device-rfid-llrp http://localhost:59989
Have you tested device discovery using the curl -X POST http://localhost:59989/api/v2/discovery endpoint and successfully added a device?
@siggiskulason , this is awesome feedback. I didn't realize you have a Speedway RFID reader. I was worried about this cache issue once the discovery was triggered. The SDK cache issue was fix for profiles.... Looks like it has a similar issue for devices... :-( |
@siggiskulason, hey we were missing the provision watchers in the snap and I have updated the snap for this. Please test again |
Yes, missing or invalid provision watchers will have the effect @siggiskulason has observed. This is one of the current shortcomings of the provision watcher approach. Because we are unable to get any feedback on if they worked or not, we just have to assume the device was added. |
There was a problem hiding this comment.
However, I was not able to add a device, even though the device discovery found it.
@siggiskulason, hey we were missing the provision watchers in the snap and I have updated the snap for this. Please test again
Thanks @ajcasagrande - I've tested again now. When starting up, I get:
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.45045161Z app=device-rfid-llrp source=httpserver.go:123 msg="Web server starting (localhost:59989)"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.450542373Z app=device-rfid-llrp source=messaging.go:69 msg="Setting options for secure MessageBus with AuthMode='usernamepassword' and SecretName='redisdb"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.453099247Z app=device-rfid-llrp source=messaging.go:99 msg="Connected to redis Message Bus @ redis://localhost:6379 publishing on 'edgex/events/device' prefix topic with AuthMode='usernamepassword'"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.453228273Z app=device-rfid-llrp source=init.go:154 msg="Check core-metadata service's status via Registry..."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.457804581Z app=device-rfid-llrp source=init.go:56 msg="Service clients initialize successful."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.457911334Z app=device-rfid-llrp source=restrouter.go:47 msg="Registering v2 routes..."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.464364883Z app=device-rfid-llrp source=config.go:228 msg="Checking if custom configuration ('AppCustom') exists in Configuration Provider"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.467007593Z app=device-rfid-llrp source=config.go:373 msg="Loaded custom configuration from /var/snap/edgex-device-rfid-llrp/x1/config/device-rfid-llrp/res/configuration.toml"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.482592867Z app=device-rfid-llrp source=config.go:269 msg="Custom Config loaded from file and pushed to Configuration Provider "
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.482669978Z app=device-rfid-llrp source=config.go:282 msg="Loaded custom configuration from file (0 envVars overrides applied)"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.482723204Z app=device-rfid-llrp source=config.go:328 msg="Watching for custom configuration changes has started for `AppCustom`"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.484296622Z app=device-rfid-llrp source=service.go:202 msg="device service device-rfid-llrp doesn't exist, creating a new one"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.484627673Z app=device-rfid-llrp source=config.go:322 msg="Updated custom configuration 'AppCustom' has been received from the Configuration Provider"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.488994692Z app=device-rfid-llrp source=profiles.go:54 msg="Loading pre-defined profiles from /var/snap/edgex-device-rfid-llrp/x1/config/device-rfid-llrp/res/profiles"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.495721633Z app=device-rfid-llrp source=profiles.go:97 msg="Profile LLRP-Device-Profile not found in Metadata, adding it ..."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.49933101Z app=device-rfid-llrp source=profiles.go:97 msg="Profile LLRP-Impinj-Profile not found in Metadata, adding it ..."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.506934821Z app=device-rfid-llrp source=message.go:50 msg="Service dependencies resolved..."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.507003141Z app=device-rfid-llrp source=message.go:51 msg="Starting device-rfid-llrp 1.0.1-dev.3 "
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.507032325Z app=device-rfid-llrp source=message.go:55 msg="device llrp started"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.507033828Z app=device-rfid-llrp source=autodiscovery.go:51 msg="Starting auto-discovery with duration 1h0m0s"
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.507104212Z app=device-rfid-llrp source=driver.go:805 msg="Discover was called."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=ERROR ts=2021-11-18T11:37:36.507166404Z app=device-rfid-llrp source=driver.go:815 error="open res/provision_watchers: no such file or directory" msg="Error adding provision watchers. Newly discovered devices may fail to register with EdgeX."
Nov 18 11:37:36 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:36.507057449Z app=device-rfid-llrp source=message.go:58 msg="Service started in: 209.840269ms"
Nov 18 11:37:38 sidar-XPS-8930 edgexfoundry.consul[61873]: 2021-11-18T11:37:38.183Z [INFO] agent: Synced check: check=device-rfid-llrp
Nov 18 11:37:38 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:37:38.514412714Z app=device-rfid-llrp source=driver.go:855 msg="Discovered 0 new devices in 2.007188336s."
Kicking off manually a device discovery using
curl -X POST http://localhost:59989/api/v2/discovery
I then get
Nov 18 11:38:45 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:38:45.085341414Z app=device-rfid-llrp source=driver.go:805 msg="Discover was called."
Nov 18 11:38:45 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=ERROR ts=2021-11-18T11:38:45.085450501Z app=device-rfid-llrp source=driver.go:815 error="open res/provision_watchers: no such file or directory" msg="Error adding provision watchers. Newly discovered devices may fail to register with EdgeX."
Nov 18 11:38:47 sidar-XPS-8930 edgex-device-rfid-llrp.device-rfid-llrp[63335]: level=INFO ts=2021-11-18T11:38:47.090629233Z app=device-rfid-llrp source=driver.go:855 msg="Discovered 0 new devices in 2.005094894s."
The reason is that the you need to add the following in snap/local/hooks/cmd/install/install.go for the files to be installed :
func installProvisionWatchers() error {
var err error
profs := [...]string{"impinj", "llrp"}
for _, v := range profs {
path := fmt.Sprintf("/config/device-rfid-llrp/res/provision_watchers/%s.provision.watcher.json", v)
destFile := hooks.SnapData + path
srcFile := hooks.Snap + path
if err := os.MkdirAll(filepath.Dir(destFile), 0755); err != nil {
return err
}
if err = hooks.CopyFile(srcFile, destFile); err != nil {
return err
}
}
return nil
}
and in main() add:
err = installProvisionWatchers()
if err != nil {
hooks.Error(fmt.Sprintf("edgex-device-rfid-llrp:install: %v", err))
os.Exit(1)
}
I tried this patch, then start up the snaps with:
sudo snap install edgexfoundry --channel=2.1/edge
sudo snap install ./edgex-device-rfid-llrp_1.0.1-dev.3_amd64.snap --dangerous
sudo snap connect edgexfoundry:edgex-secretstore-token edgex-device-rfid-llrp
sudo vim /var/snap/edgex-device-rfid-llrp/current/config/device-rfid-llrp/res/configuration.toml # to update the subnet
sudo snap start edgex-device-rfid-llrp.device-rfid-llrp
The snap now contains the provisionwatchers, but when running the log still shows
error="open res/provision_watchers: no such file or directory" msg="Error adding provision watchers. Newly discovered devices may fail to register with EdgeX."
The reason for that is that in driver.go the value of provisionWatcherFolder is set to res/provison_watchers. If I manually change this to "/var/snap/edgex-device-rfid-llrp/current/config/device-rfid-llrp/res/provision_watchers" and run again, then it works:
$ edgex-cli device list
Name Description ServiceName ProfileName Labels AutoEvents
SpeedwayR-10-EE-EF LLRP RFID Reader device-rfid-llrp LLRP-Impinj-Profile [RFID LLRP Impinj ] []
Could you update driver.go, to use a path from an environment variable, which can then be set in snapcraft.yaml? So similar to when we set DEVICE_PROFILESDIR: "$SNAP_DATA/config/device-rfid-llrp/res/profiles", there could be a DEFILE_PROVISIONWATCHERSDIR: "$SNAP_DATA/config/device-rfid-llrp/res/provision_watchers" or similar
So please
- add the methods to install.go
- update driver.go to use an environment variable for the provision_watchers path
- add that variable in snapcraft.yaml (
DEVICE_PROVISIONWATCHERSDIR: "$SNAP_DATA/config/device-rfid-llrp/res/provision_watchers"
Once that's done then we're happy with this as far as the snap packaging is concerned.
|
If it helps, you can skip the snap-related fixes and we'll address them once this PR is merged. |
There was a problem hiding this comment.
Looks good! There are some really small things here and there, but those are easily fixed (and mostly nit-picky anyways).
Some notes:
- I have not reviewed the
snapfolder this iteration. - I have not yet tested the latest changes. If anything comes up I will let you know.
@ajcasagrande , please do not spend time on the |
Yeah I was not planning on it based on prior discussions |
Signed-off-by: Marc-philippe Fuller <marc-philippe.fuller@intel.com>
marcpfuller
force-pushed
the
main
branch
2 times, most recently
from
0fbcaaa
to
f5a27c4
Compare
…and resolved null label issue Signed-off-by: Marc-philippe Fuller <marc-philippe.fuller@intel.com>
lenny-goodell
dismissed
siggiskulason’s stale review
Snap PRs following merge
- Add support for defining provision watcher directory - Adding auto-scan script to snap - Adding support for Consul token to auto-scan script - Other minor changes See also: edgexfoundry#52 Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- Add support for defining provision watcher directory - Adding auto-scan script to snap - Adding support for Consul token to auto-scan script - Other minor changes See also: edgexfoundry#52 Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
* fix: updates to enable snap support - Add support for defining provision watcher directory - Adding auto-scan script to snap - Adding support for Consul token to auto-scan script - Other minor changes See also: #52 Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
closes #48
Signed-off-by: Marc-philippe Fuller marc-philippe.fuller@intel.com
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/device-rfid-llrp-go/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)Readme is updated
Testing Instructions
Run service to the point where it is waiting for devices to be added
New Dependency Instructions (If applicable)
N/A