Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch device info by vendor id and vendor profile id #7224

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

vlasebian
Copy link
Contributor

@vlasebian vlasebian commented Aug 7, 2024

Summary

References #7222

Changes

  • Introduce a new EndDeviceProvisioningInfo message
  • Introduce a new endpoint, /dr/vendors/{vendor_id}/profiles/{vendor_profile_id}/provisioning-info
  • Implement bleve index for EndDeviceProvisioningInfo
  • Implement a new method in the device repository to list end device profiles by brand id
  • Added tests for the remote store and bleve store

After the discussion in this PR, I dropped all the changes related to the GetProvisioningInfo endpoint and started fresh. These are the changes that I did:

  • Dropped the profiles index from the bleve store (it was used only to fetch templates)
  • Added a new index for templates in the bleve store, so templates are returned directly instead of fetching a profile and then building the template
  • Updated the reading of profiles from the device repository to account for the profile identifiers in the vendor index too. This is how reading end device profiles works now:
  • All the profiles are read as previously (all the device models are checked for profile identifiers, then the profiles are read from their own yaml files)
  • The vendor index is checked for any profile identifiers. For each profile identifier, the profile corresponding to that identifier is enriched with the vendor profile id. This is needed for the bleve index to be able to search using vendor id + vendor profile id.
  • Dropped the profile parameter from the GetTemplate method in the store interface as it's not needed anymore
  • Updated tests, fixed linter warnings

Testing

For testing, I did curl requests to check all the endpoint bindings for GetTemplate.

Steps

Prerequisites:

  • Have tts built and running locally
  • Have an application created
  • Get a bearer token to authenticate the request (can be done by opening the console, logging in with the default credentials and checking a request header in the network tab of the developer console)
Results
  1. curl -s -H "Authorization: $TTN_AUTH_TOKEN" localhost:1885/api/v3/dr/vendors/474/profiles/1/template | jq
{
  "end_device": {
    "version_ids": {
      "brand_id": "minol-zenner",
      "model_id": "edc-communication-module",
      "hardware_version": "1.0",
      "firmware_version": "2.02.0",
      "band_id": "EU_863_870"
    },
    "lorawan_version": "MAC_V1_0_3",
    "lorawan_phy_version": "PHY_V1_0_3_REV_A",
    "supports_join": true,
    "mac_settings": {
      "supports_32_bit_f_cnt": true
    },
    "formatters": {
      "up_formatter": "FORMATTER_REPOSITORY",
      "down_formatter": "FORMATTER_REPOSITORY"
    }
  },
  "field_mask": {
    "paths": [
      "version_ids",
      "supports_join",
      "supports_class_b",
      "supports_class_c",
      "lorawan_version",
      "lorawan_phy_version",
      "formatters",
      "mac_settings.supports_32_bit_f_cnt"
    ]
  }
}
  1. curl -s -H "Authorization: $TTN_AUTH_TOKEN" localhost:1885/api/v3/dr/applications/app1/vendors/474/profiles/1/template | jq
{
  "end_device": {
    "version_ids": {
      "brand_id": "minol-zenner",
      "model_id": "edc-communication-module",
      "hardware_version": "1.0",
      "firmware_version": "2.02.0",
      "band_id": "EU_863_870"
    },
    "lorawan_version": "MAC_V1_0_3",
    "lorawan_phy_version": "PHY_V1_0_3_REV_A",
    "supports_join": true,
    "mac_settings": {
      "supports_32_bit_f_cnt": true
    },
    "formatters": {
      "up_formatter": "FORMATTER_REPOSITORY",
      "down_formatter": "FORMATTER_REPOSITORY"
    }
  },
  "field_mask": {
    "paths": [
      "version_ids",
      "supports_join",
      "supports_class_b",
      "supports_class_c",
      "lorawan_version",
      "lorawan_phy_version",
      "formatters",
      "mac_settings.supports_32_bit_f_cnt"
    ]
  }
}
  1. curl -s -H "Authorization: $TTN_AUTH_TOKEN" localhost:1885/api/v3/dr/brands/the-things-industries/models/generic-node-sensor-edition/1.0/US_902_928/template | jq
{
  "end_device": {
    "version_ids": {
      "brand_id": "the-things-industries",
      "model_id": "generic-node-sensor-edition",
      "firmware_version": "1.0",
      "band_id": "US_902_928"
    },
    "lorawan_version": "MAC_V1_0_3",
    "lorawan_phy_version": "PHY_V1_0_3_REV_A",
    "supports_join": true,
    "mac_settings": {
      "supports_32_bit_f_cnt": true
    },
    "formatters": {
      "up_formatter": "FORMATTER_REPOSITORY",
      "down_formatter": "FORMATTER_REPOSITORY"
    }
  },
  "field_mask": {
    "paths": [
      "version_ids",
      "supports_join",
      "supports_class_b",
      "supports_class_c",
      "lorawan_version",
      "lorawan_phy_version",
      "formatters",
      "mac_settings.supports_32_bit_f_cnt"
    ]
  }
}
  1. curl -s -H "Authorization: $TTN_AUTH_TOKEN" localhost:1885/api/v3/dr/applications/app1/brands/the-things-industries/models/generic-node-sensor-edition/1.0/US_902_928/template | jq
{
  "end_device": {
    "version_ids": {
      "brand_id": "the-things-industries",
      "model_id": "generic-node-sensor-edition",
      "firmware_version": "1.0",
      "band_id": "US_902_928"
    },
    "lorawan_version": "MAC_V1_0_3",
    "lorawan_phy_version": "PHY_V1_0_3_REV_A",
    "supports_join": true,
    "mac_settings": {
      "supports_32_bit_f_cnt": true
    },
    "formatters": {
      "up_formatter": "FORMATTER_REPOSITORY",
      "down_formatter": "FORMATTER_REPOSITORY"
    }
  },
  "field_mask": {
    "paths": [
      "version_ids",
      "supports_join",
      "supports_class_b",
      "supports_class_c",
      "lorawan_version",
      "lorawan_phy_version",
      "formatters",
      "mac_settings.supports_32_bit_f_cnt"
    ]
  }
}
Regressions

...

Notes for Reviewers

...

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • 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.
  • Testing: The steps/process to test this feature are clearly explained including testing for regressions.
  • Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from e2dd892 to 3e9211f Compare August 7, 2024 11:10
@vlasebian
Copy link
Contributor Author

Hey, @johanstokking! @KrishnaIyer and I were discussing the final format for the response of this endpoint. As it is implemented now, the proto message is the following:

message EndDeviceProvisioningInfo {
  EndDeviceModel end_device_model = 1;
  EndDeviceTemplate end_device_template = 2;
}

The full response looks like this:

{
  "end_device_model": {
    "name": "Generic Node (Sensor Edition)",
    "description": "The Things Industries Generic Node Sensor Edition is a LoRaWAN® development board that consists of a temperature and humidity sensor and an accelerometer. These onboard sensors can be used to measure temperature, humidity, motion, free fall, and orientation. In addition to that, it provides expansion slots for connecting various sensors. It is suitable for a wide range of use cases that cover industrial, farming, facility management, and leisure.",
    "key_provisioning": [
      "custom"
    ],
    "photos": {
      "main": "gnse.png"
    },
    "product_url": "https://www.genericnode.com/",
    "datasheet_url": "https://github.com/TheThingsIndustries/generic-node-se"
  },
  "end_device_template": {
    "end_device": {
      "version_ids": {
        "brand_id": "the-things-industries",
        "model_id": "generic-node-sensor-edition",
        "hardware_version": "1.1",
        "firmware_version": "1.0",
        "band_id": "EU_863_870"
      },
      "lorawan_version": "MAC_V1_0_3",
      "lorawan_phy_version": "PHY_V1_0_3_REV_A",
      "supports_join": true,
      "mac_settings": {
        "supports_32_bit_f_cnt": true
      },
      "formatters": {
        "up_formatter": "FORMATTER_REPOSITORY",
        "down_formatter": "FORMATTER_REPOSITORY"
      }
    },
    "field_mask": {
      "paths": [
        "version_ids",
        "supports_join",
        "supports_class_b",
        "supports_class_c",
        "lorawan_version",
        "lorawan_phy_version",
        "formatters",
        "mac_settings.supports_32_bit_f_cnt"
      ]
    }
  }
}

Our question is, is it relevant to have the full device template response? Wouldn't the device be enough for this case? What I mean is have the proto message as:

message EndDeviceProvisioningInfo {
  EndDeviceModel end_device_model = 1;
  EndDevice end_device = 2;
}

And the full response like this:

{
  "end_device_model": {
    "name": "Generic Node (Sensor Edition)",
    "description": "The Things Industries Generic Node Sensor Edition is a LoRaWAN® development board that consists of a temperature and humidity sensor and an accelerometer. These onboard sensors can be used to measure temperature, humidity, motion, free fall, and orientation. In addition to that, it provides expansion slots for connecting various sensors. It is suitable for a wide range of use cases that cover industrial, farming, facility management, and leisure.",
    "key_provisioning": [
      "custom"
    ],
    "photos": {
      "main": "gnse.png"
    },
    "product_url": "https://www.genericnode.com/",
    "datasheet_url": "https://github.com/TheThingsIndustries/generic-node-se"
  },
  "end_device": {
    "version_ids": {
      "brand_id": "the-things-industries",
      "model_id": "generic-node-sensor-edition",
      "hardware_version": "1.1",
      "firmware_version": "1.0",
      "band_id": "EU_863_870"
    },
    "lorawan_version": "MAC_V1_0_3",
    "lorawan_phy_version": "PHY_V1_0_3_REV_A",
    "supports_join": true,
    "mac_settings": {
      "supports_32_bit_f_cnt": true
    },
    "formatters": {
      "up_formatter": "FORMATTER_REPOSITORY",
      "down_formatter": "FORMATTER_REPOSITORY"
    }
  }
}

@vlasebian vlasebian marked this pull request as ready for review August 13, 2024 14:13
@vlasebian vlasebian requested review from a team as code owners August 13, 2024 14:13
@vlasebian vlasebian changed the title feature: Fetch dev info with vendor id and vendor profile id feature: Fetch device info by vendor id and vendor profile id Aug 13, 2024
@johanstokking
Copy link
Member

@vlasebian @KrishnaIyer can you please triage this PR (assignee, milestone) so I know priorities for review.

@KrishnaIyer KrishnaIyer added this to the v3.32.1 milestone Aug 14, 2024
@KrishnaIyer
Copy link
Member

Yup. @vlasebian: Always make sure to add the milestone and assign yourself to your PRs.

Copy link
Member

@KrishnaIyer KrishnaIyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only briefly looked at this. Look generally good.

pkg/devicerepository/store/remote/schema.go Outdated Show resolved Hide resolved
api/ttn/lorawan/v3/devicerepository.proto Outdated Show resolved Hide resolved
@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from bb127ec to cd2d527 Compare August 20, 2024 11:51
@vlasebian vlasebian added the c/device repository This is related to the Device Repository Component label Aug 20, 2024
Copy link
Member

@johanstokking johanstokking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only reviewed API

api/ttn/lorawan/v3/devicerepository.proto Outdated Show resolved Hide resolved
api/ttn/lorawan/v3/devicerepository.proto Outdated Show resolved Hide resolved
api/ttn/lorawan/v3/devicerepository.proto Outdated Show resolved Hide resolved
pkg/devicerepository/grpc.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/init.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/init.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/store.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/profiles.go Outdated Show resolved Hide resolved
@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from 116ef1c to 804e107 Compare August 27, 2024 07:48
@johanstokking
Copy link
Member

Looks great. I tested this locally. I had to do tools/bin/mage dev:initDeviceRepo to rebuild the index but otherwise it works.

Now, I see that the response is:

{
    "end_device_version_ids": {
        "brand_id": "the-things-industries",
        "model_id": "generic-node-sensor-edition",
        "hardware_version": "1.1",
        "firmware_version": "1.0",
        "band_id": "EU_863_870"
    },
    "end_device_template": {
        "end_device": {
            "version_ids": {
                "brand_id": "the-things-industries",
                "model_id": "generic-node-sensor-edition",
                "hardware_version": "1.1",
                "firmware_version": "1.0",
                "band_id": "EU_863_870"
            },
            "lorawan_version": "MAC_V1_0_3",
            "lorawan_phy_version": "PHY_V1_0_3_REV_A",
            "supports_join": true,
            "mac_settings": {
                "supports_32_bit_f_cnt": true
            },
            "formatters": {
                "up_formatter": "FORMATTER_REPOSITORY",
                "down_formatter": "FORMATTER_REPOSITORY"
            }
        },
        "field_mask": {
            "paths": [
                "version_ids",
                "supports_join",
                "supports_class_b",
                "supports_class_c",
                "lorawan_version",
                "lorawan_phy_version",
                "formatters",
                "mac_settings.supports_32_bit_f_cnt"
            ]
        }
    }
}

So basically I wonder why we need to return the version identifiers if they're part of the template? I guess I didn't fully realize this before, but I think we can just return EndDeviceTemplate instead?

@vlasebian
Copy link
Contributor Author

vlasebian commented Aug 29, 2024

I believe there already is an endpoint for that:

/dr/vendors/{end_device_profile_ids.vendor_id}/profiles/{end_device_profile_ids.vendor_profile_id}/template

But from what I understood at the beginning, the profiles can be specified in two places:

  • Embedded in the device yaml
  • Specified in the vendor index

This endpoint was for fetching the profiles specified in the vendor index.

@johanstokking
Copy link
Member

But from what I understood at the beginning, the profiles can be specified in two places:

* Embedded in the device yaml

* Specified in the vendor index

This endpoint was for fetching the profiles specified in the vendor index.

Ah I see now, thanks for clearing that up.

We changed the place where the vendor profile ID was defined. First, it was part of the profile itself. However, since the profile can be shared by multiple combinations of device IDs, firmware versions and hardware versions, we couldn't determine which device we're dealing with.

To overcome that limitation, we moved the vendor profile ID to a mapping at the vendor level. The other declaration no longer exists. To be backwards compatible, and to allow vendors to still use a vendor profile ID just for the profile and codec (that can be shared), we have this "secondary" use with references.

The only thing that is useful for us, however, are the "primary" identifiers as declared at the vendor-level mapping of profile ID to device ID, firmware version, hardware version and region.

If we can repurpose the existing API that you referenced to basically return the EndDeviceTemplate that we want, that would be most ideal.

@vlasebian
Copy link
Contributor Author

Sure, I can change the existing endpoint that fetches end device templates to fetch the vendor profile from the vendor index. Do you know if this endpoint is used somewhere else? I want to make sure it's a backward compatible change. Also, are we certain that profiles embedded in the end device model are not used anymore?

@johanstokking
Copy link
Member

Sure, I can change the existing endpoint that fetches end device templates to fetch the vendor profile from the vendor index. Do you know if this endpoint is used somewhere else? I want to make sure it's a backward compatible change.

I'm not aware of use of it by us, maybe API clients, but as long as you keep the types the same I wouldn't expect any compatibility issues.

Also, are we certain that profiles embedded in the end device model are not used anymore?

Yes we're sure because we removed them from the Device Repository. Adding them by mistake would cause validation to fail.

@vlasebian
Copy link
Contributor Author

Currently, these are the registered endpoints:

rpc GetTemplate(GetTemplateRequest) returns (EndDeviceTemplate) {
  option (google.api.http) = {
    get: "/dr/brands/{version_ids.brand_id}/models/{version_ids.model_id}/{version_ids.firmware_version}/{version_ids.band_id}/template"
    additional_bindings {get: "/dr/vendors/{end_device_profile_ids.vendor_id}/profiles/{end_device_profile_ids.vendor_profile_id}/template"}
    additional_bindings {get: "/dr/applications/{application_ids.application_id}/brands/{version_ids.brand_id}/models/{version_ids.model_id}/{version_ids.firmware_version}/{version_ids.band_id}/template"}
    additional_bindings {get: "/dr/applications/{application_ids.application_id}/vendors/{end_device_profile_ids.vendor_id}/profiles/{end_device_profile_ids.vendor_profile_id}/template"}
  };
}

And the GetTemplateRequest looks like this:

message GetTemplateRequest {
  // Identifiers to uniquely identify a LoRaWAN end device profile.
  message EndDeviceProfileIdentifiers {
    // VendorID managed by the LoRa Alliance, as defined in TR005.
    uint32 vendor_id = 1 [(validate.rules).uint32 = {gte: 1}];

    // ID of the LoRaWAN end device profile assigned by the vendor.
    uint32 vendor_profile_id = 2;
  }

  // Application identifiers.
  ApplicationIdentifiers application_ids = 1 [deprecated = true];

  // End device version information.
  // Use either EndDeviceVersionIdentifiers or EndDeviceProfileIdentifiers.
  EndDeviceVersionIdentifiers version_ids = 2;

  // End device profile identifiers.
  EndDeviceProfileIdentifiers end_device_profile_ids = 3;
}

Should the bindings that use something other than vendor_id and vendor_profile_id still be available? E.g.

"/dr/brands/{version_ids.brand_id}/models/{version_ids.model_id}/{version_ids.firmware_version}/{version_ids.band_id}/template"
"/dr/applications/{application_ids.application_id}/brands/{version_ids.brand_id}/models/{version_ids.model_id}/{version_ids.firmware_version}/{version_ids.band_id}/template"

Or these should be deprecated along with the version_ids field in the GetTemplateRequest?

@johanstokking johanstokking changed the title feature: Fetch device info by vendor id and vendor profile id Fetch device info by vendor id and vendor profile id Sep 6, 2024
@johanstokking
Copy link
Member

Yes, keep them available, as we provide API compatibility until we release version 4.

@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from ce853d8 to 9c25e2e Compare September 10, 2024 08:52
@github-actions github-actions bot added the ui/web This is related to a web interface label Sep 10, 2024
@vlasebian
Copy link
Contributor Author

I dropped all the changes related to the GetProvisioningInfo endpoint and started fresh. These are the changes that I did:

  • Dropped the profiles index from the bleve store (it was used only to fetch templates)
  • Added a new index for templates in the bleve store, so templates are returned directly instead of fetching a profile and then building the template
  • Updated the reading of profiles from the device repository to account for the profile identifiers in the vendor index too. This is how reading end device profiles works now:
    • All the profiles are read as previously (all the device models are checked for profile identifiers, then the profiles are read from their own yaml files)
    • The vendor index is checked for any profile identifiers. For each profile identifier, the profile corresponding to that identifier is enriched with the vendor profile id. This is needed for the bleve index to be able to search using vendor id + vendor profile id.
  • Dropped the profile parameter from the GetTemplate method in the store interface as it's not needed anymore
  • Updated tests, fixed linter warnings

Let me know if this is okay now.

Copy link
Member

@johanstokking johanstokking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, LGTM

Please update the changelog

@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from 4b19045 to a1a19d8 Compare September 10, 2024 13:47
Copy link
Member

@KrishnaIyer KrishnaIyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks pretty good. A few comments.

pkg/devicerepository/store/bleve/bleve.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/bleve.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/store.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/bleve/store.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/remote/remote.go Outdated Show resolved Hide resolved
pkg/devicerepository/store/remote/remote.go Outdated Show resolved Hide resolved
@vlasebian vlasebian force-pushed the feature/fetch-dev-info-with-vendor-id-profile-id branch from a1a19d8 to f7f8cc6 Compare September 11, 2024 08:17
@vlasebian vlasebian merged commit f51ccc6 into v3.32 Sep 11, 2024
14 of 15 checks passed
@vlasebian vlasebian deleted the feature/fetch-dev-info-with-vendor-id-profile-id branch September 11, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/device repository This is related to the Device Repository Component ui/web This is related to a web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants