TheThingsNetwork / lorawan-stack

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

Payload formatters: nesting of repository - application - device causes confusion #4081

Closed descartes closed 2 years ago

descartes commented 3 years ago

Summary

If the device repository has a JS formatter it is automatically used by the device. All good. Except there is no indication that the this is going to happen and there isn't enough indication of what is going on at each level.

Setting the application formatter type to None does not override the repository. Setting the device type to None does override the application.

Steps to Reproduce

I created a new device entry for the Arduino MKR WAN 1310 which put apparently random entries in to the console on uplinks, see https://github.com/TheThingsNetwork/lorawan-devices/issues/139

Setting the application uplink formatter to none did not turn off the repository script.

I created the formatter I wanted and put it in at the appliction level. But still the dev repro formatter was still in effect. I subsequently found I had to go in to the device to change its formatter type from 'Repository' to 'Use application payload formatter' for it to come good.

What do you see now?

No UI indication that something somewhere else will override what you have set.

What do you want to see instead?

Possibly some bold text - "Ensure you have turned off all the other bits" or similar.

Short term, add a FORMATTER_APPLICATION to the CLI device create/set for --formatters.up-formatter string FORMATTER_CAYENNELPP|FORMATTER_GRPC_SERVICE|FORMATTER_JAVASCRIPT|FORMATTER_NONE|FORMATTER_REPOSITORY

Medium term some message in the console UI saying "This is overridden by X for this application / device" and some way of blitzing any settings at device level - if I import 100 devices, I'd want them to only use the application or repository formatter, I don't want to have to adjust each one manually.

How do you propose to implement this?

Maybe I should learn React, or improve my Go from hacker to guru - otherwise, no, not me.

How do you propose to test this?

Happy to test on a development server if you want me to.

Can you do this yourself and submit a Pull Request?

Sorry, no.

kschiffer commented 3 years ago

Thanks for filing this issue @descartes 

Indeed the payload formatter prioritization is not communicated clearly enough yet. What is important to know here is that end device level PFs always take precedence over application level PFs. So anything you set on the device PF tab is going to be used for this particular end device, that includes the "None" option, which means that no PF will be applied for the end device.

Short term, add a FORMATTER_APPLICATION to the CLI device create/set for --formatters.up-formatter string FORMATTER_CAYENNELPP|FORMATTER_GRPC_SERVICE|FORMATTER_JAVASCRIPT|FORMATTER_NONE|FORMATTER_REPOSITORY

Actually, such FORMATTER_APPLICATION does not actually exist. We only call it as such in the Console when no payload formatter is applied (as in no payload formatter setting is made at all, also no FORMATTER_NONE), since using the application PF then is the practical effect. To recreate that in the CLI, you can simply omit the --formatters.[up|down]-formatter command.

I'll revisit the messages we display in the Console to make sure that these mechanics are made clear.

kschiffer commented 2 years ago

We have made a couple of improvements to address these issues:

As such, I will close this issue. Thanks for filing this @descartes. Consider reopening or creating a new issue if you disagree with seeing this issue as closed.