Sensor time_date platform: rename display_options configuration variable

[akasma74]

Context

Currently the platform has only one configuration variable - display_options.
Its name implies it changes what's displayed, but in fact it lists the actual sensors to be created in the state machine. As I understand based on this conversation, configuration has nothing to do with how it's displayed so the name definitely needs to be changed.

For example, currently to have sensor sensor.time one need to configure it like this:

sensor:
  - platform: time_date
    display_options:
      - 'time'

and there will be no othere sensors like sensor.date_time, sensor.date etc despite having that time_date entry in configuration.

It is pretty confusing, mainly because of a) the name of the configuration variable and b) indirect link between the platform's name and the sensors being created.

Proposal

There are at least 2 options to get it right.

1. Rename display_options to sensors so everything remains almost the same:

sensor:
    platform: time_date
    sensors:
       - date
       - time_date

instead of

sensor:
    platform: time_date
    display_options:
       - date
       - time_date

and the requested sensors are actually strings in a list.

2. Change it the way template platform works, i.e

sensor:
    platform: time_date
    sensors:
       date:
       time_date:

instead of

sensor:
    platform: time_date
    display_options:
       - date
       - time_date

Consequences

For the users it is a breaking change, but the fix is easy whatever option is choosen.
The benefit is it's more clear in terms of what's going on.
Option 2 it allows for further configuration of individual sensors (like format etc).

In terms of how to make this change the option 1 should be pretty easy and straightforward, just changing display_option to sensors everywhere, when the option 2 involves much more internal changes to the platform (but might bring more benefits in the long run).

[fabaff]

Currently the platform has only one configuration variable - display_options. Its name implies it changes what's displayed, but in fact it lists the actual sensors to be created in the state machine.

Doesn't it? date shows the date, beat shows the beat...It doesn't imply that something is changed. As there were no complains since the publishing I assume that it's clear with the docs how it's done and where the limitations are.

and there will be no othere sensors like sensor.date_time, sensor.date etc despite having that time_date entry in configuration.

Of course will there be no other sensors than the ones that are configured.

It is pretty confusing, mainly because of a) the name of the configuration variable and b) indirect link between the platform's name and the sensors being created.

It's a direct link from the configuration option to the sensor name and is there on purpose. It's easier to find the sensor in the overview. As it's a sensor, we need to create a sensor.

I disagree about the renaming of display_options. Changing this is only a cosmetical change with no real value than breaking the configuration.

We can discus the re-write of this integration after 4 years or so as a lot has changed and some design decisions from the past don't make sense anymore but this should go in the direction of support for config flow and having one sensor for all possible output and alike.

[akasma74]

Currently the platform has only one configuration variable - display_options. Its name implies it changes what's displayed, but in fact it lists the actual sensors to be created in the state machine.

Doesn't it? date shows the date, beat shows the beat...It doesn't imply that something is changed. As there were no complains since the publishing I assume that it's clear with the docs how it's done and where the limitations are.

In Lovelace it shows nothing unless you add it to a view somehow.
No complaints? That's the worst excuse I'm afraid.

and there will be no othere sensors like sensor.date_time, sensor.date etc despite having that time_date entry in configuration.

Of course will there be no other sensors than the ones that are configured.

It is pretty confusing, mainly because of a) the name of the configuration variable and b) indirect link between the platform's name and the sensors being created.

It's a direct link from the configuration option to the sensor name and is there on purpose. It's easier to find the sensor in the overview. As it's a sensor, we need to create a sensor.

so you're saying that it's absolutely clear what's created here?

sensor:
    platform: time_date
    display_options:
       - date
       - time_date

clearer that this?

sensor:
    platform: time_date
    sensors:
       - date
       - time_date

I disagree about the renaming of display_options. Changing this is only a cosmetical change with no real value than breaking the configuration.

As I described, it's confusing because has nothing to do with displaying sensors.
If you think users' feedback is not important and can be ignored, I'm really sorry.

[fabaff]

In Lovelace it shows nothing unless you add it to a view somehow.

That's how the handling of the sensors is done.

No complaints? That's the worst excuse I'm afraid.

It's not an excuse but a fact. It seems that it is not that unclear as you claim.

so you're saying that it's absolutely clear what's created here?

If it's not clear then the documentation should be updated. But saying three times the same thing instead of two times will not actually help.

If you think users' feedback is not important and can be ignored, I'm really sorry.

It's import and I outlined already a way to improve the integration.

[akasma74]

If it's not clear then the documentation should be updated.

I'm here because of that. Is it Catch 22?

[akasma74]

I'm really sorry to see how serious HA devs take issues.