Address post merge review on israel rail (#121872)

* Address late israel rail review

* transfers => trains
This commit is contained in:
Shai Ungar 2024-07-13 11:54:27 +03:00 committed by GitHub
parent 662760360a
commit 2dec7136c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 275 additions and 133 deletions

View File

@ -3,7 +3,7 @@
from __future__ import annotations from __future__ import annotations
from dataclasses import dataclass from dataclasses import dataclass
from datetime import datetime, timedelta from datetime import datetime
import logging import logging
from israelrailapi import TrainSchedule from israelrailapi import TrainSchedule
@ -25,22 +25,11 @@ class DataConnection:
"""A connection data class.""" """A connection data class."""
departure: datetime | None departure: datetime | None
duration: int | None
platform: str platform: str
remaining_time: str
start: str start: str
destination: str destination: str
train_number: str train_number: str
transfers: int trains: int
def calculate_duration_in_seconds(start_time: str, end_time: str) -> int | None:
"""Transform and calculate the duration from start and end time into seconds."""
end_time_date = dt_util.parse_datetime(end_time)
start_time_date = dt_util.parse_datetime(start_time)
if not end_time_date or not start_time_date:
return None
return (end_time_date - start_time_date).seconds
def departure_time(train_route: TrainRoute) -> datetime | None: def departure_time(train_route: TrainRoute) -> datetime | None:
@ -49,15 +38,6 @@ def departure_time(train_route: TrainRoute) -> datetime | None:
return start_datetime.astimezone() if start_datetime else None return start_datetime.astimezone() if start_datetime else None
def remaining_time(departure) -> timedelta | None:
"""Calculate the remaining time for the departure."""
departure_datetime = dt_util.parse_datetime(departure)
if departure_datetime:
return dt_util.as_local(departure_datetime) - dt_util.as_local(dt_util.utcnow())
return None
class IsraelRailDataUpdateCoordinator(DataUpdateCoordinator[list[DataConnection]]): class IsraelRailDataUpdateCoordinator(DataUpdateCoordinator[list[DataConnection]]):
"""A IsraelRail Data Update Coordinator.""" """A IsraelRail Data Update Coordinator."""
@ -100,13 +80,9 @@ class IsraelRailDataUpdateCoordinator(DataUpdateCoordinator[list[DataConnection]
departure=departure_time(train_routes[i]), departure=departure_time(train_routes[i]),
train_number=train_routes[i].trains[0].data["trainNumber"], train_number=train_routes[i].trains[0].data["trainNumber"],
platform=train_routes[i].trains[0].platform, platform=train_routes[i].trains[0].platform,
transfers=len(train_routes[i].trains) - 1, trains=len(train_routes[i].trains),
duration=calculate_duration_in_seconds(
train_routes[i].start_time, train_routes[i].end_time
),
start=station_name_to_id(train_routes[i].trains[0].src), start=station_name_to_id(train_routes[i].trains[0].src),
destination=station_name_to_id(train_routes[i].trains[-1].dst), destination=station_name_to_id(train_routes[i].trains[-1].dst),
remaining_time=str(remaining_time(train_routes[i].trains[0].departure)),
) )
for i in range(DEPARTURES_COUNT) for i in range(DEPARTURES_COUNT)
if len(train_routes) > i and train_routes[i] is not None if len(train_routes) > i and train_routes[i] is not None

View File

@ -13,14 +13,14 @@
"duration": { "duration": {
"default": "mdi:timeline-clock" "default": "mdi:timeline-clock"
}, },
"transfers": { "trains": {
"default": "mdi:transit-transfer" "default": "mdi:train"
}, },
"platform": { "platform": {
"default": "mdi:bus-stop-uncovered" "default": "mdi:bus-stop-uncovered"
}, },
"train_number": { "train_number": {
"default": "mdi:train" "default": "mdi:numeric"
} }
} }
} }

View File

@ -13,7 +13,6 @@ from homeassistant.components.sensor import (
SensorEntity, SensorEntity,
SensorEntityDescription, SensorEntityDescription,
) )
from homeassistant.const import UnitOfTime
from homeassistant.core import HomeAssistant from homeassistant.core import HomeAssistant
from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.entity_platform import AddEntitiesCallback
@ -50,21 +49,15 @@ DEPARTURE_SENSORS: tuple[IsraelRailSensorEntityDescription, ...] = (
) )
SENSORS: tuple[IsraelRailSensorEntityDescription, ...] = ( SENSORS: tuple[IsraelRailSensorEntityDescription, ...] = (
IsraelRailSensorEntityDescription(
key="duration",
device_class=SensorDeviceClass.DURATION,
native_unit_of_measurement=UnitOfTime.SECONDS,
value_fn=lambda data_connection: data_connection.duration,
),
IsraelRailSensorEntityDescription( IsraelRailSensorEntityDescription(
key="platform", key="platform",
translation_key="platform", translation_key="platform",
value_fn=lambda data_connection: data_connection.platform, value_fn=lambda data_connection: data_connection.platform,
), ),
IsraelRailSensorEntityDescription( IsraelRailSensorEntityDescription(
key="transfers", key="trains",
translation_key="transfers", translation_key="trains",
value_fn=lambda data_connection: data_connection.transfers, value_fn=lambda data_connection: data_connection.trains,
), ),
IsraelRailSensorEntityDescription( IsraelRailSensorEntityDescription(
key="train_number", key="train_number",

View File

@ -28,8 +28,8 @@
"departure2": { "departure2": {
"name": "Departure +2" "name": "Departure +2"
}, },
"transfers": { "trains": {
"name": "Transfers" "name": "Trains"
}, },
"platform": { "platform": {
"name": "Platform" "name": "Platform"

View File

@ -135,51 +135,3 @@ TRAINS = [
destination_station="3700", destination_station="3700",
), ),
] ]
TRAINS_WRONG_FORMAT = [
get_train_route(
train_number="1234",
departure_time="2021-10-1010:10:10",
arrival_time="2021-10-10T10:30:10",
origin_platform="1",
dest_platform="2",
origin_station="3500",
destination_station="3700",
),
get_train_route(
train_number="1235",
departure_time="2021-10-1010:20:10",
arrival_time="2021-10-10T10:40:10",
origin_platform="1",
dest_platform="2",
origin_station="3500",
destination_station="3700",
),
get_train_route(
train_number="1236",
departure_time="2021-10-1010:30:10",
arrival_time="2021-10-10T10:50:10",
origin_platform="1",
dest_platform="2",
origin_station="3500",
destination_station="3700",
),
get_train_route(
train_number="1237",
departure_time="2021-10-1010:40:10",
arrival_time="2021-10-10T11:00:10",
origin_platform="1",
dest_platform="2",
origin_station="3500",
destination_station="3700",
),
get_train_route(
train_number="1238",
departure_time="2021-10-1010:50:10",
arrival_time="2021-10-10T11:10:10",
origin_platform="1",
dest_platform="2",
origin_station="3500",
destination_station="3700",
),
]

View File

@ -143,7 +143,7 @@
'state': '2021-10-10T10:30:10+00:00', 'state': '2021-10-10T10:30:10+00:00',
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_duration-entry] # name: test_valid_config[sensor.mock_title_none-entry]
EntityRegistryEntrySnapshot({ EntityRegistryEntrySnapshot({
'aliases': set({ 'aliases': set({
}), }),
@ -155,7 +155,7 @@
'disabled_by': None, 'disabled_by': None,
'domain': 'sensor', 'domain': 'sensor',
'entity_category': None, 'entity_category': None,
'entity_id': 'sensor.mock_title_duration', 'entity_id': 'sensor.mock_title_none',
'has_entity_name': True, 'has_entity_name': True,
'hidden_by': None, 'hidden_by': None,
'icon': None, 'icon': None,
@ -165,31 +165,123 @@
'name': None, 'name': None,
'options': dict({ 'options': dict({
}), }),
'original_device_class': <SensorDeviceClass.DURATION: 'duration'>, 'original_device_class': None,
'original_icon': None, 'original_icon': None,
'original_name': 'Duration', 'original_name': None,
'platform': 'israel_rail', 'platform': 'israel_rail',
'previous_unique_id': None, 'previous_unique_id': None,
'supported_features': 0, 'supported_features': 0,
'translation_key': None, 'translation_key': 'platform',
'unique_id': 'באר יעקב אשקלון_duration', 'unique_id': 'באר יעקב אשקלון_platform',
'unit_of_measurement': <UnitOfTime.SECONDS: 's'>, 'unit_of_measurement': None,
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_duration-state] # name: test_valid_config[sensor.mock_title_none-state]
StateSnapshot({ StateSnapshot({
'attributes': ReadOnlyDict({ 'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.', 'attribution': 'Data provided by Israel rail.',
'device_class': 'duration', 'friendly_name': 'Mock Title None',
'friendly_name': 'Mock Title Duration',
'unit_of_measurement': <UnitOfTime.SECONDS: 's'>,
}), }),
'context': <ANY>, 'context': <ANY>,
'entity_id': 'sensor.mock_title_duration', 'entity_id': 'sensor.mock_title_none',
'last_changed': <ANY>, 'last_changed': <ANY>,
'last_reported': <ANY>, 'last_reported': <ANY>,
'last_updated': <ANY>, 'last_updated': <ANY>,
'state': '1200', 'state': '1',
})
# ---
# name: test_valid_config[sensor.mock_title_none_2-entry]
EntityRegistryEntrySnapshot({
'aliases': set({
}),
'area_id': None,
'capabilities': None,
'config_entry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.mock_title_none_2',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'options': dict({
}),
'original_device_class': None,
'original_icon': None,
'original_name': None,
'platform': 'israel_rail',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': 'trains',
'unique_id': 'באר יעקב אשקלון_trains',
'unit_of_measurement': None,
})
# ---
# name: test_valid_config[sensor.mock_title_none_2-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.',
'friendly_name': 'Mock Title None',
}),
'context': <ANY>,
'entity_id': 'sensor.mock_title_none_2',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '1',
})
# ---
# name: test_valid_config[sensor.mock_title_none_3-entry]
EntityRegistryEntrySnapshot({
'aliases': set({
}),
'area_id': None,
'capabilities': None,
'config_entry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.mock_title_none_3',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'options': dict({
}),
'original_device_class': None,
'original_icon': None,
'original_name': None,
'platform': 'israel_rail',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': 'train_number',
'unique_id': 'באר יעקב אשקלון_train_number',
'unit_of_measurement': None,
})
# ---
# name: test_valid_config[sensor.mock_title_none_3-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.',
'friendly_name': 'Mock Title None',
}),
'context': <ANY>,
'entity_id': 'sensor.mock_title_none_3',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '1234',
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_platform-entry] # name: test_valid_config[sensor.mock_title_platform-entry]
@ -239,6 +331,150 @@
'state': '1', 'state': '1',
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_timestamp-entry]
EntityRegistryEntrySnapshot({
'aliases': set({
}),
'area_id': None,
'capabilities': None,
'config_entry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.mock_title_timestamp',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'options': dict({
}),
'original_device_class': <SensorDeviceClass.TIMESTAMP: 'timestamp'>,
'original_icon': None,
'original_name': 'Timestamp',
'platform': 'israel_rail',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': 'departure0',
'unique_id': 'באר יעקב אשקלון_departure',
'unit_of_measurement': None,
})
# ---
# name: test_valid_config[sensor.mock_title_timestamp-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.',
'device_class': 'timestamp',
'friendly_name': 'Mock Title Timestamp',
}),
'context': <ANY>,
'entity_id': 'sensor.mock_title_timestamp',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '2021-10-10T10:10:10+00:00',
})
# ---
# name: test_valid_config[sensor.mock_title_timestamp_2-entry]
EntityRegistryEntrySnapshot({
'aliases': set({
}),
'area_id': None,
'capabilities': None,
'config_entry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.mock_title_timestamp_2',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'options': dict({
}),
'original_device_class': <SensorDeviceClass.TIMESTAMP: 'timestamp'>,
'original_icon': None,
'original_name': 'Timestamp',
'platform': 'israel_rail',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': 'departure1',
'unique_id': 'באר יעקב אשקלון_departure1',
'unit_of_measurement': None,
})
# ---
# name: test_valid_config[sensor.mock_title_timestamp_2-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.',
'device_class': 'timestamp',
'friendly_name': 'Mock Title Timestamp',
}),
'context': <ANY>,
'entity_id': 'sensor.mock_title_timestamp_2',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '2021-10-10T10:20:10+00:00',
})
# ---
# name: test_valid_config[sensor.mock_title_timestamp_3-entry]
EntityRegistryEntrySnapshot({
'aliases': set({
}),
'area_id': None,
'capabilities': None,
'config_entry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.mock_title_timestamp_3',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'options': dict({
}),
'original_device_class': <SensorDeviceClass.TIMESTAMP: 'timestamp'>,
'original_icon': None,
'original_name': 'Timestamp',
'platform': 'israel_rail',
'previous_unique_id': None,
'supported_features': 0,
'translation_key': 'departure2',
'unique_id': 'באר יעקב אשקלון_departure2',
'unit_of_measurement': None,
})
# ---
# name: test_valid_config[sensor.mock_title_timestamp_3-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.',
'device_class': 'timestamp',
'friendly_name': 'Mock Title Timestamp',
}),
'context': <ANY>,
'entity_id': 'sensor.mock_title_timestamp_3',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '2021-10-10T10:30:10+00:00',
})
# ---
# name: test_valid_config[sensor.mock_title_train_number-entry] # name: test_valid_config[sensor.mock_title_train_number-entry]
EntityRegistryEntrySnapshot({ EntityRegistryEntrySnapshot({
'aliases': set({ 'aliases': set({
@ -286,7 +522,7 @@
'state': '1234', 'state': '1234',
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_transfers-entry] # name: test_valid_config[sensor.mock_title_trains-entry]
EntityRegistryEntrySnapshot({ EntityRegistryEntrySnapshot({
'aliases': set({ 'aliases': set({
}), }),
@ -298,7 +534,7 @@
'disabled_by': None, 'disabled_by': None,
'domain': 'sensor', 'domain': 'sensor',
'entity_category': None, 'entity_category': None,
'entity_id': 'sensor.mock_title_transfers', 'entity_id': 'sensor.mock_title_trains',
'has_entity_name': True, 'has_entity_name': True,
'hidden_by': None, 'hidden_by': None,
'icon': None, 'icon': None,
@ -310,26 +546,26 @@
}), }),
'original_device_class': None, 'original_device_class': None,
'original_icon': None, 'original_icon': None,
'original_name': 'Transfers', 'original_name': 'Trains',
'platform': 'israel_rail', 'platform': 'israel_rail',
'previous_unique_id': None, 'previous_unique_id': None,
'supported_features': 0, 'supported_features': 0,
'translation_key': 'transfers', 'translation_key': 'trains',
'unique_id': 'באר יעקב אשקלון_transfers', 'unique_id': 'באר יעקב אשקלון_trains',
'unit_of_measurement': None, 'unit_of_measurement': None,
}) })
# --- # ---
# name: test_valid_config[sensor.mock_title_transfers-state] # name: test_valid_config[sensor.mock_title_trains-state]
StateSnapshot({ StateSnapshot({
'attributes': ReadOnlyDict({ 'attributes': ReadOnlyDict({
'attribution': 'Data provided by Israel rail.', 'attribution': 'Data provided by Israel rail.',
'friendly_name': 'Mock Title Transfers', 'friendly_name': 'Mock Title Trains',
}), }),
'context': <ANY>, 'context': <ANY>,
'entity_id': 'sensor.mock_title_transfers', 'entity_id': 'sensor.mock_title_trains',
'last_changed': <ANY>, 'last_changed': <ANY>,
'last_reported': <ANY>, 'last_reported': <ANY>,
'last_updated': <ANY>, 'last_updated': <ANY>,
'state': '0', 'state': '1',
}) })
# --- # ---

View File

@ -12,7 +12,7 @@ from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er from homeassistant.helpers import entity_registry as er
from . import goto_future, init_integration from . import goto_future, init_integration
from .conftest import TRAINS, TRAINS_WRONG_FORMAT, get_time from .conftest import TRAINS, get_time
from tests.common import MockConfigEntry, snapshot_platform from tests.common import MockConfigEntry, snapshot_platform
@ -26,7 +26,7 @@ async def test_valid_config(
) -> None: ) -> None:
"""Ensure everything starts correctly.""" """Ensure everything starts correctly."""
await init_integration(hass, mock_config_entry) await init_integration(hass, mock_config_entry)
assert len(hass.states.async_entity_ids()) == 7 assert len(hass.states.async_entity_ids()) == 6
await snapshot_platform(hass, entity_registry, snapshot, mock_config_entry.entry_id) await snapshot_platform(hass, entity_registry, snapshot, mock_config_entry.entry_id)
@ -38,7 +38,7 @@ async def test_update_train(
) -> None: ) -> None:
"""Ensure the train data is updated.""" """Ensure the train data is updated."""
await init_integration(hass, mock_config_entry) await init_integration(hass, mock_config_entry)
assert len(hass.states.async_entity_ids()) == 7 assert len(hass.states.async_entity_ids()) == 6
departure_sensor = hass.states.get("sensor.mock_title_departure") departure_sensor = hass.states.get("sensor.mock_title_departure")
expected_time = get_time(10, 10) expected_time = get_time(10, 10)
assert departure_sensor.state == expected_time assert departure_sensor.state == expected_time
@ -47,27 +47,12 @@ async def test_update_train(
await goto_future(hass, freezer) await goto_future(hass, freezer)
assert len(hass.states.async_entity_ids()) == 7 assert len(hass.states.async_entity_ids()) == 6
departure_sensor = hass.states.get("sensor.mock_title_departure") departure_sensor = hass.states.get("sensor.mock_title_departure")
expected_time = get_time(10, 20) expected_time = get_time(10, 20)
assert departure_sensor.state == expected_time assert departure_sensor.state == expected_time
async def test_no_duration_wrong_date_format(
hass: HomeAssistant,
mock_israelrail: AsyncMock,
mock_config_entry: MockConfigEntry,
) -> None:
"""Ensure the duration is not set when there is no departure time."""
mock_israelrail.query.return_value = TRAINS_WRONG_FORMAT
await init_integration(hass, mock_config_entry)
assert len(hass.states.async_entity_ids()) == 7
departure_sensor = hass.states.get("sensor.mock_title_train_number")
assert departure_sensor.state == "1234"
duration_sensor = hass.states.get("sensor.mock_title_duration")
assert duration_sensor.state == "unknown"
async def test_fail_query( async def test_fail_query(
hass: HomeAssistant, hass: HomeAssistant,
freezer: FrozenDateTimeFactory, freezer: FrozenDateTimeFactory,
@ -76,9 +61,9 @@ async def test_fail_query(
) -> None: ) -> None:
"""Ensure the integration handles query failures.""" """Ensure the integration handles query failures."""
await init_integration(hass, mock_config_entry) await init_integration(hass, mock_config_entry)
assert len(hass.states.async_entity_ids()) == 7 assert len(hass.states.async_entity_ids()) == 6
mock_israelrail.query.side_effect = Exception("error") mock_israelrail.query.side_effect = Exception("error")
await goto_future(hass, freezer) await goto_future(hass, freezer)
assert len(hass.states.async_entity_ids()) == 7 assert len(hass.states.async_entity_ids()) == 6
departure_sensor = hass.states.get("sensor.mock_title_departure") departure_sensor = hass.states.get("sensor.mock_title_departure")
assert departure_sensor.state == STATE_UNAVAILABLE assert departure_sensor.state == STATE_UNAVAILABLE