Expose method to set last activated on scene

[thomasddn]

Proposed change

Goal

This PR adds a method implementors of integrations can use to notify HA that a scene was (externally) activated. See the approved architectural change request: https://github.com/home-assistant/architecture/discussions/1236 for more information.

Problem

I came across the following issue.

Scene class:

• There is a final method _async_activate which sets __last_activated and then calls async_activate.
• The async_activate is meant to be overwritten by integrations

I've moved setting __last_activated and writing the state to a separate method _set_activated so integrations could call it when the scene was activated externally. However, activating the scene within HA would call _set_activated twice: once due to being called from _async_activate and once when the integration reports back that the scene has been activated.

Solution

I considered letting integrations override _async_activate instead of async_activate when they want to report externally activated scenes. But I find the difference between those two method names visually too subtle, possibly leading to unwanted errors. It would also be not that visible in the code that you are then responsible for setting the last activated field.

So I've taken the approach like in the coordinator. An optional field _activate_method that can hold a reference to the method you want to use for activating the scene. If the field has been assigned, that will be used instead of the default _async_activate. This makes developers very aware that they are doing something different and are responsible for making it work. In short: if they want to report externally activated scenes, they must set the field and call _set_activated when appropriate.

Because developers are likely to name their activate method _async_activate, I've renamed the original one to _async_internal_activate, so they would not override the method accidentally.

Another approach would be to add a boolean field indicating if the integration will be responsible for calling _set_activated. Then that boolean can be checked in _activate_method to see if the base class should call _set_activated or not.

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [ ] Breaking change (fix/feature causing existing functionality to break)
• [ ] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue: https://github.com/home-assistant/architecture/discussions/1236
• Link to documentation pull request:
• Link to developer documentation pull request:
• Link to frontend pull request:

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] I have followed the perfect PR recommendations
• [x] The code has been formatted using Ruff (ruff format homeassistant tests)
• [x] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• [ ] Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• [ ] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [ ] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (scene) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of scene can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign scene Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[thomasddn]

@abmantis done

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[abmantis]

I think this is fine, but would like a second pair of eyes since it changes the core scene entity.

[thomasddn]

@frenck The problem is that, with just providing an extra method, the time will be set twice if the scene is activated by HA. As a result any automation that might be attached to it, will fire twice.

In the description of this PR I've outlined how I came to this solution, but I'm open to suggestions for other solutions than what is currently implemented.

[frenck]

I get what you are saying... however, this should be part of the architectural proposal and not a discussion or question in the pull request.

../Frenck

[thomasddn]

@frenck Should I then create a new proposal, or what is the way forward here?

[abmantis]

Hey @thomasddn ! We've discussed this in the core meeting and have agreed on the following proposal:

• Break Scene class into two classes: Scene + SceneBase, where Scene extends from SceneBase.
• Scene keeps the the automatic last_activated.
• Add _async_record_activation(), in SceneBase which should only set the timestamp (without async_write_ha_state)
• Add _record_activation() for sync versions, which should use run_callback_threadsafe() to call the the async one.

[abmantis]

LGTM! Can you update the dev docs too, please?

[MartinHjelmare]

Please update the PR description with the current solution.

[abmantis]

Thanks @thomasddn !