diff --git a/homeassistant/components/aurora_abb_powerone/__init__.py b/homeassistant/components/aurora_abb_powerone/__init__.py index 2c3d0c546cd..21724acaff6 100644 --- a/homeassistant/components/aurora_abb_powerone/__init__.py +++ b/homeassistant/components/aurora_abb_powerone/__init__.py @@ -25,12 +25,12 @@ PLATFORMS = ["sensor"] _LOGGER = logging.getLogger(__name__) -async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): +async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Aurora ABB PowerOne from a config entry.""" comport = entry.data[CONF_PORT] address = entry.data[CONF_ADDRESS] - serclient = AuroraSerialClient(address, comport, parity="N", timeout=1) + ser_client = AuroraSerialClient(address, comport, parity="N", timeout=1) # To handle yaml import attempts in darkeness, (re)try connecting only if # unique_id not yet assigned. if entry.unique_id is None: @@ -67,19 +67,19 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): return False hass.config_entries.async_update_entry(entry, unique_id=new_id) - hass.data.setdefault(DOMAIN, {})[entry.unique_id] = serclient + hass.data.setdefault(DOMAIN, {})[entry.entry_id] = ser_client hass.config_entries.async_setup_platforms(entry, PLATFORMS) return True -async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): +async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) # It should not be necessary to close the serial port because we close # it after every use in sensor.py, i.e. no need to do entry["client"].close() if unload_ok: - hass.data[DOMAIN].pop(entry.unique_id) + hass.data[DOMAIN].pop(entry.entry_id) return unload_ok diff --git a/homeassistant/components/aurora_abb_powerone/aurora_device.py b/homeassistant/components/aurora_abb_powerone/aurora_device.py index d2aed5ec7a8..d9cfb744231 100644 --- a/homeassistant/components/aurora_abb_powerone/aurora_device.py +++ b/homeassistant/components/aurora_abb_powerone/aurora_device.py @@ -1,11 +1,13 @@ """Top level class for AuroraABBPowerOneSolarPV inverters and sensors.""" from __future__ import annotations +from collections.abc import Mapping import logging +from typing import Any from aurorapy.client import AuroraSerialClient -from homeassistant.helpers.entity import Entity +from homeassistant.helpers.entity import DeviceInfo, Entity from .const import ( ATTR_DEVICE_NAME, @@ -20,10 +22,10 @@ from .const import ( _LOGGER = logging.getLogger(__name__) -class AuroraDevice(Entity): +class AuroraEntity(Entity): """Representation of an Aurora ABB PowerOne device.""" - def __init__(self, client: AuroraSerialClient, data) -> None: + def __init__(self, client: AuroraSerialClient, data: Mapping[str, Any]) -> None: """Initialise the basic device.""" self._data = data self.type = "device" @@ -44,7 +46,7 @@ class AuroraDevice(Entity): return self._available @property - def device_info(self): + def device_info(self) -> DeviceInfo: """Return device specific attributes.""" return { "identifiers": {(DOMAIN, self._data[ATTR_SERIAL_NUMBER])}, diff --git a/homeassistant/components/aurora_abb_powerone/config_flow.py b/homeassistant/components/aurora_abb_powerone/config_flow.py index 012fe7b14bb..3d292857266 100644 --- a/homeassistant/components/aurora_abb_powerone/config_flow.py +++ b/homeassistant/components/aurora_abb_powerone/config_flow.py @@ -1,5 +1,8 @@ """Config flow for Aurora ABB PowerOne integration.""" +from __future__ import annotations + import logging +from typing import Any from aurorapy.client import AuroraError, AuroraSerialClient import serial.tools.list_ports @@ -7,6 +10,7 @@ import voluptuous as vol from homeassistant import config_entries, core from homeassistant.const import CONF_ADDRESS, CONF_PORT +from homeassistant.data_entry_flow import FlowResult from .const import ( ATTR_FIRMWARE, @@ -22,7 +26,9 @@ from .const import ( _LOGGER = logging.getLogger(__name__) -def validate_and_connect(hass: core.HomeAssistant, data): +def validate_and_connect( + hass: core.HomeAssistant, data: dict[str, Any] +) -> dict[str, str]: """Validate the user input allows us to connect. Data has the keys from DATA_SCHEMA with values provided by the user. @@ -50,15 +56,15 @@ def validate_and_connect(hass: core.HomeAssistant, data): return ret -def scan_comports(): +def scan_comports() -> tuple[list[str] | None, str | None]: """Find and store available com ports for the GUI dropdown.""" - comports = serial.tools.list_ports.comports(include_links=True) - comportslist = [] - for port in comports: - comportslist.append(port.device) + com_ports = serial.tools.list_ports.comports(include_links=True) + com_ports_list = [] + for port in com_ports: + com_ports_list.append(port.device) _LOGGER.debug("COM port option: %s", port.device) - if len(comportslist) > 0: - return comportslist, comportslist[0] + if len(com_ports_list) > 0: + return com_ports_list, com_ports_list[0] _LOGGER.warning("No com ports found. Need a valid RS485 device to communicate") return None, None @@ -67,18 +73,17 @@ class AuroraABBConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for Aurora ABB PowerOne.""" VERSION = 1 - CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL def __init__(self): """Initialise the config flow.""" self.config = None - self._comportslist = None - self._defaultcomport = None + self._com_ports_list = None + self._default_com_port = None - async def async_step_import(self, config: dict): + async def async_step_import(self, config: dict[str, Any]) -> FlowResult: """Import a configuration from config.yaml.""" if self.hass.config_entries.async_entries(DOMAIN): - return self.async_abort(reason="already_setup") + return self.async_abort(reason="already_configured") conf = {} conf[CONF_PORT] = config["device"] @@ -87,14 +92,16 @@ class AuroraABBConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_create_entry(title=DEFAULT_INTEGRATION_TITLE, data=conf) - async def async_step_user(self, user_input=None): + async def async_step_user( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: """Handle a flow initialised by the user.""" errors = {} - if self._comportslist is None: + if self._com_ports_list is None: result = await self.hass.async_add_executor_job(scan_comports) - self._comportslist, self._defaultcomport = result - if self._defaultcomport is None: + self._com_ports_list, self._default_com_port = result + if self._default_com_port is None: return self.async_abort(reason="no_serial_ports") # Handle the initial step. @@ -103,14 +110,6 @@ class AuroraABBConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): info = await self.hass.async_add_executor_job( validate_and_connect, self.hass, user_input ) - info.update(user_input) - # Bomb out early if someone has already set up this device. - device_unique_id = info["serial_number"] - await self.async_set_unique_id(device_unique_id) - self._abort_if_unique_id_configured() - - return self.async_create_entry(title=info["title"], data=info) - except OSError as error: if error.errno == 19: # No such device. errors["base"] = "invalid_serial_port" @@ -127,10 +126,18 @@ class AuroraABBConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): error, ) errors["base"] = "cannot_connect" + else: + info.update(user_input) + # Bomb out early if someone has already set up this device. + device_unique_id = info["serial_number"] + await self.async_set_unique_id(device_unique_id) + self._abort_if_unique_id_configured() + return self.async_create_entry(title=info["title"], data=info) + # If no user input, must be first pass through the config. Show initial form. config_options = { - vol.Required(CONF_PORT, default=self._defaultcomport): vol.In( - self._comportslist + vol.Required(CONF_PORT, default=self._default_com_port): vol.In( + self._com_ports_list ), vol.Required(CONF_ADDRESS, default=DEFAULT_ADDRESS): vol.In( range(MIN_ADDRESS, MAX_ADDRESS + 1) diff --git a/homeassistant/components/aurora_abb_powerone/sensor.py b/homeassistant/components/aurora_abb_powerone/sensor.py index 4f196c39630..35be4e2b7d7 100644 --- a/homeassistant/components/aurora_abb_powerone/sensor.py +++ b/homeassistant/components/aurora_abb_powerone/sensor.py @@ -29,7 +29,7 @@ from homeassistant.const import ( ) import homeassistant.helpers.config_validation as cv -from .aurora_device import AuroraDevice +from .aurora_device import AuroraEntity from .const import DEFAULT_ADDRESS, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -84,7 +84,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities) -> None: """Set up aurora_abb_powerone sensor based on a config entry.""" entities = [] - client = hass.data[DOMAIN][config_entry.unique_id] + client = hass.data[DOMAIN][config_entry.entry_id] data = config_entry.data for sens in SENSOR_TYPES: @@ -94,7 +94,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities) -> None: async_add_entities(entities, True) -class AuroraSensor(AuroraDevice, SensorEntity): +class AuroraSensor(AuroraEntity, SensorEntity): """Representation of a Sensor on a Aurora ABB PowerOne Solar inverter.""" def __init__( @@ -106,7 +106,7 @@ class AuroraSensor(AuroraDevice, SensorEntity): """Initialize the sensor.""" super().__init__(client, data) self.entity_description = entity_description - self.availableprev = True + self.available_prev = True def update(self): """Fetch new state data for the sensor. @@ -114,7 +114,7 @@ class AuroraSensor(AuroraDevice, SensorEntity): This is the only method that should fetch new data for Home Assistant. """ try: - self.availableprev = self._attr_available + self.available_prev = self._attr_available self.client.connect() if self.entity_description.key == "instantaneouspower": # read ADC channel 3 (grid power output) @@ -145,7 +145,7 @@ class AuroraSensor(AuroraDevice, SensorEntity): else: raise error finally: - if self._attr_available != self.availableprev: + if self._attr_available != self.available_prev: if self._attr_available: _LOGGER.info("Communication with %s back online", self.name) else: diff --git a/homeassistant/components/aurora_abb_powerone/strings.json b/homeassistant/components/aurora_abb_powerone/strings.json index b705c5f69a5..bed403bd641 100644 --- a/homeassistant/components/aurora_abb_powerone/strings.json +++ b/homeassistant/components/aurora_abb_powerone/strings.json @@ -12,11 +12,10 @@ "error": { "cannot_connect": "Unable to connect, please check serial port, address, electrical connection and that inverter is on (in daylight)", "invalid_serial_port": "Serial port is not a valid device or could not be openned", - "cannot_open_serial_port": "Cannot open serial port, please check and try again", - "unknown": "[%key:common::config_flow::error::unknown%]" + "cannot_open_serial_port": "Cannot open serial port, please check and try again" }, "abort": { - "already_configured": "Device is already configured", + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", "no_serial_ports": "No com ports found. Need a valid RS485 device to communicate." } } diff --git a/tests/components/aurora_abb_powerone/test_config_flow.py b/tests/components/aurora_abb_powerone/test_config_flow.py index 620d7e10e53..e9a6a100f4a 100644 --- a/tests/components/aurora_abb_powerone/test_config_flow.py +++ b/tests/components/aurora_abb_powerone/test_config_flow.py @@ -1,5 +1,6 @@ """Test the Aurora ABB PowerOne Solar PV config flow.""" from datetime import timedelta +import logging from logging import INFO from unittest.mock import patch @@ -17,7 +18,9 @@ from homeassistant.config_entries import ConfigEntryState from homeassistant.const import CONF_ADDRESS, CONF_PORT from homeassistant.util.dt import utcnow -from tests.common import async_fire_time_changed +from tests.common import MockConfigEntry, async_fire_time_changed + +TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"} def _simulated_returns(index, global_measure=None): @@ -163,10 +166,60 @@ async def test_form_invalid_com_ports(hass): assert len(mock_clientclose.mock_calls) == 1 +async def test_import_invalid_com_ports(hass, caplog): + """Test we display correct info when the comport is invalid..""" + + caplog.set_level(logging.ERROR) + with patch( + "aurorapy.client.AuroraSerialClient.connect", + side_effect=OSError(19, "...no such device..."), + return_value=None, + ): + await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA + ) + configs = hass.config_entries.async_entries(DOMAIN) + assert len(configs) == 1 + entry = configs[0] + assert entry.state == ConfigEntryState.SETUP_ERROR + assert "Failed to connect to inverter: " in caplog.text + + +async def test_import_com_port_wont_open(hass): + """Test we display correct info when comport won't open.""" + + with patch( + "aurorapy.client.AuroraSerialClient.connect", + side_effect=AuroraError("..could not open port..."), + ): + await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA + ) + configs = hass.config_entries.async_entries(DOMAIN) + assert len(configs) == 1 + entry = configs[0] + assert entry.state == ConfigEntryState.SETUP_ERROR + + +async def test_import_other_oserror(hass): + """Test we display correct info when comport won't open.""" + + with patch( + "aurorapy.client.AuroraSerialClient.connect", + side_effect=OSError(18, "...another error..."), + ): + await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TEST_DATA + ) + configs = hass.config_entries.async_entries(DOMAIN) + assert len(configs) == 1 + entry = configs[0] + assert entry.state == ConfigEntryState.SETUP_ERROR + + # Tests below can be deleted after deprecation period is finished. async def test_import_day(hass): """Test .yaml import when the inverter is able to communicate.""" - TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"} with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None,), patch( "aurorapy.client.AuroraSerialClient.serial_number", @@ -195,7 +248,6 @@ async def test_import_day(hass): async def test_import_night(hass): """Test .yaml import when the inverter is inaccessible (e.g. darkness).""" - TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"} # First time round, no response. with patch( @@ -241,13 +293,14 @@ async def test_import_night(hass): assert entry.unique_id assert len(mock_connect.mock_calls) == 1 - assert hass.states.get("sensor.power_output").state == "45.7" + power = hass.states.get("sensor.power_output") + assert power + assert power.state == "45.7" assert len(hass.config_entries.async_entries(DOMAIN)) == 1 async def test_import_night_then_user(hass): """Attempt yaml import and fail (dark), but user sets up manually before auto retry.""" - TEST_DATA = {"device": "/dev/ttyUSB7", "address": 3, "name": "MyAuroraPV"} # First time round, no response. with patch( @@ -322,3 +375,29 @@ async def test_import_night_then_user(hass): await hass.async_block_till_done() assert entry.state == ConfigEntryState.NOT_LOADED assert len(hass.config_entries.async_entries(DOMAIN)) == 1 + + +async def test_import_already_existing(hass): + """Test configuration.yaml import when already configured.""" + TESTDATA = {"device": "/dev/ttyUSB7", "address": 7, "name": "MyAuroraPV"} + + entry = MockConfigEntry( + domain=DOMAIN, + title="MyAuroraPV", + unique_id="0123456", + data={ + CONF_PORT: "/dev/ttyUSB7", + CONF_ADDRESS: 7, + ATTR_FIRMWARE: "1.234", + ATTR_MODEL: "9.8.7.6 (A.B.C)", + ATTR_SERIAL_NUMBER: "9876543", + "title": "PhotoVoltaic Inverters", + }, + ) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_IMPORT}, data=TESTDATA + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" diff --git a/tests/components/aurora_abb_powerone/test_sensor.py b/tests/components/aurora_abb_powerone/test_sensor.py index c1d1d4ad5c8..0a7b7e33302 100644 --- a/tests/components/aurora_abb_powerone/test_sensor.py +++ b/tests/components/aurora_abb_powerone/test_sensor.py @@ -12,16 +12,10 @@ from homeassistant.components.aurora_abb_powerone.const import ( DEFAULT_INTEGRATION_TITLE, DOMAIN, ) -from homeassistant.config_entries import SOURCE_IMPORT from homeassistant.const import CONF_ADDRESS, CONF_PORT -from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util -from tests.common import ( - MockConfigEntry, - assert_setup_component, - async_fire_time_changed, -) +from tests.common import MockConfigEntry, async_fire_time_changed TEST_CONFIG = { "sensor": { @@ -56,49 +50,10 @@ def _mock_config_entry(): }, source="dummysource", entry_id="13579", + unique_id="654321", ) -async def test_setup_platform_valid_config(hass): - """Test that (deprecated) yaml import still works.""" - with patch("aurorapy.client.AuroraSerialClient.connect", return_value=None), patch( - "aurorapy.client.AuroraSerialClient.measure", - side_effect=_simulated_returns, - ), patch( - "aurorapy.client.AuroraSerialClient.serial_number", - return_value="9876543", - ), patch( - "aurorapy.client.AuroraSerialClient.version", - return_value="9.8.7.6", - ), patch( - "aurorapy.client.AuroraSerialClient.pn", - return_value="A.B.C", - ), patch( - "aurorapy.client.AuroraSerialClient.firmware", - return_value="1.234", - ), patch( - "aurorapy.client.AuroraSerialClient.cumulated_energy", - side_effect=_simulated_returns, - ), assert_setup_component( - 1, "sensor" - ): - assert await async_setup_component(hass, "sensor", TEST_CONFIG) - await hass.async_block_till_done() - power = hass.states.get("sensor.power_output") - assert power - assert power.state == "45.7" - - # try to set up a second time - should abort. - - result = await hass.config_entries.flow.async_init( - DOMAIN, - data=TEST_CONFIG, - context={"source": SOURCE_IMPORT}, - ) - assert result["type"] == "abort" - assert result["reason"] == "already_setup" - - async def test_sensors(hass): """Test data coming back from inverter.""" mock_entry = _mock_config_entry()