Jewish calendar binary sensor by tsvi · Pull Request #26200 · home-assistant/core

tsvi · 2019-08-25T16:43:54Z

Breaking Change:

Change in configuration.yaml

sensor:
  - platform: jewish_calendar
    sensors:
       - date
       - ...

To

jewish_calendar:

Automations based on sensor.issur_melacha_in_effect comparison to True/False need to be updated to use binary_sensor.issur_melacha_in_effect on and off states.

Explanation

Move Jewish calendar sensor to its own platform
Split sensors into sensors and binary sensors
Removal of the "sensors" key, following ADR003 removing data selectors, instead show all of the sensors.

The issur_melacha_in_effect was supposed to be a binary sensor. I wasn't aware of the distinction back when I first developed this integration. This fixes this issue by creating a new platform, supporting both types of sensors.

Description:

Make Jewish calendar its own platform with two types of sensors: sensor and binary sensor. In a future PR I plan to split the sensor furthermore in data and time related so that I can provide datetime attributes in the same fashion as the date sensor. This will provide easier ways of writing automations.

Related issue (if applicable): fixes #20315
As part of this change, I implemented @MartinHjelmare suggestion for testing the components,

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10231

Example entry for `configuration.yaml` (if applicable):

jewish_calendar:
  language: hebrew
  diaspora: False

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist

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

Documentation added/updated in home-assistant.io

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

The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

Updates for PR home-assistant/core#26200

tsvi · 2019-09-03T12:29:29Z

Finally got all the tests to pass. @MartinHjelmare any more fixes necessary?

@MartinHjelmare

As part of this, move tests to use async_setup_component instead of testing JewishCalendarSensor as suggested by @MartinHjelmare here: home-assistant#24958 (review)

…_time_zone in tests

Don't use unnecessary alter time in binary sensor Remove unused alter_time

…l values

…entity

This reverts commit 74371740eaeb6e73c1a374725b05207071648ee1.

MartinHjelmare

Looks good!

tsvi · 2019-09-03T20:29:59Z

Fixed the imports as requested

tsvi · 2019-09-03T21:10:31Z

I think we need a breaking change label here.

MartinHjelmare · 2019-09-03T21:12:54Z

Please add to the breaking change paragraph what the user needs to do to cope with the breaking change.

tsvi · 2019-09-06T03:58:54Z

@MartinHjelmare can you merge the PR? Or are we waiting for another review?

MartinHjelmare · 2019-09-06T11:23:31Z

We're all good! Changing the PR description doesn't generate a github notification.

homeassistant added the cla-signed label Aug 25, 2019

probot-home-assistant Bot added has-tests integration: jewish_calendar new-platform labels Aug 25, 2019

tsvi force-pushed the jcal_binary_sensor branch from 8d5352b to 085866e Compare August 26, 2019 09:51

tsvi added a commit to tsvi/home-assistant.github.io that referenced this pull request Aug 26, 2019

Updates docs for Jewish calendar

1d347a6

Updates for PR home-assistant/core#26200

tsvi mentioned this pull request Aug 26, 2019

Updates docs for Jewish calendar home-assistant/home-assistant.io#10231

Merged

2 tasks

tsvi changed the title ~~WIP: Jewish calendar binary sensor~~ Jewish calendar binary sensor Aug 28, 2019

MartinHjelmare requested changes Aug 28, 2019

View reviewed changes

tsvi force-pushed the jcal_binary_sensor branch 2 times, most recently from 6d0af80 to b105c7d Compare August 29, 2019 06:39

MartinHjelmare reviewed Aug 29, 2019

View reviewed changes

Comment thread tests/components/jewish_calendar/test_binary_sensor.py Outdated

Comment thread homeassistant/components/jewish_calendar/sensor.py Outdated

Comment thread tests/components/jewish_calendar/test_sensor.py Outdated

MartinHjelmare reviewed Aug 31, 2019

View reviewed changes

Comment thread tests/components/jewish_calendar/test_binary_sensor.py Outdated

MartinHjelmare reviewed Aug 31, 2019

View reviewed changes

Comment thread tests/components/jewish_calendar/__init__.py Outdated

MartinHjelmare reviewed Sep 3, 2019

View reviewed changes

Comment thread tests/components/jewish_calendar/__init__.py Outdated

MartinHjelmare reviewed Sep 3, 2019

View reviewed changes

Comment thread homeassistant/components/jewish_calendar/binary_sensor.py Outdated

tsvi added 13 commits September 3, 2019 18:32

Move jewish calendar to its own platform

a4433fd

Fix tests for Jewish Calendar platform

ee155a5

As part of this, move tests to use async_setup_component instead of testing JewishCalendarSensor as suggested by @MartinHjelmare here: home-assistant#24958 (review)

Get sensors to update during test

5e1c8dc

Use hass.config.set_time_zone instead of directly calling set_default…

6b9c0af

…_time_zone in tests

Cleanup log messages

3bbf625

Rename result from weekly_portion to parshat_hashavua

65d29c4

Fix english/hebrew tests

a467dbd

Fix updating of issue melacha binary sensor

a766b72

Fix docstrings of binary sensor

d97a599

Reset timezones before and after each test

f2d3bc5

Use correct entity_id for day of the omer tests

9a66823

Fix omer tests

156daab

Cleanup and rearrange tests

42ede7a

tsvi added 12 commits September 3, 2019 18:32

Don't use unnecessary alter_time in sensors

1ccca66

Don't use unnecessary alter time in binary sensor Remove unused alter_time

Simply set hass.config.time_zone instead of murking around with globa…

3960547

…l values

Use async_fire_time_changed instead of directly calling async_update_…

77508c6

…entity

Removing debug messaging during init of integration

7c92dd6

Capitalize constants

a71ed76

Collect all Entities before calling async_add_entities

7329208

Revert "Don't use unnecessary alter_time in sensors"

26fb77f

This reverts commit 74371740eaeb6e73c1a374725b05207071648ee1.

Use test time instead of utc_now

05797b9

Remove superfluous testing

781adb8

Fix triggering of time changed

b0a12a6

Fix failing tests due to side-effects

e39e624

Use dt_util.as_utc instead of reimplementing it's functionality

879e2f6

tsvi force-pushed the jcal_binary_sensor branch from 6886e4e to 879e2f6 Compare September 3, 2019 15:36

tsvi added 2 commits September 3, 2019 19:24

Use dict[key] for default values

611b668

Move 3rd party imports to the top of the module

726914c

MartinHjelmare approved these changes Sep 3, 2019

View reviewed changes

MartinHjelmare reviewed Sep 3, 2019

View reviewed changes

Comment thread homeassistant/components/jewish_calendar/__init__.py Outdated

Comment thread homeassistant/components/jewish_calendar/binary_sensor.py Outdated

Comment thread homeassistant/components/jewish_calendar/sensor.py Outdated

Fix imports

d0ad1c8

MartinHjelmare approved these changes Sep 3, 2019

View reviewed changes

MartinHjelmare added the breaking-change label Sep 3, 2019

MartinHjelmare merged commit 815e7a7 into home-assistant:dev Sep 6, 2019

lock Bot locked and limited conversation to collaborators Sep 7, 2019

tsvi deleted the jcal_binary_sensor branch September 11, 2019 15:06

tsvi restored the jcal_binary_sensor branch September 11, 2019 15:07

tsvi deleted the jcal_binary_sensor branch September 11, 2019 15:28

Uh oh!

Conversation

tsvi commented Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking Change:

Explanation

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tsvi commented Sep 3, 2019

Uh oh!

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tsvi commented Sep 3, 2019

Uh oh!

tsvi commented Sep 3, 2019

Uh oh!

MartinHjelmare commented Sep 3, 2019

Uh oh!

tsvi commented Sep 6, 2019

Uh oh!

MartinHjelmare commented Sep 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tsvi commented Aug 25, 2019 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):