Considering a default pattern for string and number channels

[lolodomo]

All the string channels provided by the Sonos binding are defined without any pattern. As the result, the channel value is not displayed in UI if you use the auto-link mode of Paper UI and you forgot to define a label with pattern "%s" in your sitemap.

I propose to add a default pattern "%s" to these string channels.
@kgoderis @kaikreuzer : are you ok with that proposal ?

[lolodomo]

Any feedback ?

[kgoderis]

Ok for me

[sjsf]

makes sense

[lolodomo]

I have done the changes for the Sonos binding but in fact almost all bindings should be fixed !.

[lolodomo]

Finally I think the good approach was my PR #3262 that would define a default pattern whatever the channel (all bindings). I just have to adjust the test to be sure to add a default pattern only when expected.

[lolodomo]

To be clear, here is my general proposal that will apply to all bindings and all UIs.
When no pattern is defined in label from sitemap, no pattern is defined in label from tem and no pattern is defined in channel, if the channel is associated to a string or number item, the UI item registry uses a default pattern to return the item value in the label.
For string item, the default pattern would be naturally "%s". For number item, I propose "%.0f".

Is it ok for you ?

I ping @kaikreuzer as he was worried when I first tried to fix that.

[sjsf]

Hmmm, I'm a little undecided....

Pros:

• some bindings define patterns, some don't. This proposal just fosters consistency from a user's perspective
• for automatically created items this certainly makes sense (as you pointed out in Avoid having no value displayed in UIs for auto-created string items (fix #3248) #3262)
• To my personal experience this is the by far more common use-case than not displaying a state

Cons:

• for hand-crafted *.items files it feels wrong to mess with whatever the user defined
• it turns the current default behavior upside-down. So people not wanting to display a state at a place will be disappointed

So I'm pretty confident this sounds like a good idea to do in the ChannelItemProvider. This would also mean it does not mess with user's configurations and will apply only to items which are linked to channels. And as @kaikreuzer already pointed out: "[...] that there are many valid use cases where we do not want the state to be provided/displayed; we should make sure that this is not broken".

I'm not so sure though about the implications of doing it for all such items in the UI layer. For this, I don't have a complete enough picture on how people are using them currently, which use-cases will be broken and how common they are.

[lolodomo]

As a reminder, it is possible to use "xxx []" as label in an item or sitemap file to not display the item value.

[kaikreuzer]

I agree with @lolodomo that #3800 seems to be the wrong approach - it just adds "the obvious" and requires all binding authors to do so.
#3262 also does not look right as it will only affect UIs that use sitemaps (btw, does the Paper UI actually already show the states? If so, why? Does it have this logic already implicitly in its own code?)
My concern from #3262 with the example of group items is a bit relaxed by now as we now HAVE groups without states - so in the example, we would actually still see no state, even if we defined a default pattern.

this sounds like a good idea to do in the ChannelItemProvider

Don't you rather mean the ChannelStateDescriptionProvider?

for hand-crafted *.items files it feels wrong to mess with whatever the user defined

Maybe this would be the right moment to break the item file format and to introduce a separate "pattern" field (instead of encoding it within the label). We could then offer a item-file migration that would set the pattern to "", if no square brackets are in the label - this would make it take precedence over the value from ChannelStateDescriptionProvider. Items where "pattern" is null would instead use the default from the channel.

[lolodomo]

If I correctly understand you, you are not adressing the original problem that is items created by Paper UI without pattern and linked to channel without default pattern. In this case, we would like a default display in particular for string and number channels.

Regarding your proposal of change in DSL, we just have to keep a way to disable the display of the state, being possible now by using [].

[kaikreuzer]

you are not adressing the original problem

Why not? The ChannelStateDescriptionProvider would provide the pattern for them, which would then be used, right?

we just have to keep a way to disable the display of the state, being possible now by using [].

See @SJKA's concern (which I share) that this would mean that all existing item files will suddenly behave differently - where "My Label" so far did NOT show any state, users would now manually have to change it to "My Label []" in order to keep the status quo.

[lolodomo]

If ChannelStateDescriptionProvider would provide the missing pattern, that's ok for me.

And I agree that "My Label" in item file should show the state using the pattern defined in the channel or the default pattern we will add if not defined in the channel. To disable the display of this value, the user could use "My Label []".
My remark was about your idea to break DSL to have two different fields, label and pattern. In this case, I have no idea how the user could ask "no display of state" ?

[kaikreuzer]

By setting pattern="" (which the migration-tool would automatically add).

[lolodomo]

Ok @kaikreuzer, that should work. I am not sure that changing the DSL and implementing a kind of migration worth the effort. All users using DSL are more or less advanced users who know how to use the label.

By updating ChannelStateDescriptionProvider rather than ChannelItemProvider, my understanding is that we are not changing the creation of items by Paper UI. Using ChannelStateDescriptionProvider has the advantage that it would work even for items already created by Paper UI.
@SJKA : let me know if you want to work on this enhancement in ChannelStateDescriptionProvider. If not, I can do it. I think the code change is easy.

[lolodomo]

But I agree that it would have been better to separate label and pattern for state since the beginning.
The question is just: isn't it too late to do that change ? What effort to implement the change ? We will have to communicate about a migration tool ??

[lolodomo]

Could the issue #2729 not be a problem for the change we imagined ? As it was explained we can have several StateDescriptionProvider and we don't know which one is returned.

[lolodomo]

PR submitted for your review.