lorawan-stack icon indicating copy to clipboard operation
lorawan-stack copied to clipboard

Load device profile on import

Open nicholaspcr opened this issue 3 years ago • 3 comments

Summary

References #5512 Blocked by #5659

The main idea of this is to apply the end profile loading on the back-end, that way there can be some sort of control on the front-end (controlled through the injection of the application_id to the request). Therefore this doesn't close the issue but adds the functionality. There still needs to be the UI change on the front-end but the CLI should work.

Changes

  • Add vendor_id and and vendor_profile_id fields to csv import
  • Add vendor_id and vendor_profile_id to allowed field mask list on end_device creation.
  • Add EndDeviceProfileIdentifiers to convert service. Allows for setting a fallback value for fetching templates for all devices added.
  • Add more test cases for ttscsv
  • Add profile fetcher to DTC and unit tests for it.

Testing

Unit tests and manual testing. Manual testing:

  • Temporarily add the application_id field to the converter and use the console.
  • Run the template through the CLI.
Regressions

The old way of handling the request should still work, this is added in a way that the loading only happen if the user specify the vendor_id and vendor_profile_id on the CSV file or if the console provide the fallback values in the request.

Notes for Reviewers

Will add the documentation after this is approved.

Checklist

  • [x] Scope: The referenced issue is addressed, there are no unrelated changes.
  • [x] Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • [ ] Documentation: Relevant documentation is added or updated.
  • [x] Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • [x] Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

nicholaspcr avatar Jul 18 '22 18:07 nicholaspcr

Since you're chaining PRs, you can make this one target feature/remove-app-id-from-dr in order to make things easier to review.

adriansmares avatar Jul 29 '22 14:07 adriansmares

Recent changes on this PR.

@adriansmares pointed out something pertinent, the end-device payload formatter should be set to point to the DR. While there is some handling on the backend that automatically does this, it only happens when if the info parameter is set and from the references bellow I couldn't find it being set on the remote or local stores.

I ended up taking the simpler approach of just adding the formatter to the fetched profile.

CC: @adriansmares @KrishnaIyer

nicholaspcr avatar Aug 04 '22 22:08 nicholaspcr

Blocking this since there are some chances that were discussed with @adriansmares that are necessary for this PR, but for now I'm prioritizing the entity limit notification.

The changes discussed were:

  • Removing the fieldMask added on this PR (should no longer be needed)
  • Update the Device Repository in order to get the full versionID when making the request only with vendorID and vendorProfileID.
  • Return the DR as a formatter when applicable ( this when making the request only with vendorID and vendorProfileID )
  • Update the API to use oneof in order to also accept passing the full versionID, instead of just accepting the endDeviceProfileID

nicholaspcr avatar Aug 09 '22 15:08 nicholaspcr

  • Update the Device Repository in order to get the full versionID when making the request only with vendorID and vendorProfileID.

Unfortunately this isn't possible.

The vendor ID can be mapped 1:0..1 with brands, that's the easy part.

However, many devices can use the same LoRaWAN device profile and codec. This is a feature of the Device Repository: declare the profile and codec once, and reference many times.

We cannot turn this around. We cannot go from a device profile (by ID) to versions. That would be a 1:N relation: we could return all version IDs that reference the profile, but we cannot return the exact version IDs.

So, on import, we can't use vendor ID and profile ID as keys. We may only go as far as getting the device profile by vendor ID and profile ID (so LoRaWAN version, RP version, class B/C parameters etc), which is still helpful.

johanstokking avatar Aug 16 '22 11:08 johanstokking

  • Update the Device Repository in order to get the full versionID when making the request only with vendorID and vendorProfileID.

Unfortunately this isn't possible.

The vendor ID can be mapped 1:0..1 with brands, that's the easy part.

However, many devices can use the same LoRaWAN device profile and codec. This is a feature of the Device Repository: declare the profile and codec once, and reference many times.

We cannot turn this around. We cannot go from a device profile (by ID) to versions. That would be a 1:N relation: we could return all version IDs that reference the profile, but we cannot return the exact version IDs.

So, on import, we can't use vendor ID and profile ID as keys. We may only go as far as getting the device profile by vendor ID and profile ID (so LoRaWAN version, RP version, class B/C parameters etc), which is still helpful.

With this in mind I just changed the API so that the convert request receives the versionIDs and changed the validation.

In regards to this:

We may only go as far as getting the device profile by vendor ID and profile ID (so LoRaWAN version, RP version, class B/C parameters etc), which is still helpful.

The same information should be able to be fetched when passing the other fields in the versionID, no? I ended up allowing the user to provide the vendorID and profileID but the way I see it the current profile fetching doesn't really use those two specific values since I believe the same data could be fetched while using the other fields.

nicholaspcr avatar Aug 17 '22 23:08 nicholaspcr

The same information should be able to be fetched when passing the other fields in the versionID, no? I ended up allowing the user to provide the vendorID and profileID but the way I see it the current profile fetching doesn't really use those two specific values since I believe the same data could be fetched while using the other fields.

Yes right. But sometimes the version IDs aren't available. For instance, in the standard LoRa Alliance QR code, there's only vendor ID and profile ID, and all we can do is prefill the device profile. The user still has to select which device it is.

johanstokking avatar Aug 18 '22 09:08 johanstokking

The same information should be able to be fetched when passing the other fields in the versionID, no? I ended up allowing the user to provide the vendorID and profileID but the way I see it the current profile fetching doesn't really use those two specific values since I believe the same data could be fetched while using the other fields.

Yes right. But sometimes the version IDs aren't available. For instance, in the standard LoRa Alliance QR code, there's only vendor ID and profile ID, and all we can do is prefill the device profile. The user still has to select which device it is.

With this in mind I opted to change things a bit, hopefully its more appropriate this way. I wrote a second fetcher on DTC that only takes into consideration the vendorID/ProfileID and instead of providing one fetcher to TTSCSV I pass both the vendorID fetcher and the versionID fetchers.

nicholaspcr avatar Aug 18 '22 14:08 nicholaspcr

@nicholaspcr please request review when this is ready for another pass.

johanstokking avatar Aug 22 '22 09:08 johanstokking