Fivem code quality improvements (#66086)

* specify config type

* move coordinator outside try block

* rename gamename to game_name

* remove log in __init__

* Remove logging and minify update

* Add types to parameters

* Remove name from device

* Remove update listener

* Remove status icon

* Dont allow duplicate entries

* Use default translation string

* Remove online and port from coordinator

homeassistant/components/fivem/__init__.py

@@ -10,7 +10,7 @@ from typing import Any
 from fivem import FiveM, FiveMServerOfflineError
 
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, Platform
+from homeassistant.const import CONF_HOST, CONF_PORT, Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.entity import DeviceInfo, EntityDescription
@@ -45,14 +45,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         entry.data[CONF_PORT],
     )
 
-    try:
     coordinator = FiveMDataUpdateCoordinator(hass, entry.data, entry.entry_id)
+
+    try:
         await coordinator.initialize()
     except FiveMServerOfflineError as err:
         raise ConfigEntryNotReady from err
 
     await coordinator.async_config_entry_first_refresh()
-    entry.async_on_unload(entry.add_update_listener(update_listener))
 
     hass.data.setdefault(DOMAIN, {})[entry.entry_id] = coordinator
 
@@ -69,32 +69,23 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     return unload_ok
 
 
-async def update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None:
-    """Update listener."""
-    await hass.config_entries.async_reload(entry.entry_id)
-
-
 class FiveMDataUpdateCoordinator(DataUpdateCoordinator[dict[str, Any]]):
     """Class to manage fetching FiveM data."""
 
-    def __init__(self, hass: HomeAssistant, config_data, unique_id: str) -> None:
+    def __init__(
+        self, hass: HomeAssistant, config_data: Mapping[str, Any], unique_id: str
+    ) -> None:
         """Initialize server instance."""
-        self._hass = hass
-
         self.unique_id = unique_id
         self.server = None
         self.version = None
-        self.gamename: str | None = None
+        self.game_name: str | None = None
 
-        self.server_name = config_data[CONF_NAME]
         self.host = config_data[CONF_HOST]
-        self.port = config_data[CONF_PORT]
-        self.online = False
 
-        self._fivem = FiveM(self.host, self.port)
+        self._fivem = FiveM(self.host, config_data[CONF_PORT])
 
         update_interval = timedelta(seconds=SCAN_INTERVAL)
-        _LOGGER.debug("Data will be updated every %s", update_interval)
 
         super().__init__(hass, _LOGGER, name=DOMAIN, update_interval=update_interval)
 
@@ -103,24 +94,15 @@ class FiveMDataUpdateCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         info = await self._fivem.get_info_raw()
         self.server = info["server"]
         self.version = info["version"]
-        self.gamename = info["vars"]["gamename"]
+        self.game_name = info["vars"]["gamename"]
 
     async def _async_update_data(self) -> dict[str, Any]:
         """Get server data from 3rd party library and update properties."""
-        was_online = self.online
-
         try:
             server = await self._fivem.get_server()
-            self.online = True
+        except FiveMServerOfflineError as err:
-        except FiveMServerOfflineError:
+            raise UpdateFailed from err
-            self.online = False
 
-        if was_online and not self.online:
-            _LOGGER.warning("Connection to '%s:%s' lost", self.host, self.port)
-        elif not was_online and self.online:
-            _LOGGER.info("Connection to '%s:%s' (re-)established", self.host, self.port)
-
-        if self.online:
         players_list: list[str] = []
         for player in server.players:
             players_list.append(player.name)
@@ -133,13 +115,11 @@ class FiveMDataUpdateCoordinator(DataUpdateCoordinator[dict[str, Any]]):
             NAME_PLAYERS_ONLINE: len(players_list),
             NAME_PLAYERS_MAX: server.max_players,
             NAME_RESOURCES: len(resources_list),
-                NAME_STATUS: self.online,
+            NAME_STATUS: self.last_update_success,
             ATTR_PLAYERS_LIST: players_list,
             ATTR_RESOURCES_LIST: resources_list,
         }
 
-        raise UpdateFailed
-
 
 @dataclass
 class FiveMEntityDescription(EntityDescription):
@@ -163,13 +143,13 @@ class FiveMEntity(CoordinatorEntity):
         super().__init__(coordinator)
         self.entity_description = description
 
-        self._attr_name = f"{self.coordinator.server_name} {description.name}"
+        self._attr_name = f"{self.coordinator.host} {description.name}"
         self._attr_unique_id = f"{self.coordinator.unique_id}-{description.key}".lower()
         self._attr_device_info = DeviceInfo(
             identifiers={(DOMAIN, self.coordinator.unique_id)},
             manufacturer=MANUFACTURER,
             model=self.coordinator.server,
-            name=self.coordinator.server_name,
+            name=self.coordinator.host,
             sw_version=self.coordinator.version,
         )
 

homeassistant/components/fivem/binary_sensor.py

@@ -9,7 +9,7 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
 from . import FiveMEntity, FiveMEntityDescription
-from .const import DOMAIN, ICON_STATUS, NAME_STATUS
+from .const import DOMAIN, NAME_STATUS
 
 
 class FiveMBinarySensorEntityDescription(
@@ -22,7 +22,6 @@ BINARY_SENSORS: tuple[FiveMBinarySensorEntityDescription, ...] = (
     FiveMBinarySensorEntityDescription(
         key=NAME_STATUS,
         name=NAME_STATUS,
-        icon=ICON_STATUS,
         device_class=BinarySensorDeviceClass.CONNECTIVITY,
     ),
 )

homeassistant/components/fivem/config_flow.py

@@ -8,8 +8,7 @@ from fivem import FiveM, FiveMServerOfflineError
 import voluptuous as vol
 
 from homeassistant import config_entries
-from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT
+from homeassistant.const import CONF_HOST, CONF_PORT
-from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResult
 import homeassistant.helpers.config_validation as cv
 
@@ -21,22 +20,21 @@ DEFAULT_PORT = 30120
 
 STEP_USER_DATA_SCHEMA = vol.Schema(
     {
-        vol.Required(CONF_NAME): cv.string,
         vol.Required(CONF_HOST): cv.string,
         vol.Optional(CONF_PORT, default=DEFAULT_PORT): cv.port,
     }
 )
 
 
-async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> None:
+async def validate_input(data: dict[str, Any]) -> None:
     """Validate the user input allows us to connect."""
 
     fivem = FiveM(data[CONF_HOST], data[CONF_PORT])
     info = await fivem.get_info_raw()
 
-    gamename = info.get("vars")["gamename"]
+    game_name = info.get("vars")["gamename"]
-    if gamename is None or gamename != "gta5":
+    if game_name is None or game_name != "gta5":
-        raise InvalidGamenameError
+        raise InvalidGameNameError
 
 
 class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
@@ -56,21 +54,22 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         errors = {}
 
         try:
-            await validate_input(self.hass, user_input)
+            await validate_input(user_input)
         except FiveMServerOfflineError:
             errors["base"] = "cannot_connect"
-        except InvalidGamenameError:
+        except InvalidGameNameError:
-            errors["base"] = "invalid_gamename"
+            errors["base"] = "invalid_game_name"
         except Exception:  # pylint: disable=broad-except
             _LOGGER.exception("Unexpected exception")
             errors["base"] = "unknown"
         else:
-            return self.async_create_entry(title=user_input[CONF_NAME], data=user_input)
+            self._async_abort_entries_match(user_input)
+            return self.async_create_entry(title=user_input[CONF_HOST], data=user_input)
 
         return self.async_show_form(
             step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors
         )
 
 
-class InvalidGamenameError(Exception):
+class InvalidGameNameError(Exception):
     """Handle errors in the game name from the api."""

homeassistant/components/fivem/const.py

@@ -8,7 +8,6 @@ DOMAIN = "fivem"
 ICON_PLAYERS_MAX = "mdi:account-multiple"
 ICON_PLAYERS_ONLINE = "mdi:account-multiple"
 ICON_RESOURCES = "mdi:playlist-check"
-ICON_STATUS = "mdi:lan"
 
 MANUFACTURER = "Cfx.re"
 

homeassistant/components/fivem/strings.json

@@ -11,10 +11,11 @@
     },
     "error": {
       "cannot_connect": "Failed to connect. Please check the host and port and try again. Also ensure that you are running the latest FiveM server.",
-      "invalid_gamename": "The api of the game you are trying to connect to is not a FiveM game."
+      "invalid_game_name": "The api of the game you are trying to connect to is not a FiveM game.",
+      "unknown_error": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {
-      "already_configured": "FiveM server is already configured"
+      "already_configured": "[%key:common::config_flow::abort::already_configured_service%]"
     }
   }
 }

homeassistant/components/fivem/translations/en.json

@@ -1,11 +1,12 @@
 {
     "config": {
         "abort": {
-            "already_configured": "FiveM server is already configured"
+            "already_configured": "Service is already configured"
         },
         "error": {
             "cannot_connect": "Failed to connect. Please check the host and port and try again. Also ensure that you are running the latest FiveM server.",
-            "invalid_gamename": "The api of the game you are trying to connect to is not a FiveM game."
+            "invalid_game_name": "The api of the game you are trying to connect to is not a FiveM game.",
+            "unknown_error": "Unexpected error"
         },
         "step": {
             "user": {

tests/components/fivem/test_config_flow.py

@@ -6,18 +6,17 @@ from fivem import FiveMServerOfflineError
 from homeassistant import config_entries
 from homeassistant.components.fivem.config_flow import DEFAULT_PORT
 from homeassistant.components.fivem.const import DOMAIN
-from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT
+from homeassistant.const import CONF_HOST, CONF_PORT
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import RESULT_TYPE_CREATE_ENTRY, RESULT_TYPE_FORM
 
 USER_INPUT = {
-    CONF_NAME: "Dummy Server",
     CONF_HOST: "fivem.dummyserver.com",
     CONF_PORT: DEFAULT_PORT,
 }
 
 
-def __mock_fivem_info_success():
+def _mock_fivem_info_success():
     return {
         "resources": [
             "fivem",
@@ -31,7 +30,7 @@ def __mock_fivem_info_success():
     }
 
 
-def __mock_fivem_info_invalid():
+def _mock_fivem_info_invalid():
     return {
         "plugins": [
             "sample",
@@ -42,8 +41,8 @@ def __mock_fivem_info_invalid():
     }
 
 
-def __mock_fivem_info_invalid_gamename():
+def _mock_fivem_info_invalid_game_name():
-    info = __mock_fivem_info_success()
+    info = _mock_fivem_info_success()
     info["vars"]["gamename"] = "redm"
 
     return info
@@ -69,7 +68,7 @@ async def test_form(hass: HomeAssistant) -> None:
 
     with patch(
         "fivem.fivem.FiveM.get_info_raw",
-        return_value=__mock_fivem_info_success(),
+        return_value=_mock_fivem_info_success(),
     ), patch(
         "homeassistant.components.fivem.async_setup_entry",
         return_value=True,
@@ -81,7 +80,7 @@ async def test_form(hass: HomeAssistant) -> None:
         await hass.async_block_till_done()
 
     assert result2["type"] == RESULT_TYPE_CREATE_ENTRY
-    assert result2["title"] == USER_INPUT[CONF_NAME]
+    assert result2["title"] == USER_INPUT[CONF_HOST]
     assert result2["data"] == USER_INPUT
     assert len(mock_setup_entry.mock_calls) == 1
 
@@ -114,7 +113,7 @@ async def test_form_invalid(hass: HomeAssistant) -> None:
 
     with patch(
         "fivem.fivem.FiveM.get_info_raw",
-        return_value=__mock_fivem_info_invalid(),
+        return_value=_mock_fivem_info_invalid(),
     ):
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -126,7 +125,7 @@ async def test_form_invalid(hass: HomeAssistant) -> None:
     assert result2["errors"] == {"base": "unknown"}
 
 
-async def test_form_invalid_gamename(hass: HomeAssistant) -> None:
+async def test_form_invalid_game_name(hass: HomeAssistant) -> None:
     """Test we get the form."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
@@ -134,7 +133,7 @@ async def test_form_invalid_gamename(hass: HomeAssistant) -> None:
 
     with patch(
         "fivem.fivem.FiveM.get_info_raw",
-        return_value=__mock_fivem_info_invalid_gamename(),
+        return_value=_mock_fivem_info_invalid_game_name(),
     ):
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -143,4 +142,4 @@ async def test_form_invalid_gamename(hass: HomeAssistant) -> None:
         await hass.async_block_till_done()
 
     assert result2["type"] == RESULT_TYPE_FORM
-    assert result2["errors"] == {"base": "invalid_gamename"}
+    assert result2["errors"] == {"base": "invalid_game_name"}