Feat uci mqtt forwarder add commands and metadata

[devleesch001]

Implemente Commands and Metadata of chirpstack-mqtt-forwarder with UCI in chirpstack-mqtt-forwarder.sh for openWrt init.d service.

UCI config file

config commands 'reboot'
	option command '["/usr/bin/reboot"]'

config commands 'shutdown'
	option command '["/usr/bin/shutdown"]'

config metadata 'datetime'
	option command '["date", "-R"]'

config metadata 'serial_number'
	option static '1234'

generate

[metadata]
[metadata.static]
serial_number=1234
[metadata.commands]
datetime=["date", "-R"]
[commands]
reboot=["/usr/bin/reboot"]
shutdown=["/usr/bin/shutdown"]

[devleesch001]

I did not use the uci list via config_list_foreach because this does not guarantee the order of the list, apart from the order of the commands arguments is essential

[brocaar]

Thanks, in general I think this looks good! Before I merge this, let me look into if toml_array_validator is needed, or if we can use a different syntax which allows to iterate over the command arguments. Not tested, but wouldn't it be better to make option command a list command? And then iterate over the uci list output of command? That might also make it easier to integrate this into LuCI?

An example is how the timeservers are configured in /etc/config/system

config timeserver 'ntp'
	option enabled '1'
	option enable_server '0'
	list server '0.openwrt.pool.ntp.org'
	list server '1.openwrt.pool.ntp.org'
	list server '2.openwrt.pool.ntp.org'
	list server '3.openwrt.pool.ntp.org'

uci show system

system.@system[0]=system
system.@system[0].hostname='chirpstack-3b780a'
system.@system[0].timezone='UTC'
system.@system[0].ttylogin='0'
system.@system[0].log_size='64'
system.@system[0].urandom_seed='0'
system.ntp=timeserver
system.ntp.enabled='1'
system.ntp.enable_server='0'
system.ntp.server='0.openwrt.pool.ntp.org' '1.openwrt.pool.ntp.org' '2.openwrt.pool.ntp.org' '3.openwrt.pool.ntp.org'

[devleesch001]

Hello,

I was wondering about that.

What worried me most was that to generate the configuration, the order of the options is, in this particular case, really important. This doesn't seem to be the case for other configurations, such as the timeserver, or the filter of the chirpstack-mqtt-forwarder.

For example, here the order is irrelevant:

config filters
	list dev_addr_prefix '0000ff00/24
	list dev_addr_prefix '0000aa00
	list join_eui_prefix '0000ff0000000000/24'
	list join_eui_prefix '0000aa0000000000/24'

If an external tool happens to sort the list, or an interface to display the list in the wrong order, this could break things.

Another point being the display in the interface, I don't know how this works in detail so I may be wrong.

But I don't think the display and configuration of commands and metadata should resemble that of lists (like filters or ntp servers). This could be annoying, as you might have to modify an argument in the middle of the list, which doesn't seem to be possible from the interface, and I think it's really impractical for this specific case.

But it could just be an implementation detail on LuCi and therefore not a problem.

If the current method really isn't suitable, I can change it to use the uci list.

[brocaar]

Ah that is what you meant with:

I did not use the uci list via config_list_foreach because this does not guarantee the order of the list

Do you have a reference that confirms that the order is not preserved? When reading https://openwrt.org/docs/guide-user/base-system/uci#file_syntax, it says:

In the lines starting with a list keyword an option with multiple values is defined. All list statements that share the same name, collection in our example, will be combined into a single list of values with the same order as in the configuration file.

So it seems order is guaranteed?

This could be annoying, as you might have to modify an argument in the middle of the list, which doesn't seem to be possible from the interface, and I think it's really impractical for this specific case.

I agree, but this is a limitation of the UI, not the config format. Even if the UI limits this, I think it is better to define the config format in the correct way. I don't think mixing TOML within UCI is a good idea. E.g. if we would like to parse this config and execute it within the LuCI (e.g. testing the command output), we would need to parse the TOML list first.

Btw, it might actually be possible to make list elements editable once set (default is false):

https://openwrt.github.io/luci/jsapi/LuCI.form.DynamicList.html#editable

---

Could you allow me to push changes to this pull-request (there should be a setting that you can toggle in the PR)? Then I can look into the LuCI part. Maybe it is better merge this PR together with the LuCI changes, as the config format defines how it will be displayed in LuCI.

[devleesch001]

Oh I didn't see that

Do you have a reference that confirms that the order is not preserved? When reading https://openwrt.org/docs/guide-user/base-system/uci#file_syntax, it says:

I agree

think it is better to define the config format in the correct way. I don't think mixing TOML within UCI is a good idea

I will modify the script to use UCI list rather than toml format.

I can allow you to push, but Allow edits by maintainers is already active if is that do you need

On my fork the branch of this pull request is feat-uci-mqtt-forwarder-add-commands-and-metadata

[devleesch001]

I update script.

now UCI config file

config commands 'reboot'
        list command '/usr/bin/reboot'

config commands 'shutdown'
        list command '/usr/bin/shutdown'
        list command 'now'

config metadata 'datetime'
        list command 'date'
        list command '-R'

config metadata 'serial_number'
        option static '1234'

generate this toml file

[metadata]
[metadata.static]
serial_number=1234
[metadata.commands]
datetime=["date","-R",]
[commands]
reboot=["/usr/bin/reboot",]
shutdown=["/usr/bin/shutdown","now",]