Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reload of input_text doesn't reset to initial #36536

Closed
kloggy opened this issue Jun 7, 2020 · 16 comments
Closed

Reload of input_text doesn't reset to initial #36536

kloggy opened this issue Jun 7, 2020 · 16 comments

Comments

@kloggy
Copy link

kloggy commented Jun 7, 2020

The problem

Pretty much as the title says.

The ‘reload’ service should be just that and completely reload input texts as though HA was restarting including the initial value.

At the very least it should be an option.

For that matter, this applies to any input_*

Environment

  • Home Assistant Core release with the issue: 110.3
  • Last working Home Assistant Core release (if known): n/a
  • Operating environment (Home Assistant/Supervised/Docker/venv): Home Assistant Supervised
  • Integration causing this issue: input_text
  • Link to integration documentation on our website: https://www.home-assistant.io/integrations/input_text/

Problem-relevant configuration.yaml

`service: input_text.reload`

Traceback/Error logs

Additional information

@probot-home-assistant
Copy link

input_text documentation
input_text source
(message by IssueLinks)

@probot-home-assistant
Copy link

Hey there @home-assistant/core, mind taking a look at this issue as its been labeled with a integration (input_text) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@spacegaier
Copy link
Member

Currently, all input_* components use skip_reset=True which means the entity gets not destroyed and recreated and therefore is not receiving the initial value most likely.

async def reload_service_handler(service_call: ServiceCallType) -> None:
"""Reload yaml entities."""
conf = await component.async_prepare_reload(skip_reset=True)
if conf is None:
conf = {DOMAIN: {}}
await yaml_collection.async_load(
[{CONF_ID: id_, **(cfg or {})} for id_, cfg in conf.get(DOMAIN, {}).items()]
)

@kloggy
Copy link
Author

kloggy commented Jun 16, 2020

I don't know Python but it seems in that case that it would be trivial to either correct this (as I consider it a bug) or allow an option to not reset. Which could be even better? (No breaking change ;-) )

@Adminiuga
Copy link
Contributor

The intention of the reload service was so that you could edit the configuration.yaml, add new inputs, call the reload service and have new inputs show up in HA without affecting the existing one. It wasn't intended to emulate the full reset of HA.

@spacegaier
Copy link
Member

I just tested it in my local dev instance and changing the skip_reset does not solve the issue. It recreates all the input_text entities, but they keep their state.

Independent of the actual technical solution: Simply changing the behavior could affect users that are already relying on it only reloading the configuration, but not resetting the initial value.

My personal feeling is, that it might be best to keep "reload" as it is and instead add a second service "reset" or "reinitialize" that goes back to the initial state.

@kloggy
Copy link
Author

kloggy commented Jun 16, 2020

Which is why an option that defaults to the current behaviour could work?

Either way I don't have a strong view on the solution but I do think it should be possible to have the 'initial' value (in all input_*) to be reset.

It's too late now but I suggest 'Reload' is a poorly named service if it was designed to "add new inputs" [my emphasis] and if "It wasn't intended to emulate the full reset of HA".

@spacegaier
Copy link
Member

It's too late now but I suggest 'Reload' is a poorly named service if it was designed to "add new inputs" [my emphasis] and if "It wasn't intended to emulate the full reset of HA".

It is not only for "adding new inputs" (since those can actually be added directly in the HA frontend without any YAML and then you don't need the reload for that) but re-loading the whole configuration for the inputs (e.g. min/max lengths).

I think you are actually contradicting yourself a bit since you write that it wasn't intended to emulate a full reset which is true, hence the service name "reload" and not "reset". So we could implement a new service "reset" that sets the state to the initial value.

@balloob
Copy link
Member

balloob commented Jun 16, 2020

Initial value is the initial value that it will get at startup of Home Assistant. It is not the value the entity is set to when it is initialized AND when we reload configuration.

@balloob balloob closed this as completed Jun 16, 2020
@spacegaier
Copy link
Member

@balloob But if it is not meant to be reset, what is then the purpose of this initial parameter?

Since it keeps its state between HA restarts via the core.restore_state, this value will only ever be relevant once: During the very first input entity creation. If that is done through the HA frontend directly, then it actually never comes into play, since the frontend is not even providing an input field for it.

@kloggy
Copy link
Author

kloggy commented Jun 16, 2020

@spacegaier I'm slightly confused that you think I was contradicting myself. I was quoting you.

I did say quite early on that an 'option' (which could of course be a new service) to 'reset' would be a solution. But I really think another new service is a bit of a sledge hammer to crack a nut. Reset, Reload, isn't it just becoming a question of semantics now. It is all part of the same thing, surely?

@balloob
Copy link
Member

balloob commented Jun 16, 2020

Input entities that have initial defined will not have their state restored.

@spacegaier
Copy link
Member

@kloggy That quote wasn't actually from me, but doesn't matter, especially now that the proposal has already been rejected.

@balloob Shouldn't then the initial parameter be in the frontend so users can change it or was it omitted by design?

@balloob
Copy link
Member

balloob commented Jun 16, 2020

Reloading does not the same as a restart.

We're reloading the configuration while the entity is already part of Home Assistant. The initial value is only applied when the entity is added to Home Assistant (on startup).

@point-4ward
Copy link
Contributor

I don't have a strong opinion either way (I just clicked the link from the forum question), but this behaviour is not consistent with the reload services that came before it.

Reloading automations resets them to their initial_state: value - thus imo so should reloading anything else, or none should do it.

@kloggy
Copy link
Author

kloggy commented Jun 17, 2020

Ok, so I get, but do not agree with, the reasoning

"We're reloading the configuration while the entity is already part of Home Assistant. The initial value is only applied when the entity is added to Home Assistant (on startup)."

But this just strengthens the view that the Service is at best confusingly named.

Reload: re is shorthand for 'repeat'. i.e. do whatever was done when it was load ed.
Reload should have probably been called 'load' i.e load new input_* because that seems to be what it does.

Obviously this isn't going to change now but we could consider fixing both the inconsistencies (functional and naming) by allowing an option to also reset to initial, defaulting to 'no' to prevent breaking changes.

Except that this has been closed :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants