Add sensor platform to Sun

[gjohansson-ST]

Breaking change

All attributes for the sun.sun entity has been removed and replaced by sensor entities. Please update all automations, templates etc. that uses these attributes into their respective sensor entity.

Proposed change

Adds sensor platform to sun integration

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)
• [x] 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:
• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/24704

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] The code has been formatted using Black (black --fast homeassistant tests)
• [x] Tests have been added to verify that the new code works.

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

• [x] 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.
• [ ] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

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

[home-assistant[bot]]

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

Code owner commands

Code owners of sun can trigger bot actions by commenting:

• @home-assistant close Closes the issue.
• @home-assistant rename Awesome new title Change the title of the issue.
• @home-assistant unassign sun Removes the current integration label and assignees on the issue, add the integration domain after the command.

[disforw]

+breaking change

[gjohansson-ST]

+breaking change

~~Yes. Apparently missed to tick the box. Now done but someone needs to add the label~~ Not any longer

[KevinCathcart]

Should this be an immediate breaking change, or merely adding the new sensors, while deprecating the attributes to be removed later? (I have no strong opinions). For the rest of this message I'll assume it should be an immediate breaking change.

With this, you should remove the recorder platform, as there would be no attributes to exclude. Honestly I would like to see the position sensors not stored in recorder by default, since those change rather frequently, and not a lot of people actually use them. I don't think there is a way for a sensor to request not being included in the recorder, but having those two sensors be disabled by default would cover it, and those few people who need them, can enable them and either filter them from recorder, or not. The key thing here is that sun part of default_config:, so any such frequently updating sensors will be present on everybody's system by default.

That then raises the question of if the other sensors should be disabled by default or not. I have no strong feelings here. My immediate reaction is fewer active entities is better, especially because most of these sensors are somewhat niche, but I certainly acknowledge that others may feel more strongly about having them enabled by default.

[gjohansson-ST]

Now I found this one #77733 I will move the attributes back and then the other PR can be adjusted to move in more sensors. Also a good suggestion to maybe have a few sensors disabled by default

[KevinCathcart]

Long term I do think the attributes should go away, so I would be good with formally deprecating them. I’m not sure it is actually feasible to warn about using the attributes? That might be why hard cutovers are often done in these situations.

I just feel it is generally better to give people time to migrate while letting their existing automations continue to work, at least when doing so is possible and is not a lot of additional effort.

[decompil3d]

Thank you for doing this -- I'm the author of #77733 and was just starting to investigate how to add this platform. Glad you beat me to it. I can definitely just add the additional sensors once this is merged.

Suggestion for this change: can the new sensor entity IDs be prefixed with sun_? For example, sensor.sun_next_rising. Otherwise, they are hard to grok when writing automations, etc.

[gjohansson-ST]

Long term I do think the attributes should go away, so I would be good with formally deprecating them.

Not possible as far as I know.

Suggestion for this change: can the new sensor entity IDs be prefixed

Added

[Rudd-O]

I like this a lot.

[decompil3d]

Hoping this gets reviewed & merged soon 🙂 I'll post a fast-follow PR with the previous sensors and then I have a new card in the works that'll use them.

[Swamp-Ig]

I'm down as code owner of this although I haven't touched it in years. All seems pretty sensible on the face of it, but there's possibly an architectural decision here that needs to be made: do we want to move away from sun.sun?

Probably the main issue really is that there's already issues around recording excess garbage in the recorder, this makes it twice as bad. Realistically sun.sun won't go away in any kind of hurry.

Would a series of template sensors be a better option for people who want to use this, rather than changing the core?

[Swamp-Ig]

OK looks like the architectural decision has been made

[Swamp-Ig]

@gjohansson-ST is this good to go now? It's all approved, can merge if you are ready.

[gjohansson-ST]

@gjohansson-ST is this good to go now? It's all approved, can merge if you are ready.

@Swamp-Ig Please do. It's complete