Quality fixes for Jewish Calendar (#148689)

homeassistant/components/jewish_calendar/binary_sensor.py

@@ -13,8 +13,7 @@ from homeassistant.components.binary_sensor import (
     BinarySensorEntityDescription,
 )
 from homeassistant.const import EntityCategory
-from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
+from homeassistant.core import HomeAssistant
-from homeassistant.helpers import event
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
 from homeassistant.util import dt as dt_util
 
@@ -23,36 +22,29 @@ from .entity import JewishCalendarConfigEntry, JewishCalendarEntity
 PARALLEL_UPDATES = 0
 
 
-@dataclass(frozen=True)
+@dataclass(frozen=True, kw_only=True)
-class JewishCalendarBinarySensorMixIns(BinarySensorEntityDescription):
+class JewishCalendarBinarySensorEntityDescription(BinarySensorEntityDescription):
-    """Binary Sensor description mixin class for Jewish Calendar."""
-
-    is_on: Callable[[Zmanim, dt.datetime], bool] = lambda _, __: False
-
-
-@dataclass(frozen=True)
-class JewishCalendarBinarySensorEntityDescription(
-    JewishCalendarBinarySensorMixIns, BinarySensorEntityDescription
-):
     """Binary Sensor Entity description for Jewish Calendar."""
 
+    is_on: Callable[[Zmanim], Callable[[dt.datetime], bool]]
+
 
 BINARY_SENSORS: tuple[JewishCalendarBinarySensorEntityDescription, ...] = (
     JewishCalendarBinarySensorEntityDescription(
         key="issur_melacha_in_effect",
         translation_key="issur_melacha_in_effect",
-        is_on=lambda state, now: bool(state.issur_melacha_in_effect(now)),
+        is_on=lambda state: state.issur_melacha_in_effect,
     ),
     JewishCalendarBinarySensorEntityDescription(
         key="erev_shabbat_hag",
         translation_key="erev_shabbat_hag",
-        is_on=lambda state, now: bool(state.erev_shabbat_chag(now)),
+        is_on=lambda state: state.erev_shabbat_chag,
         entity_registry_enabled_default=False,
     ),
     JewishCalendarBinarySensorEntityDescription(
         key="motzei_shabbat_hag",
         translation_key="motzei_shabbat_hag",
-        is_on=lambda state, now: bool(state.motzei_shabbat_chag(now)),
+        is_on=lambda state: state.motzei_shabbat_chag,
         entity_registry_enabled_default=False,
     ),
 )
@@ -73,9 +65,7 @@ async def async_setup_entry(
 class JewishCalendarBinarySensor(JewishCalendarEntity, BinarySensorEntity):
     """Representation of an Jewish Calendar binary sensor."""
 
-    _attr_should_poll = False
     _attr_entity_category = EntityCategory.DIAGNOSTIC
-    _update_unsub: CALLBACK_TYPE | None = None
 
     entity_description: JewishCalendarBinarySensorEntityDescription
 
@@ -83,40 +73,12 @@ class JewishCalendarBinarySensor(JewishCalendarEntity, BinarySensorEntity):
     def is_on(self) -> bool:
         """Return true if sensor is on."""
         zmanim = self.make_zmanim(dt.date.today())
-        return self.entity_description.is_on(zmanim, dt_util.now())
+        return self.entity_description.is_on(zmanim)(dt_util.now())
 
-    async def async_added_to_hass(self) -> None:
+    def _update_times(self, zmanim: Zmanim) -> list[dt.datetime | None]:
-        """Run when entity about to be added to hass."""
+        """Return a list of times to update the sensor."""
-        await super().async_added_to_hass()
+        return [
-        self._schedule_update()
+            zmanim.netz_hachama.local + dt.timedelta(days=1),
-
+            zmanim.candle_lighting,
-    async def async_will_remove_from_hass(self) -> None:
+            zmanim.havdalah,
-        """Run when entity will be removed from hass."""
+        ]
-        if self._update_unsub:
-            self._update_unsub()
-            self._update_unsub = None
-        return await super().async_will_remove_from_hass()
-
-    @callback
-    def _update(self, now: dt.datetime | None = None) -> None:
-        """Update the state of the sensor."""
-        self._update_unsub = None
-        self._schedule_update()
-        self.async_write_ha_state()
-
-    def _schedule_update(self) -> None:
-        """Schedule the next update of the sensor."""
-        now = dt_util.now()
-        zmanim = self.make_zmanim(dt.date.today())
-        update = zmanim.netz_hachama.local + dt.timedelta(days=1)
-        candle_lighting = zmanim.candle_lighting
-        if candle_lighting is not None and now < candle_lighting < update:
-            update = candle_lighting
-        havdalah = zmanim.havdalah
-        if havdalah is not None and now < havdalah < update:
-            update = havdalah
-        if self._update_unsub:
-            self._update_unsub()
-        self._update_unsub = event.async_track_point_in_time(
-            self.hass, self._update, update
-        )

homeassistant/components/jewish_calendar/entity.py

@@ -1,17 +1,24 @@
 """Entity representing a Jewish Calendar sensor."""
 
+from abc import abstractmethod
 from dataclasses import dataclass
 import datetime as dt
+import logging
 
 from hdate import HDateInfo, Location, Zmanim
 from hdate.translator import Language, set_language
 
 from homeassistant.config_entries import ConfigEntry
+from homeassistant.core import CALLBACK_TYPE, callback
+from homeassistant.helpers import event
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
 from homeassistant.helpers.entity import Entity, EntityDescription
+from homeassistant.util import dt as dt_util
 
 from .const import DOMAIN
 
+_LOGGER = logging.getLogger(__name__)
+
 type JewishCalendarConfigEntry = ConfigEntry[JewishCalendarData]
 
 
@@ -39,6 +46,8 @@ class JewishCalendarEntity(Entity):
     """An HA implementation for Jewish Calendar entity."""
 
     _attr_has_entity_name = True
+    _attr_should_poll = False
+    _update_unsub: CALLBACK_TYPE | None = None
 
     def __init__(
         self,
@@ -63,3 +72,55 @@ class JewishCalendarEntity(Entity):
             candle_lighting_offset=self.data.candle_lighting_offset,
             havdalah_offset=self.data.havdalah_offset,
         )
+
+    async def async_added_to_hass(self) -> None:
+        """Call when entity is added to hass."""
+        await super().async_added_to_hass()
+        self._schedule_update()
+
+    async def async_will_remove_from_hass(self) -> None:
+        """Run when entity will be removed from hass."""
+        if self._update_unsub:
+            self._update_unsub()
+            self._update_unsub = None
+        return await super().async_will_remove_from_hass()
+
+    @abstractmethod
+    def _update_times(self, zmanim: Zmanim) -> list[dt.datetime | None]:
+        """Return a list of times to update the sensor."""
+
+    def _schedule_update(self) -> None:
+        """Schedule the next update of the sensor."""
+        now = dt_util.now()
+        zmanim = self.make_zmanim(now.date())
+        update = dt_util.start_of_local_day() + dt.timedelta(days=1)
+
+        for update_time in self._update_times(zmanim):
+            if update_time is not None and now < update_time < update:
+                update = update_time
+
+        if self._update_unsub:
+            self._update_unsub()
+        self._update_unsub = event.async_track_point_in_time(
+            self.hass, self._update, update
+        )
+
+    @callback
+    def _update(self, now: dt.datetime | None = None) -> None:
+        """Update the sensor data."""
+        self._update_unsub = None
+        self._schedule_update()
+        self.create_results(now)
+        self.async_write_ha_state()
+
+    def create_results(self, now: dt.datetime | None = None) -> None:
+        """Create the results for the sensor."""
+        if now is None:
+            now = dt_util.now()
+
+        _LOGGER.debug("Now: %s Location: %r", now, self.data.location)
+
+        today = now.date()
+        zmanim = self.make_zmanim(today)
+        dateinfo = HDateInfo(today, diaspora=self.data.diaspora)
+        self.data.results = JewishCalendarDataResults(dateinfo, zmanim)

homeassistant/components/jewish_calendar/sensor.py

@@ -17,16 +17,11 @@ from homeassistant.components.sensor import (
     SensorEntityDescription,
 )
 from homeassistant.const import EntityCategory
-from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
+from homeassistant.core import HomeAssistant
-from homeassistant.helpers import event
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
 from homeassistant.util import dt as dt_util
 
-from .entity import (
+from .entity import JewishCalendarConfigEntry, JewishCalendarEntity
-    JewishCalendarConfigEntry,
-    JewishCalendarDataResults,
-    JewishCalendarEntity,
-)
 
 _LOGGER = logging.getLogger(__name__)
 PARALLEL_UPDATES = 0
@@ -217,7 +212,7 @@ async def async_setup_entry(
     config_entry: JewishCalendarConfigEntry,
     async_add_entities: AddConfigEntryEntitiesCallback,
 ) -> None:
-    """Set up the Jewish calendar sensors ."""
+    """Set up the Jewish calendar sensors."""
     sensors: list[JewishCalendarBaseSensor] = [
         JewishCalendarSensor(config_entry, description) for description in INFO_SENSORS
     ]
@@ -231,59 +226,15 @@ async def async_setup_entry(
 class JewishCalendarBaseSensor(JewishCalendarEntity, SensorEntity):
     """Base class for Jewish calendar sensors."""
 
-    _attr_should_poll = False
     _attr_entity_category = EntityCategory.DIAGNOSTIC
-    _update_unsub: CALLBACK_TYPE | None = None
 
     entity_description: JewishCalendarBaseSensorDescription
 
-    async def async_added_to_hass(self) -> None:
+    def _update_times(self, zmanim: Zmanim) -> list[dt.datetime | None]:
-        """Call when entity is added to hass."""
+        """Return a list of times to update the sensor."""
-        await super().async_added_to_hass()
+        if self.entity_description.next_update_fn is None:
-        self._schedule_update()
+            return []
-
+        return [self.entity_description.next_update_fn(zmanim)]
-    async def async_will_remove_from_hass(self) -> None:
-        """Run when entity will be removed from hass."""
-        if self._update_unsub:
-            self._update_unsub()
-            self._update_unsub = None
-        return await super().async_will_remove_from_hass()
-
-    def _schedule_update(self) -> None:
-        """Schedule the next update of the sensor."""
-        now = dt_util.now()
-        zmanim = self.make_zmanim(now.date())
-        update = None
-        if self.entity_description.next_update_fn:
-            update = self.entity_description.next_update_fn(zmanim)
-        next_midnight = dt_util.start_of_local_day() + dt.timedelta(days=1)
-        if update is None or now > update:
-            update = next_midnight
-        if self._update_unsub:
-            self._update_unsub()
-        self._update_unsub = event.async_track_point_in_time(
-            self.hass, self._update_data, update
-        )
-
-    @callback
-    def _update_data(self, now: dt.datetime | None = None) -> None:
-        """Update the sensor data."""
-        self._update_unsub = None
-        self._schedule_update()
-        self.create_results(now)
-        self.async_write_ha_state()
-
-    def create_results(self, now: dt.datetime | None = None) -> None:
-        """Create the results for the sensor."""
-        if now is None:
-            now = dt_util.now()
-
-        _LOGGER.debug("Now: %s Location: %r", now, self.data.location)
-
-        today = now.date()
-        zmanim = self.make_zmanim(today)
-        dateinfo = HDateInfo(today, diaspora=self.data.diaspora)
-        self.data.results = JewishCalendarDataResults(dateinfo, zmanim)
 
     def get_dateinfo(self, now: dt.datetime | None = None) -> HDateInfo:
         """Get the next date info."""

homeassistant/components/jewish_calendar/services.py

@@ -50,7 +50,6 @@ def async_setup_services(hass: HomeAssistant) -> None:
         today = now.date()
         event_date = get_astral_event_date(hass, SUN_EVENT_SUNSET, today)
         if event_date is None:
-            _LOGGER.error("Can't get sunset event date for %s", today)
             raise HomeAssistantError(
                 translation_domain=DOMAIN, translation_key="sunset_event"
             )

tests/components/jewish_calendar/test_binary_sensor.py

@@ -6,11 +6,8 @@ from typing import Any
 from freezegun.api import FrozenDateTimeFactory
 import pytest
 
-from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN
+from homeassistant.const import STATE_OFF, STATE_ON
-from homeassistant.components.jewish_calendar.const import DOMAIN
-from homeassistant.const import CONF_PLATFORM, STATE_OFF, STATE_ON
 from homeassistant.core import HomeAssistant
-from homeassistant.setup import async_setup_component
 
 from tests.common import async_fire_time_changed
 
@@ -140,17 +137,3 @@ async def test_issur_melacha_sensor_update(
     async_fire_time_changed(hass)
     await hass.async_block_till_done()
     assert hass.states.get(sensor_id).state == results[1]
-
-
-async def test_no_discovery_info(
-    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
-) -> None:
-    """Test setup without discovery info."""
-    assert BINARY_SENSOR_DOMAIN not in hass.config.components
-    assert await async_setup_component(
-        hass,
-        BINARY_SENSOR_DOMAIN,
-        {BINARY_SENSOR_DOMAIN: {CONF_PLATFORM: DOMAIN}},
-    )
-    await hass.async_block_till_done()
-    assert BINARY_SENSOR_DOMAIN in hass.config.components

tests/components/jewish_calendar/test_config_flow.py

@@ -2,7 +2,7 @@
 
 from unittest.mock import AsyncMock
 
-from homeassistant import config_entries, setup
+from homeassistant import config_entries
 from homeassistant.components.jewish_calendar.const import (
     CONF_CANDLE_LIGHT_MINUTES,
     CONF_DIASPORA,
@@ -28,19 +28,18 @@ from tests.common import MockConfigEntry
 
 async def test_step_user(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None:
     """Test user config."""
-    await setup.async_setup_component(hass, "persistent_notification", {})
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
     assert result["type"] is FlowResultType.FORM
     assert result["step_id"] == "user"
 
-    result2 = await hass.config_entries.flow.async_configure(
+    result = await hass.config_entries.flow.async_configure(
         result["flow_id"],
         {CONF_DIASPORA: DEFAULT_DIASPORA, CONF_LANGUAGE: DEFAULT_LANGUAGE},
     )
 
-    assert result2["type"] is FlowResultType.CREATE_ENTRY
+    assert result["type"] is FlowResultType.CREATE_ENTRY
 
     await hass.async_block_till_done()
     assert len(mock_setup_entry.mock_calls) == 1

tests/components/jewish_calendar/test_sensor.py

@@ -8,11 +8,7 @@ from hdate.holidays import HolidayDatabase
 from hdate.parasha import Parasha
 import pytest
 
-from homeassistant.components.jewish_calendar.const import DOMAIN
-from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
-from homeassistant.const import CONF_PLATFORM
 from homeassistant.core import HomeAssistant
-from homeassistant.setup import async_setup_component
 from homeassistant.util import dt as dt_util
 
 from tests.common import MockConfigEntry, async_fire_time_changed
@@ -569,17 +565,3 @@ async def test_sensor_does_not_update_on_time_change(
     async_fire_time_changed(hass)
     await hass.async_block_till_done()
     assert hass.states.get(sensor_id).state == results["new_state"]
-
-
-async def test_no_discovery_info(
-    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
-) -> None:
-    """Test setup without discovery info."""
-    assert SENSOR_DOMAIN not in hass.config.components
-    assert await async_setup_component(
-        hass,
-        SENSOR_DOMAIN,
-        {SENSOR_DOMAIN: {CONF_PLATFORM: DOMAIN}},
-    )
-    await hass.async_block_till_done()
-    assert SENSOR_DOMAIN in hass.config.components

tests/components/jewish_calendar/test_service.py

@@ -4,7 +4,13 @@ import datetime as dt
 
 import pytest
 
-from homeassistant.components.jewish_calendar.const import DOMAIN
+from homeassistant.components.jewish_calendar.const import (
+    ATTR_AFTER_SUNSET,
+    ATTR_DATE,
+    ATTR_NUSACH,
+    DOMAIN,
+)
+from homeassistant.const import CONF_LANGUAGE
 from homeassistant.core import HomeAssistant
 
 
@@ -14,10 +20,10 @@ from homeassistant.core import HomeAssistant
         pytest.param(
             dt.datetime(2025, 3, 20, 21, 0),
             {
-                "date": dt.date(2025, 3, 20),
+                ATTR_DATE: dt.date(2025, 3, 20),
-                "nusach": "sfarad",
+                ATTR_NUSACH: "sfarad",
-                "language": "he",
+                CONF_LANGUAGE: "he",
-                "after_sunset": False,
+                ATTR_AFTER_SUNSET: False,
             },
             "",
             id="no_blessing",
@@ -25,10 +31,10 @@ from homeassistant.core import HomeAssistant
         pytest.param(
             dt.datetime(2025, 3, 20, 21, 0),
             {
-                "date": dt.date(2025, 5, 20),
+                ATTR_DATE: dt.date(2025, 5, 20),
-                "nusach": "ashkenaz",
+                ATTR_NUSACH: "ashkenaz",
-                "language": "he",
+                CONF_LANGUAGE: "he",
-                "after_sunset": False,
+                ATTR_AFTER_SUNSET: False,
             },
             "היום שבעה ושלושים יום שהם חמישה שבועות ושני ימים בעומר",
             id="ahskenaz-hebrew",
@@ -36,10 +42,10 @@ from homeassistant.core import HomeAssistant
         pytest.param(
             dt.datetime(2025, 3, 20, 21, 0),
             {
-                "date": dt.date(2025, 5, 20),
+                ATTR_DATE: dt.date(2025, 5, 20),
-                "nusach": "sfarad",
+                ATTR_NUSACH: "sfarad",
-                "language": "en",
+                CONF_LANGUAGE: "en",
-                "after_sunset": True,
+                ATTR_AFTER_SUNSET: True,
             },
             "Today is the thirty-eighth day, which are five weeks and three days of the Omer",
             id="sefarad-english-after-sunset",
@@ -47,23 +53,23 @@ from homeassistant.core import HomeAssistant
         pytest.param(
             dt.datetime(2025, 3, 20, 21, 0),
             {
-                "date": dt.date(2025, 5, 20),
+                ATTR_DATE: dt.date(2025, 5, 20),
-                "nusach": "sfarad",
+                ATTR_NUSACH: "sfarad",
-                "language": "en",
+                CONF_LANGUAGE: "en",
-                "after_sunset": False,
+                ATTR_AFTER_SUNSET: False,
             },
             "Today is the thirty-seventh day, which are five weeks and two days of the Omer",
             id="sefarad-english-before-sunset",
         ),
         pytest.param(
             dt.datetime(2025, 5, 20, 21, 0),
-            {"nusach": "sfarad", "language": "en"},
+            {ATTR_NUSACH: "sfarad", CONF_LANGUAGE: "en"},
             "Today is the thirty-eighth day, which are five weeks and three days of the Omer",
             id="sefarad-english-after-sunset-without-date",
         ),
         pytest.param(
             dt.datetime(2025, 5, 20, 6, 0),
-            {"nusach": "sfarad"},
+            {ATTR_NUSACH: "sfarad"},
             "היום שבעה ושלושים יום שהם חמישה שבועות ושני ימים לעומר",
             id="sefarad-english-before-sunset-without-date",
         ),