Thing config validator doesn't allow for serial ports with rfc2217://127.0.0.1:2000 syntax

[t2000]

I just tried the latest snapshot of openhab and noticed that my novafinedust things had an error on startup.

For a normal user it is IMPOSSIBLE to find out what was wrong... With knowledge of the core code I was able to activate the right TRACE level for the core.thing.internal.ThingManagerImpl to see the issue.

Such things should definitely e logged on WARN level!

-02-24 20:19:34.147 [DEBUG] [core.thing.internal.ThingManagerImpl] - Thing novafinedust:SDS011:fineDustSensor will be enabled.
2022-02-24 20:19:34.147 [DEBUG] [core.thing.internal.ThingManagerImpl] - Calling 'NovaFineDustHandlerFactory.registerHandler()' for thing 'novafinedust:SDS011:fineDustSensor'.
2022-02-24 20:19:34.181 [TRACE] [core.thing.internal.ThingManagerImpl] - Failed to validate config for 'novafinedust:SDS011:fineDustSensor' with URI 'thing-type:novafinedust:SDS011': {port=The value rfc2217://10.0.6.100:2002 does not match allowed parameter options. Allowed options are: [ParameterOption [value="/dev/ttyUSB2", label="/dev/ttyUSB2"], ParameterOption [value="/dev/ttyUSB1", label="/dev/ttyUSB1"], ParameterOption [value="/dev/ttyUSB0", label="/dev/ttyUSB0"], ParameterOption [value="/dev/ttyUSB2", label="/dev/ttyUSB2"], ParameterOption [value="/dev/ttyUSB1", label="/dev/ttyUSB1"], ParameterOption [value="/dev/ttyUSB0", label="/dev/ttyUSB0"]]}

Then I was wondering why my smartmeter thing still worked, although they also use thr RFC syntax for serial port over ethernet.

Turns out the difference from https://github.com/openhab/openhab-addons/blob/e84e2800a46b3de98d59777383e6b634cb896af5/bundles/org.openhab.binding.smartmeter/src/main/resources/OH-INF/thing/thing-types.xml#L12-L17

to

https://github.com/openhab/openhab-addons/blob/e84e2800a46b3de98d59777383e6b634cb896af5/bundles/org.openhab.binding.novafinedust/src/main/resources/OH-INF/thing/thing-types.xml#L17-L21

is this line:

<limitToOptions>false</limitToOptions>

If we do not change the validation of config options that have

<context>serial-port</context>

set, I fear that other bindings are affected as well. For the novafinedust I will provide a fix (more a workaround) now. But I think this should be fixed her ein the core, so we allow the rfc2217 syntax for serial ports too.

Edit: For reference a .things file

smartmeter:meter:energyhome "Home"       [ port="rfc2217://127.0.0.1:2000", mode="D", baudrate="9600" ]

[wborn]

I will provide a fix (more a workaround) now. But I think this should be fixed her ein the core, so we allow the rfc2217 syntax for serial ports too.

<limitToOptions>false</limitToOptions>

This is not a workaround. It should always be added when configuring serial ports as not all ports can be discovered when using custom named ports with udev rules, see https://github.com/openhab/openhab-addons/pull/7625

[J-N-K]

I was about to write the same. The documentation is quite clear: it is necessary to set <limitToOptions>false</limitToOptions> if you define options and want to allow other values.

[t2000]

Well the thing is that I did not explicitly give any options! See https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.novafinedust/src/main/resources/OH-INF/thing/thing-types.xml#L17-22

Maybe these option are provided implicitly via the serial-port context, but this is not transparent to a developer!

Hence I would expect the default: limit-to-options=false for such a context!

[wborn]

It might be an idea to add a serial-port context example to the examples in the docs to make the serial-port context magic more clear.

According to the docs if and how the context is used depends on each UI:

Context is used to provide some semantic details about the parameter. The UIs use it to render different kind of input widgets.

AFAIK in the backend the context is only used for providing options using the ConfigOptionProvider interface. So adding a context to a config parameter is not used for any other config parameter attributes like limit-to-options (or label/description etc.)

[t2000]

Adding a description tot he docs certainly doesn't hurt.

But the context isn't only used by UIs. This is simply not true anymore... I am on my mobile now, but in the last weeks there were several commits about a component in the core which validates Thing configuration parameters!

And that is what I am talking about. Independent of any UI, certain values will be simply not allowed to be set.

[cweitkamp]

I like the idea to add more meaning to the context for configuration parameters. We can add e.g. default patterns for network-addresses or mac-addresses and let the validator check for violations. As already quoted above from the docs it provides semantic details. This must not only be used by UIs or other consumers.

[lolodomo]

So we must check all existing bindings and add limitToOptions false for any parameter having the serial-port context?

[J-N-K]

Yes or implement what @cweitkamp suggested.

[lolodomo]

It looks like @wborn already did it. https://github.com/openhab/openhab-addons/pull/7625/files So we just have to check again for recent added bindings.

[J-N-K]

Since we agree that limitToOptions=false is the correct way to handle this, I'm closing here.