Separate weekday condition from time condition

[Misiu]

Breaking change

While working on adding support for weekday condition in automation editor I got a suggestion from @bramkragten to split time and weekday condition into two separate conditions.
I've created this issue in architecture repo, this PR is its consequence.

Currently, both conditions will work, in 0.109 the weekday property of time will be removed.

Proposed change

Instead:

condition:
  condition: time
  after: '15:00:00'
  before: '02:00:00'
  weekday:
    - mon
    - wed
    - fri

users will write:

condition:
  - condition: time
    after: '15:00:00'
    before: '02:00:00'
  - condition: weekday
    days:
      - mon
      - wed
      - fri

Type of change

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

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

• This PR fixes or closes issue: fixes Split weekday from time condition into a separate condition architecture#356
• This PR is related to issue: Automations condition.weekday missing in Automation GUI frontend#3833
• Link to documentation pull request: will add after acceptance

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• 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.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

• No score or internal
• 🥈 Silver
• 🥇 Gold
• 🏆 Platinum

[Misiu]

for this line I get this error:

homeassistant/helpers/config_validation.py:896:63: E231 missing whitespace after ','

If I add space then black is throwing the error, if I remove it then flake8 throws the error.
Very confusing.

[javicalle]

If I add space then black is throwing the error, if I remove it then flake8 throws the error.

Maybe with a breakline?

WEEKDAY_CONDITION_SCHEMA = vol.Schema(
    {
        vol.Required(CONF_CONDITION): "weekday",
        "days": weekdays,
    }
)

[Misiu]

@javicalle not I get errors from black :/

WEEKDAY_CONDITION_SCHEMA = vol.Schema(
-    {
-        vol.Required(CONF_CONDITION): "weekday",
-        "days": weekdays,
-    }
+    {vol.Required(CONF_CONDITION): "weekday", "days": weekdays,}
 )

Any other ideas on how to fix this? Maybe it is some kind of bug? I can't fulfill two conditions at once.

[javicalle]

Sorry, I have no clue.

[Misiu]

Sorry, I have no clue.

me neither :)

[Misiu]

@javicalle I had to remove the trailing comma as suggested here: psf/black#1288
All green now.

[stale]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

[Misiu]

This isn't stale, it just needs some attention.

[MartinHjelmare]

We won't review this until the architecture issue is approved.

[Misiu]

@MartinHjelmare I'm aware that there are big changes going on in HA and this is a tiny change.
How can I draw attention to architecture issue? Can I ask for your opinion? I don't want to mention anyone there because I don't want to get banned.

[stale]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.