Thing configuration "show advanced" doesn't work properly

[Nadahar]

The problem

I'm developing a binding, and while working on the Thing configuration, I could not get advanced to work. All configuration parameters would show regardless of the value of advanced. By using the REST API, I could verify that the configuration descriptions were correctly populated. The only difference I could see was that the Show advanced checkbox would appear, but checking/unchecking it would make no difference.

Then, after more or less giving up on getting this to work, I moved on and started to set default values in the configuration descriptions. To my surprise, the parameters with default values would "disappear". After scratching my head a bit, I suddenly realized that advanced only works for configuration parameters that have a default value. They do indeed "reappear" when checking Show advanced. So, it seems to me like the advanced and default properties are somehow linked when they should not.

Expected behavior

I expect the advanced and default properties to work independently in Main UI.

Steps to reproduce

1. Make a Think configuration with an advanced parameter that has no default value.

[Nadahar]

I just noticed that as soon as one of the "advanced" parameters get a non-default value, they are no longer hidden. This means that this might not be a bug, but intended behavior. I know that the documentation says that this UI might hide advanced parameters, but I've spent quite a lot of time "not getting this to work", and I really think it should, at least, be better documented.

I'd also say that it might be better if the "differ from default" logic only applied to parameters with a default. It's not always that this logic makes sense, in my case for example, there's no fixed default as the parameters are retrieved from the devices, and different versions/models might have different factory settings. They are still very much advanced and should not be changed in most cases.

[florian-h05]

The UI does only apply this logic if there is a default value defined for this parameter, see

openhab-webui/bundles/org.openhab.ui/web/src/components/config/config-sheet.vue

Line 115 in ecb2cae

(p.default !== undefined && this.configuration[p.name] !== undefined && this.configuration[p.name] !== null ? this.configuration[p.name].toString() !== p.default : this.configuration[p.name] !== undefined))

Can you please share the config descriptions provided by the REST API?
And also share the Thing config as returned by the REST API.

[florian-h05]

#2888 might improve the behaviour by additionaly checking for not null and not empty.

[Nadahar]

Here are some config descriptions:

    "uri": "thing:milllan:panel-heater:e7c586766b",
    "parameters": [
      {
        "description": "The time zone offset from UTC in minutes.",
        "label": "Time Zone Offset",
        "name": "timeZoneOffset",
        "required": false,
        "type": "INTEGER",
        "min": -840,
        "max": 720,
        "stepsize": 15,
        "readOnly": false,
        "multiple": false,
        "groupName": "general",
        "advanced": true,
        "verify": false,
        "limitToOptions": true,
        "unit": "min",
        "options": [],
        "filterCriteria": []
      },
      {
        "description": "The PID controller's proportional gain factor K<sub>p</sub>.",
        "label": "Kp",
        "name": "pidKp",
        "required": false,
        "type": "DECIMAL",
        "min": 0,
        "stepsize": 1,
        "readOnly": false,
        "multiple": false,
        "groupName": "pid",
        "advanced": true,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },
      {
        "description": "The PID controller's integral gain factor K<sub>i</sub>.",
        "label": "Ki",
        "name": "pidKi",
        "required": false,
        "type": "DECIMAL",
        "min": 0,
        "stepsize": 0.01,
        "readOnly": false,
        "multiple": false,
        "groupName": "pid",
        "advanced": true,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },
      {
        "description": "The PID controller's derivative gain factor K<sub>d</sub>.",
        "label": "Kd",
        "name": "pidKd",
        "required": false,
        "type": "DECIMAL",
        "min": 0,
        "stepsize": 1,
        "readOnly": false,
        "multiple": false,
        "groupName": "pid",
        "advanced": true,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },

...and the corresponding configuration:

    "configuration": {
      "timeZoneOffset": 60,
      "pidKp": 70,
      "pidKi": 0.02,
      "pidKd": 4500
    },

There's a lot of configuration there, so I've only posted a few of them for better overview.

[Nadahar]

When looking at the code you linked, these quality for this:
no default value is defined and the param has a value

They all have values, which are fetched from the device and populated by the binding.

This logic effectively defines "no value" as the default if none is defined. It might make sense in some scenarios, it definitely doesn't in this case.

[florian-h05]

This logic doesn’t not effectively define the default, it shows a value that YOUR binding provides.
The idea behind this logic was to avoid common confusion caused when something did not behave as default due to advanced parameters.

[Nadahar]

I get the behavior when a default is defined. I'm just not so sure that it's a good thing that it's now "impossible" to have an advanced parameter with a value that isn't shown, even when "no known default" exists.

[florian-h05]

I understand your argument.
On the other side showing a advanced value without a default if it has a value is also helpful for UI widgets eg, but this behaviour there could also be created by explicitly defining an empty default in the widget definition.

I think hence think we can change this behaviour to only apply to params which have a default defined. Do you agree (it’s what you’ve asked for)?

[Nadahar]

I think hence think we can change this behaviour to only apply to params which have a default defined. Do you agree (it’s what you’ve asked for)?

I absolutely agree, I think that would be the best all in all - but I am of course limited in that I don't know all situations that this change might impact.

[florian-h05]

I have though a bit about this and I think it is fine to change that.

[Nadahar]

Thanks 👍