search

TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
975 stars 306 forks source link

Harmonize device registry get/set signatures #181

Closed johanstokking closed 2 years ago

johanstokking commented 5 years ago

Summary:

Harmonize device registry get/set signatures.

Why do we need this?

For a consistent developer experience.

What is already there? What do you see now?

  1. AS device registry takes ttnpb.EndDeviceIdentifiers and its primary identifiers
  2. NS device registry takes the primary identifiers as strings

The reason I don't like 2 is that everywhere in the codebase we assume that ttnpb.*Identifiers has filled out primary identifiers. Otherwise validators fail. Otherwise the unique package panics. There is no reason to clarify that some method or function requires primary identifiers to be present as part of the call signature. pkg/unique doesn't even do this.

What is missing? What do you want to see?

NS to use same signature as AS.

How do you propose to implement this?

Copy paste interface methods and implement accordingly.

What can you do yourself and what do you need help with?

Review.

johanstokking commented 5 years ago

Pushing this to Backlog.

This is a candidate to close; I don't think we should do this in the end after all the refactorings and harmonizations that we did in the device registries.

nejraselimovic commented 3 years ago

@johanstokking should we close this or not?

johanstokking commented 3 years ago

Let's leave open for future reference.

johanstokking commented 2 years ago

This is already addressed. As far as there are any inconsistencies, we should address that as part of https://github.com/TheThingsIndustries/lorawan-stack/issues/2256