ttn-lw-cli end-devices create doesn't set AppEUI (JoinEUI) correctly

[LasseRegin]

Summary

When creating an end device using the ttn-lw-cli end-devices create command and providing arguments using a json file, the AppEUI is not correctly set even though it is defined in the json file.

Steps to Reproduce

1. Export device from existing application using

   ttn-lw-cli end-devices get \
   --application-id $APPLICATION_ID \
   --device-id $DEVICE_ID \
   --name \
   --description \
   --supports-join \
   --root-keys > device_info.json

2. Create device in another application using

   ttn-lw-cli end-devices create \
   --application-id $APPLICATION_ID_2  < device_info.json

What do you see now?

Device shows 0000000000000000 as AppEUI on the console.

Output of ttn-lw-cli end-devices get shows

{
  "ids": {
    "device_id": "w...",
    "application_ids": {
      "application_id": "w..."
    },
    "dev_eui": "BF...",
    "join_eui": "0000000000000000"
  },
  ...
}

What do you want to see instead?

I want to see the join_eui set to the ID defined in the exported json file device_info.json.

Environment

CLI version

> ttn-lw-cli version
The Things Network Command-line Interface: ttn-lw-cli
Version:             3.18.1
Build date:          2022-03-09T10:50:38Z
Git commit:          f7d29b11e
Go version:          go1.17.7
OS/Arch:             darwin/amd64

MacOS version

> sw_vers -productVersion
12.2.1

How do you propose to implement this?

Make the cli command use the joinEUI defined in the json file.

How do you propose to test this?

Test if the ID from the json file is properly registered by the command.

Can you do this yourself and submit a Pull Request?

@KrishnaIyer already gave some support on this on Slack. Thank you for this.

[LasseRegin]

NOTE: By manually adding the joinEUI flag in the create command the ID is correctly set:

ttn-lw-cli end-devices create \
    --join-eui $APP_EUI \
    --application-id $APPLICATION_ID_2  < device_info.json

however I still believe the original command should work.

[nicholaspcr]

I did some testing and confirmed that this indeed happens in the v3.18.1 binaries that are available here.

Although it doesn't happen with the v3.18.2 binaries. I'm looking into what commit changed this behaviour, once I find it it we will do a re-release of the version v3.18.1.

@LasseRegin It shouldn't take much time to find and update the v3.18.1 cli binary but if alternatively you could use the v3.18.2 version found here.

[nicholaspcr]

@KrishnaIyer @adriansmares

It seems that in the v3.18.0 the end-device command changed in a way that if the flag values are not set, the valued obtained from the json object will be discarded and the default value will always be used. It was later fixed on the v3.18.2 but currently in both the v3.18.1 and v3.18.0 releases there is a bug where the JoinEui obtained from the json file will never be used.

The line that causes it is https://github.com/TheThingsNetwork/lorawan-stack/blob/f7d29b11eefd87b37fe1ff8eec08b93ff77e3cce/cmd/ttn-lw-cli/commands/end_devices.go#L458

which should be

if device.Ids.JoinEui == nil { 

I'm a bit out of the loop on the proceedings regarding fixes of previous releases and the re-release process, so I opted to tag both of you and hopefully get the chance to follow up how its done.

[adriansmares]

So apparently the bug that resets the JoinEUI in 3.18.0/3.18.1 got added in https://github.com/TheThingsNetwork/lorawan-stack/pull/5202, then it got reverted in https://github.com/TheThingsNetwork/lorawan-stack/pull/5324. I'm now curious if 3.18.2 suffers from https://github.com/TheThingsNetwork/lorawan-stack/issues/5140 again.

Since we have a working binary (3.18.2) we don't need to pull / change releases, but we should check if https://github.com/TheThingsNetwork/lorawan-stack/issues/5140 returned.

[nicholaspcr]

The bug in #5140 indeed has returned, will do a quick fix for it then.

I assumed that even with the working binary of 3.18.2 we would re-release the other binaries but makes sense to leave as is for minor releases. I can create a PR to fix the bug of the logger displaying that the default value was used but the question now is if we should do a re-release of this minor fix or if it's ok to leave it be on the 3.19 release. I don't think the bug fix was specified in the changelog so I'm not sure if its needed or not.

[nicholaspcr]

Since the bug was confirmed to not exist in the latest minor version I'm closing this issue.

@LasseRegin please use the 3.18.2 version to avoid having the error that you mentioned.