ronantakizawa/codereview-bench huggingface dataset:
Repository: home-assistant/core
File: tests/components/bang_olufsen/test_binary_sensor.py (Python)

Code before:
    entity_registry: EntityRegistry,
    mock_mozart_client: AsyncMock,
    mock_config_entry_a5: MockConfigEntry,
) -> None:
    """Test the battery charging time entity."""
    # Ensure battery entities are created
    mock_mozart_client.get_battery_state.return_value = TEST_BATTERY

    # Load entry
    mock_config_entry_a5.add_to_hass(hass)
    await hass.config_entries.async_setup(mock_config_entry_a5.entry_id)
    await mock_websocket_connection(hass, mock_mozart_client)
    await hass.async_block_till_done()

    # Initial state is False
    assert (states := hass.states.get(TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID))
    assert states.state == "off"

    # Check binary sensor reacts as expected to WebSocket events
    battery_callback = mock_mozart_client.get_battery_notifications.call_args[0][0]

    battery_callback(BatteryState(is_charging=True))
    await hass.async_block_till_done()

    assert (states := hass.states.get(TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID))
    assert states.state == "on"


Review comment (suggestion):
```suggestion
    assert states.state == STATE_ON
```

Code after:
    hass: HomeAssistant,
    entity_registry: EntityRegistry,
    mock_mozart_client: AsyncMock,
    mock_config_entry_a5: MockConfigEntry,
) -> None:
    """Test the battery charging time entity."""
    # Ensure battery entities are created
    mock_mozart_client.get_battery_state.return_value = TEST_BATTERY

    # Load entry
    mock_config_entry_a5.add_to_hass(hass)
    await hass.config_entries.async_setup(mock_config_entry_a5.entry_id)
    await mock_websocket_connection(hass, mock_mozart_client)
    await hass.async_block_till_done()

    # Initial state is False
    assert (states := hass.states.get(TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID))
    assert states.state == STATE_OFF

    # Check binary sensor reacts as expected to WebSocket events
    battery_callback = mock_mozart_client.get_battery_notifications.call_args[0][0]

    battery_callback(BatteryState(is_charging=True))
    await hass.async_block_till_done()

    assert (states := hass.states.get(TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID))
    assert states.state == STATE_ON
Repository: home-assistant/core
File: tests/components/bang_olufsen/test_binary_sensor.py (Python)

Code before:
"""Test the bang_olufsen binary sensor entities."""

from unittest.mock import AsyncMock

from mozart_api.models import BatteryState

from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity_registry import EntityRegistry

from .conftest import mock_websocket_connection
from .const import TEST_BATTERY, TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID

from tests.common import MockConfigEntry


async def test_battery_charging(
    hass: HomeAssistant,
    entity_registry: EntityRegistry,
    mock_mozart_client: AsyncMock,
    mock_config_entry_a5: MockConfigEntry,
) -> None:
    """Test the battery charging time entity."""
    # Ensure battery entities are created
    mock_mozart_client.get_battery_state.return_value = TEST_BATTERY

    # Load entry
    mock_config_entry_a5.add_to_hass(hass)
    await hass.config_entries.async_setup(mock_config_entry_a5.entry_id)
    await mock_websocket_connection(hass, mock_mozart_client)
    await hass.async_block_till_done()

    # Initial state is False
    assert (states := hass.states.get(TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID))

Review comment (suggestion):
```suggestion
from homeassistant.const import STATE_OFF, STATE_ON
from homeassistant.core import HomeAssistant
```

Code after:
"""Test the bang_olufsen binary sensor entities."""

from unittest.mock import AsyncMock

from mozart_api.models import BatteryState

from homeassistant.const import STATE_OFF, STATE_ON
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity_registry import EntityRegistry

from .conftest import mock_websocket_connection
from .const import TEST_BATTERY, TEST_BATTERY_CHARGING_BINARY_SENSOR_ENTITY_ID

from tests.common import MockConfigEntry


async def test_battery_charging(
    hass: HomeAssistant,
    entity_registry: EntityRegistry,
    mock_mozart_client: AsyncMock,
    mock_config_entry_a5: MockConfigEntry,
) -> None:
    """Test the battery charging time entity."""
    # Ensure battery entities are created
    mock_mozart_client.get_battery_state.return_value = TEST_BATTERY

    # Load entry
    mock_config_entry_a5.add_to_hass(hass)
    await hass.config_entries.async_setup(mock_config_entry_a5.entry_id)
    await mock_websocket_connection(hass, mock_mozart_client)
    await hass.async_block_till_done()

    # Initial state is False
Repository: home-assistant/core
File: homeassistant/components/downloader/services.py (Python)

Code before:
                f"{DOMAIN}_{DOWNLOAD_FAILED_EVENT}",
                {"url": url, "filename": filename},
            )

            # Remove file if we started downloading but failed
            if final_path and os.path.isfile(final_path):
                os.remove(final_path)

    threading.Thread(target=do_download).start()


@callback
def async_setup_services(hass: HomeAssistant) -> None:
    """Register the services for the downloader component."""
    async_register_admin_service(
        hass,
        DOMAIN,
        SERVICE_DOWNLOAD_FILE,
        download_file,
        schema=vol.Schema(
            {
                vol.Optional(ATTR_FILENAME): cv.string,
                vol.Optional(ATTR_SUBDIR): cv.string,
                vol.Required(ATTR_URL): cv.url,
                vol.Optional(ATTR_OVERWRITE, default=False): cv.boolean,
                vol.Optional(ATTR_HEADERS, default={}): dict,
            }
        ),
    )


Review comment (suggestion):
```suggestion
                vol.Optional(ATTR_HEADERS, default=dict): vol.Schema(
            		{cv.string: cv.string}
        		),
```
Should do the trick, I believe. :)

Code after:
                f"{DOMAIN}_{DOWNLOAD_FAILED_EVENT}",
                {"url": url, "filename": filename},
            )

            # Remove file if we started downloading but failed
            if final_path and os.path.isfile(final_path):
                os.remove(final_path)

    threading.Thread(target=do_download).start()


@callback
def async_setup_services(hass: HomeAssistant) -> None:
    """Register the services for the downloader component."""
    async_register_admin_service(
        hass,
        DOMAIN,
        SERVICE_DOWNLOAD_FILE,
        download_file,
        schema=vol.Schema(
            {
                vol.Optional(ATTR_FILENAME): cv.string,
                vol.Optional(ATTR_SUBDIR): cv.string,
                vol.Required(ATTR_URL): cv.url,
                vol.Optional(ATTR_OVERWRITE, default=False): cv.boolean,
                vol.Optional(ATTR_HEADERS, default=dict): vol.Schema(
                    {cv.string: cv.string}
                ),
            }
        ),
    )
Repository: home-assistant/core
File: homeassistant/components/hdfury/button.py (Python)

Code before:
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN
from .entity import HDFuryEntity


@dataclass(kw_only=True, frozen=True)
class HDFuryButtonEntityDescription(ButtonEntityDescription):
    """Description for HDFury button entities."""

    press_fn: Callable[[HDFuryAPI], Awaitable[None]]


BUTTONS: tuple[HDFuryButtonEntityDescription, ...] = (
    HDFuryButtonEntityDescription(
        key="reboot",
        translation_key="reboot",
        entity_category=EntityCategory.CONFIG,
        press_fn=lambda client: client.issue_reboot(),
    ),
    HDFuryButtonEntityDescription(
        key="issue_hotplug",
        translation_key="issue_hotplug",
        entity_category=EntityCategory.CONFIG,
        press_fn=lambda client: client.issue_hotplug(),
    ),
)


async def async_setup_entry(
    hass: HomeAssistant,
    entry: ConfigEntry,
    async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
    """Set up buttons using the platform schema."""

    coordinator = entry.runtime_data

    entities: list[HDFuryEntity] = [
        HDFuryButton(coordinator, description) for description in BUTTONS
    ]

    async_add_entities(entities)


class HDFuryButton(HDFuryEntity, ButtonEntity):
    """HDFury Button Class."""

    entity_description: HDFuryButtonEntityDescription

Review comment (suggestion):
1. Use the device class for rebooting
2. If we have the device class you can omit the translation key

Code after:
    ButtonEntityDescription,
)
from homeassistant.const import EntityCategory
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN
from .coordinator import HDFuryConfigEntry
from .entity import HDFuryEntity


@dataclass(kw_only=True, frozen=True)
class HDFuryButtonEntityDescription(ButtonEntityDescription):
    """Description for HDFury button entities."""

    press_fn: Callable[[HDFuryAPI], Awaitable[None]]


BUTTONS: tuple[HDFuryButtonEntityDescription, ...] = (
    HDFuryButtonEntityDescription(
        key="reboot",
        device_class=ButtonDeviceClass.RESTART,
        entity_category=EntityCategory.CONFIG,
        press_fn=lambda client: client.issue_reboot(),
    ),
    HDFuryButtonEntityDescription(
        key="issue_hotplug",
        translation_key="issue_hotplug",
        entity_category=EntityCategory.CONFIG,
        press_fn=lambda client: client.issue_hotplug(),
    ),
)


async def async_setup_entry(
    hass: HomeAssistant,
    entry: HDFuryConfigEntry,
    async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
    """Set up buttons using the platform schema."""

    coordinator = entry.runtime_data

    async_add_entities(
        HDFuryButton(coordinator, description) for description in BUTTONS
    )


class HDFuryButton(HDFuryEntity, ButtonEntity):
    """HDFury Button Class."""
Repository: home-assistant/core
File: tests/components/jvc_projector/test_binary_sensor.py (Python)

Code before:
        BINARY_SENSOR_DOMAIN,
        DOMAIN,
        f"{MOCK_MAC}_power",
        config_entry=mock_config_entry,
        suggested_object_id="jvc_projector_power",
    )

    # Ignored entity to get to 100% coverage
    entity_registry.async_get_or_create(
        BINARY_SENSOR_DOMAIN,
        DOMAIN,
        f"{MOCK_MAC}_ignore",
        config_entry=mock_config_entry,
        suggested_object_id="jvc_projector_ignore",
    )

    await hass.config_entries.async_setup(mock_config_entry.entry_id)
    await hass.async_block_till_done()

    entity = hass.states.get(ENTITY_ID)
    assert entity
    entry = entity_registry.async_get(entity.entity_id)
    assert entry

    coordinator = mock_config_entry.runtime_data
    assert entry.unique_id == f"{coordinator.unique_id}_power"


Review comment (suggestion):
we shouldn't touch the coordinator in the tests though! Instead we can just replace this with what we expect to make sure the test can't break when other components change

Code after:
"""Tests for the JVC Projector binary sensor device."""

from unittest.mock import MagicMock

from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN
from homeassistant.components.jvc_projector.const import DOMAIN
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er

from . import MOCK_MAC, MOCK_MAC_FORMATED

from tests.common import MockConfigEntry

ENTITY_ID = "binary_sensor.jvc_projector_power"


async def test_entity_state(
    hass: HomeAssistant,
    entity_registry: er.EntityRegistry,
    mock_device: MagicMock,
    mock_integration: MockConfigEntry,
) -> None:
    """Tests entity state is registered."""
    entity = hass.states.get(ENTITY_ID)
    assert entity
    assert entity_registry.async_get(entity.entity_id)


async def test_migrate_old_unique_id(
    hass: HomeAssistant,
    entity_registry: er.EntityRegistry,
    mock_device: MagicMock,
    mock_config_entry: MockConfigEntry,
) -> None:
    """Tests power binary sensor unique id is reformatted."""
    mock_config_entry.add_to_hass(hass)

    # Entity to be migrated
    entity_registry.async_get_or_create(
        BINARY_SENSOR_DOMAIN,
        DOMAIN,
        f"{MOCK_MAC}_power",
        config_entry=mock_config_entry,
        suggested_object_id="jvc_projector_power",
    )

    # Ignored entity to get to 100% coverage
    entity_registry.async_get_or_create(
        BINARY_SENSOR_DOMAIN,
        DOMAIN,
        f"{MOCK_MAC}_ignore",
        config_entry=mock_config_entry,
        suggested_object_id="jvc_projector_ignore",
    )

    await hass.config_entries.async_setup(mock_config_entry.entry_id)
    await hass.async_block_till_done()

    entity = hass.states.get(ENTITY_ID)
    assert entity
    entry = entity_registry.async_get(entity.entity_id)
    assert entry
    assert entry.unique_id == f"{MOCK_MAC_FORMATED}_power"
Repository: home-assistant/core
File: homeassistant/components/teslemetry/coordinator.py (Python)

Code before:
    SubscriptionRequired,
    TeslaFleetError,
)
from tesla_fleet_api.teslemetry import EnergySite, Vehicle

from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

if TYPE_CHECKING:
    from . import TeslemetryConfigEntry

from .const import ENERGY_HISTORY_FIELDS, LOGGER
from .helpers import flatten

RETRY_EXCEPTIONS = (
    InvalidResponse,
    RateLimited,
    ServiceUnavailable,
    GatewayTimeout,
)


def _get_retry_after(e: TeslaFleetError) -> float:
    """Calculate wait time from exception."""
    data = getattr(e, "data", {})
    if isinstance(data, dict):
        if after := data.get("after"):
            return float(after)
    return 10.0


VEHICLE_INTERVAL = timedelta(seconds=60)
VEHICLE_WAIT = timedelta(minutes=15)
ENERGY_LIVE_INTERVAL = timedelta(seconds=30)
ENERGY_INFO_INTERVAL = timedelta(seconds=30)
ENERGY_HISTORY_INTERVAL = timedelta(seconds=60)

ENDPOINTS = [
    VehicleDataEndpoint.CHARGE_STATE,
    VehicleDataEndpoint.CLIMATE_STATE,
    VehicleDataEndpoint.DRIVE_STATE,
    VehicleDataEndpoint.LOCATION_DATA,
    VehicleDataEndpoint.VEHICLE_STATE,
    VehicleDataEndpoint.VEHICLE_CONFIG,
]


class TeslemetryVehicleDataCoordinator(DataUpdateCoordinator[dict[str, Any]]):
    """Class to manage fetching data from the Teslemetry API."""


Review comment (suggestion):
Not sure why we use getattr, as the data is always available for a TeslaFleetError

Code after:
    SubscriptionRequired,
    TeslaFleetError,
)
from tesla_fleet_api.teslemetry import EnergySite, Vehicle

from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryAuthFailed
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

if TYPE_CHECKING:
    from . import TeslemetryConfigEntry

from .const import ENERGY_HISTORY_FIELDS, LOGGER
from .helpers import flatten

RETRY_EXCEPTIONS = (
    InvalidResponse,
    RateLimited,
    ServiceUnavailable,
    GatewayTimeout,
)


def _get_retry_after(e: TeslaFleetError) -> float:
    """Calculate wait time from exception."""
    if isinstance(e.data, dict):
        if after := e.data.get("after"):
            return float(after)
    return 10.0


VEHICLE_INTERVAL = timedelta(seconds=60)
VEHICLE_WAIT = timedelta(minutes=15)
ENERGY_LIVE_INTERVAL = timedelta(seconds=30)
ENERGY_INFO_INTERVAL = timedelta(seconds=30)
ENERGY_HISTORY_INTERVAL = timedelta(seconds=60)

ENDPOINTS = [
    VehicleDataEndpoint.CHARGE_STATE,
    VehicleDataEndpoint.CLIMATE_STATE,
    VehicleDataEndpoint.DRIVE_STATE,
    VehicleDataEndpoint.LOCATION_DATA,
    VehicleDataEndpoint.VEHICLE_STATE,
    VehicleDataEndpoint.VEHICLE_CONFIG,
]


class TeslemetryVehicleDataCoordinator(DataUpdateCoordinator[dict[str, Any]]):
    """Class to manage fetching data from the Teslemetry API."""

    config_entry: TeslemetryConfigEntry
Repository: home-assistant/core
File: homeassistant/components/pooldose/entity.py (Python)

Code before:
"""Base entity for Seko Pooldose integration."""

from __future__ import annotations

from collections.abc import Awaitable, Callable
from typing import Any, Literal

from pooldose.type_definitions import DeviceInfoDict, ValueDict

from homeassistant.const import CONF_MAC
from homeassistant.exceptions import ServiceValidationError
from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC, DeviceInfo
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.update_coordinator import CoordinatorEntity

from .const import DOMAIN, MANUFACTURER
from .coordinator import PooldoseCoordinator


def device_info(
    info: DeviceInfoDict | None, unique_id: str, mac: str | None = None
) -> DeviceInfo:
    """Create device info for PoolDose devices."""
    if info is None:
        info = {}

    api_version = (info.get("API_VERSION") or "").removesuffix("/")

    return DeviceInfo(
        identifiers={(DOMAIN, unique_id)},

Review comment (suggestion):
```suggestion
from collections.abc import Callable, Coroutine
```

Code after:
"""Base entity for Seko Pooldose integration."""

from __future__ import annotations

from collections.abc import Callable, Coroutine
from typing import Any, Literal

from pooldose.type_definitions import DeviceInfoDict, ValueDict

from homeassistant.const import CONF_MAC
from homeassistant.exceptions import ServiceValidationError
from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC, DeviceInfo
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.update_coordinator import CoordinatorEntity

from .const import DOMAIN, MANUFACTURER
from .coordinator import PooldoseCoordinator


def device_info(
    info: DeviceInfoDict | None, unique_id: str, mac: str | None = None
) -> DeviceInfo:
    """Create device info for PoolDose devices."""
    if info is None:
        info = {}

    api_version = (info.get("API_VERSION") or "").removesuffix("/")

    return DeviceInfo(
        identifiers={(DOMAIN, unique_id)},
Repository: home-assistant/core
File: homeassistant/components/mycroft/notify.py (Python)

Code before:
"""Mycroft AI notification platform."""

from __future__ import annotations

import logging

from mycroftapi import MycroftAPI

from homeassistant.components.notify import BaseNotificationService
from homeassistant.core import HomeAssistant
from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType

_LOGGER = logging.getLogger(__name__)


def get_service(
    hass: HomeAssistant,
    config: ConfigType,
    discovery_info: DiscoveryInfoType | None = None,
) -> MycroftNotificationService:
    """Get the Mycroft notification service."""
    return MycroftNotificationService(hass.data["mycroft"])


class MycroftNotificationService(BaseNotificationService):
    """The Mycroft Notification Service."""

    def __init__(self, mycroft_ip):
        """Initialize the service."""
        self.mycroft_ip = mycroft_ip

    def send_message(self, message="", **kwargs):
        """Send a message mycroft to speak on instance."""

        text = message
        mycroft = MycroftAPI(self.mycroft_ip)
        if mycroft is not None:
            mycroft.speak_text(text)
        else:
            _LOGGER.log("Could not reach this instance of mycroft")


Review comment (suggestion):
`_LOGGER.log` was causing mypy issue `Missing positional argument "msg" in call to "log" of "Logger"`

Code after:

def get_service(
    hass: HomeAssistant,
    config: ConfigType,
    discovery_info: DiscoveryInfoType | None = None,
) -> MycroftNotificationService:
    """Get the Mycroft notification service."""
    return MycroftNotificationService(hass.data[DOMAIN])


class MycroftNotificationService(BaseNotificationService):
    """The Mycroft Notification Service."""

    def __init__(self, mycroft_ip: str) -> None:
        """Initialize the service."""
        self.mycroft_ip = mycroft_ip

    def send_message(self, message: str = "", **kwargs: Any) -> None:
        """Send a message mycroft to speak on instance."""

        text = message
        mycroft = MycroftAPI(self.mycroft_ip)
        if mycroft is not None:
            mycroft.speak_text(text)
        else:
            _LOGGER.warning("Could not reach this instance of mycroft")
Repository: home-assistant/core
File: homeassistant/components/hikvision/__init__.py (Python)

Code before:
    CONF_PASSWORD,
    CONF_PORT,
    CONF_SSL,
    CONF_USERNAME,
    Platform,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers import device_registry as dr

from .const import DOMAIN

_LOGGER = logging.getLogger(__name__)

PLATFORMS = [Platform.BINARY_SENSOR, Platform.CAMERA]


@dataclass
class HikvisionData:
    """Data class for Hikvision runtime data."""

    camera: HikCamera
    device_id: str
    device_name: str
    device_type: str
    channels: list[VideoChannel] = field(default_factory=list)


type HikvisionConfigEntry = ConfigEntry[HikvisionData]


async def async_setup_entry(hass: HomeAssistant, entry: HikvisionConfigEntry) -> bool:
    """Set up Hikvision from a config entry."""
    host = entry.data[CONF_HOST]
    port = entry.data[CONF_PORT]
    username = entry.data[CONF_USERNAME]
    password = entry.data[CONF_PASSWORD]
    ssl = entry.data[CONF_SSL]

    protocol = "https" if ssl else "http"
    url = f"{protocol}://{host}"

    try:
        camera = await hass.async_add_executor_job(
            HikCamera, url, port, username, password, ssl
        )
    except requests.exceptions.RequestException as err:
        raise ConfigEntryNotReady(f"Unable to connect to {host}") from err

    device_id = camera.get_id
    if device_id is None:

Review comment (suggestion):
Instead, store the channels in a `dict` with `channel.id` as key, this way you only have to do simple dict lookups

Code after:
    CONF_HOST,
    CONF_PASSWORD,
    CONF_PORT,
    CONF_SSL,
    CONF_USERNAME,
    Platform,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers import device_registry as dr

from .const import DOMAIN

_LOGGER = logging.getLogger(__name__)

PLATFORMS = [Platform.BINARY_SENSOR, Platform.CAMERA]


@dataclass
class HikvisionData:
    """Data class for Hikvision runtime data."""

    camera: HikCamera
    device_id: str
    device_name: str
    device_type: str
    host: str
    username: str
    password: str
    channels: dict[int, VideoChannel] = field(default_factory=dict)


type HikvisionConfigEntry = ConfigEntry[HikvisionData]


async def async_setup_entry(hass: HomeAssistant, entry: HikvisionConfigEntry) -> bool:
    """Set up Hikvision from a config entry."""
    host = entry.data[CONF_HOST]
    port = entry.data[CONF_PORT]
    username = entry.data[CONF_USERNAME]
    password = entry.data[CONF_PASSWORD]
    ssl = entry.data[CONF_SSL]

    protocol = "https" if ssl else "http"
    url = f"{protocol}://{host}"

    try:
        camera = await hass.async_add_executor_job(
            HikCamera, url, port, username, password, ssl
        )
    except requests.exceptions.RequestException as err: