Change cutoff point of sun conditions to astronomical midnight

[emontnemery]

Background:

The cutoff point for the sun conditions is midnight in local time. This works OK in most cases, as long as the sun sets before midnight in local time. In places with very skewed time zones, the sun conditions however fail.

Note: There is another issue where sun up/down events from the wrong date are used when evaluating the sun conditions home-assistant/home-assistant#23664.

Normal case

In this case, the sun sets before midnight in local time, and all is fine.

The problem

If the time local time is very skewed compared to astronomical time, it breaks down: The example is similar to the summer time in Kotzebue, Alaska, which has an extremely skewed local time zone. In this case, "after sunset" is true from 03:00 until 23:59, which is not when the sun is down, and arguably illogical.

Proposed change

Change cutoff point for the sun conditions to be astronomical midnight instead of midnight in local time:

[OttoWinter]

The edge-case is indeed un-intuitive.

However, I think changing it to be astronomical midnight would be un-intuitive to all 99% of other users. I don't think this will be easy to explain in the docs and the normal user will struggle to understand what is meant. This more or less boils down to what a user expects when you say "before sunrise" - at least to me the existing logic makes more sense.

Disclaimer: I don't really use the sun condition myself (mostly use elevation < 0 check). Some real-world usages of this would be interesting.

I mean, there's another interesting edge-case too: What happens when there is no sunset/sunrise? I imagine those locations with a skewed local time zone also have to deal with the "no sunset/rise" edge case - so why not have them deal with this other edge case with some templates too?

[emontnemery]

I think changing it to be astronomical midnight would be un-intuitive to all 99% of other users. I don't think this will be easy to explain in the docs and the normal user will struggle to understand what is meant.

Yes, this is indeed a tough one! But for the 99% of the users, the local time zone is also reasonably well aligned with astronomical time, and I think it shouldn't matter if the sun conditions cutoff point is at 23.30, 00.00 or 00.00 local time. Changing the cutoff point to astronomical midnight, also mean the conditions "work" regardless of the time zone used by home assistant.

What happens when there is no sunset/sunrise? I imagine those locations with a skewed local time zone also have to deal with the "no sunset/rise" edge case.

You're right! If there is no sunset or sunrise, the sun conditions currently evaluate to "False", meaning any automations will anyway fail to work reliably during polar day or polar night. And as you say, the locations with a local time zone skewed enough to have sunset before sunrise local time, are very likely to experience polar day or polar night.

Maybe the right thing to do here is to not mess with the sun conditions, but instead add a warning here: https://www.home-assistant.io/docs/scripts/conditions/#sun-condition, dis-encouraging from using the sun-conditions and instead suggest to check for elevations?

[OttoWinter]

I think it shouldn't matter if the sun conditions cutoff point is at 23.30, 00.00 or 00.00 local time.

Depends on the way it is used I think. Some examples of this being used in the wild would help - for me the only real thing that makes sense is checking if the sun is below or above the horizon.

[cgtobi]

Would an optional offset be a acceptable solution? So, that the user can offset the cutoff point by a reasonable amount? Or make that shift of the cutoff point to the astronomical midnight an option which defaults to false. That way the status quo would be conserved but for such corner cases it can be adjusted.

[emontnemery]

for me the only real thing that makes sense is checking if the sun is below or above the horizon.

@OttoWinter Yeah, i think you're right. Hence, maybe the right thing to do is to simply update the docs to warn against using the sun conditions.

Would an optional offset be a acceptable solution? So, that the user can offset the cutoff point by a reasonable amount?

@cgtobi It means we expect the user to check their local time offset versus astronomical time themselves? Note that the offset changes with daylight saving time. If changing the cutoff point to astronomical midnight is too complicated to explain, making it user configurable must be even worse..

[emontnemery]

Some proposed documentation updates here home-assistant/home-assistant.io#9423, warning against using the sun conditons.

[OttoWinter]

I mean I would just add a new condition called sun.below_horizon and sun.above_horizon (maybe even with an angle property) - checking elevation attribute is fine but why not make this common task easier on users? That will certainly help with avoiding the current conditions.

[emontnemery]

@OttoWinter You don't think this is good enough?

condition:
    condition: state  # from sunset until sunrise
    entity_id: sun.sun
    state: 'below_horizon'

[Swamp-Ig]

@ottowinter There's a template sensor documented in the docs that does this already

# Example configuration.yaml entry
binary_sensor:
  - platform: template
    sensors:
      sun_up:
        friendly_name: "Sun is up"
        value_template: >-
          {{ states.sun.sun.attributes.elevation|float > 0 }}

[emontnemery]

@Swamp-Ig I think @OttoWinter's concern was this is not user friendly enough:

condition:
    condition: state
    entity_id: sun.sun
    state: 'below_horizon' # from sunset until sunrise

And it would be better to be able to do:

condition:
    condition: sun
    elevation: 'below_horizon' # from sunset until sunrise

[OttoWinter]

@emontnemery Ah yes checking via state is possible too of course, didn't think of that.

Yeah my concern was that users would not use the elevation > 0 option because it involved magic templates that they might not understand. But via checking state with state condition is fine.

That being said, I would fully support adding more conditions throughout the code base. Best would be if components everywhere could register a new condition like with other registries.

[emontnemery]

I would fully support adding more conditions throughout the code base

@OttoWinter There was a previous attempt here, #98, which was dropped in favor of improving the docs instead of adding new conditions..

Best would be if components everywhere could register a new condition like with other registries.

Yes, that would be super nice, and is suggested here: home-assistant/architecture#178 I will probably have time to have a go at that in a couple of weeks, but my idea for an MVP is MQTT triggers for use with buttons/remotes, not sun conditions :)

[Swamp-Ig]

I would fully support adding more conditions throughout the code base

The main ones really are sensor.solar_elevation, sensor.solar_rising, and sensor.solar_azimuth.

There's a whole heap you could have, many of them derived from these three. If one was do "do it properly" you'd allow the sun device to be configured to include whatever you wanted, much like some weather components do.

Here's a pretty exhaustive list of everything I can think of:

sun:
  monitored_conditions:
    # boolean_sensors
    - solar_day # sun is up
    - solar_rising # after solar midnight, before noon
    - solar_twilight # civil twilight
    - solar_nautical_twilight
    - solar_astronomical_twilight
    # sensors expressing degrees
    - solar_elevation
    - solar_azimuth
    - solar_zenith_angle
    - solar_right_ascension
    - solar_declination
    # enumerated state morning/afternoon/dusk/evening/early_morning/dawn
    # where dusk and dawn are civil twilight, 
    # evening is before solar midnight, and early_morning is after it
    - solar_phase
    # for the next event in degrees
    - solar_next_zenith_elevation
    - solar_next_nadir_elevation
    # sensors expressing timestamps of the next event
    - solar_next_zenith
    - solar_next_nadir
    - solar_next_sunrise
    - solar_next_sunset
    - solar_next_dusk_end # end of civil twilght
    - solar_next_dawn_begin # start of civil twilight
    - solar_next_nautical_dusk_end
    - solar_next_nautical_dawn_begin
    - solar_next_astronomical_dusk_end
    - solar_next_astronomical_dawn_begin
    # you could even have all the next_ events in last_ form too.

I am not of course suggesting that all of these are available, but it does become a question of how far down this particular rabbit hole does one want to go?

[OttoWinter]

@Swamp-Ig The thing I was talking about (and what I believe @emontnemery was referring to too) was automation conditions. For example the existing sun condition.

This is not about creating new sensors. Another positive aspect of adding new sun conditions would be that it wouldn't rely on state machine - quite recently I've seen some people complain about huge DBs because the sun entity is generating tons of events.

By creating dedicated sun conditions/triggers for this stuff we wouldn't need to work through the state machine (and thus reduce the sun entity update interval).

[Swamp-Ig]

Either way really :grin:, it's the same set.

Quite recently I've seen some people complain about huge DBs because the sun entity is generating tons of events.

Got a PR going in tomorrow to fix that. Currently it's an event every 30 seconds, which is 2,880 events. After tomorrow it will be 360 + 40 +6 = 406, which is heaps better.

If you split it out into sensors, then the elevation sensor would still update 360 + 40 times per day (once for each degree of elevation, plus every half degree between -10 and +10). So not really a disaster. For sure if you turned that into a condition then there's less, but IDK if 400 is really going to break the bank that much.

The upside is that you don't have to have a special-case condition, it just becomes a state change condition like basically everything else. Plus if you want to implement solar_right_assention it's not a big deal.

[Swamp-Ig]

Here's a link to the PR that at least turns down the sun.sun device's events: home-assistant/home-assistant#23832

[emontnemery]

So, it seems the conclusion is to:

• Not mess with the sun conditions
• Update documentation
  • Explain the limitations of sun conditions
  • Add some examples of ways other than sun conditions to do automation conditions related to the sun
  • Both done in home-assistant/home-assistant.io#9423

@armills Any opinion?

[frenck]

This architecture issue is old, stale, and possibly obsolete. Things changed a lot over the years. Additionally, we have been moving to discussions for these architectural discussions.

For that reason, I'm going to close this issue.

../Frenck