From 557b6d511bea82537eafc1f8c0806760e0c49102 Mon Sep 17 00:00:00 2001 From: elmurato <1382097+elmurato@users.noreply.github.com> Date: Mon, 24 Jul 2023 20:23:11 +0200 Subject: [PATCH] Improve reading of MOTD and bump mcstatus to 11.0.0 (#95715) * Improve reading of MOTD, bump mcstatus to 10.0.3 and getmac to 0.9.4 * Revert bump of getmac * Bump mcstatus to 11.0.0-rc3. Use new MOTD parser. * Bump mcstatus to 11.0.0 --- .../components/minecraft_server/__init__.py | 34 +++++++---------- .../minecraft_server/config_flow.py | 5 ++- .../components/minecraft_server/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- .../minecraft_server/test_config_flow.py | 38 ++++++++++++------- 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/homeassistant/components/minecraft_server/__init__.py b/homeassistant/components/minecraft_server/__init__.py index 801b27ee971..da897a9767f 100644 --- a/homeassistant/components/minecraft_server/__init__.py +++ b/homeassistant/components/minecraft_server/__init__.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta import logging from typing import Any -from mcstatus.server import MinecraftServer as MCStatus +from mcstatus.server import JavaServer from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT, Platform @@ -69,9 +69,6 @@ async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> class MinecraftServer: """Representation of a Minecraft server.""" - # Private constants - _MAX_RETRIES_STATUS = 3 - def __init__( self, hass: HomeAssistant, unique_id: str, config_data: Mapping[str, Any] ) -> None: @@ -88,16 +85,16 @@ class MinecraftServer: self.srv_record_checked = False # 3rd party library instance - self._mc_status = MCStatus(self.host, self.port) + self._server = JavaServer(self.host, self.port) # Data provided by 3rd party library - self.version = None - self.protocol_version = None - self.latency_time = None - self.players_online = None - self.players_max = None + self.version: str | None = None + self.protocol_version: int | None = None + self.latency_time: float | None = None + self.players_online: int | None = None + self.players_max: int | None = None self.players_list: list[str] | None = None - self.motd = None + self.motd: str | None = None # Dispatcher signal name self.signal_name = f"{SIGNAL_NAME_PREFIX}_{self.unique_id}" @@ -133,13 +130,11 @@ class MinecraftServer: # with data extracted out of SRV record. self.host = srv_record[CONF_HOST] self.port = srv_record[CONF_PORT] - self._mc_status = MCStatus(self.host, self.port) + self._server = JavaServer(self.host, self.port) # Ping the server with a status request. try: - await self._hass.async_add_executor_job( - self._mc_status.status, self._MAX_RETRIES_STATUS - ) + await self._server.async_status() self.online = True except OSError as error: _LOGGER.debug( @@ -176,9 +171,7 @@ class MinecraftServer: async def _async_status_request(self) -> None: """Request server status and update properties.""" try: - status_response = await self._hass.async_add_executor_job( - self._mc_status.status, self._MAX_RETRIES_STATUS - ) + status_response = await self._server.async_status() # Got answer to request, update properties. self.version = status_response.version.name @@ -186,7 +179,8 @@ class MinecraftServer: self.players_online = status_response.players.online self.players_max = status_response.players.max self.latency_time = status_response.latency - self.motd = (status_response.description).get("text") + self.motd = status_response.motd.to_plain() + self.players_list = [] if status_response.players.sample is not None: for player in status_response.players.sample: @@ -244,7 +238,7 @@ class MinecraftServerEntity(Entity): manufacturer=MANUFACTURER, model=f"Minecraft Server ({self._server.version})", name=self._server.name, - sw_version=self._server.protocol_version, + sw_version=str(self._server.protocol_version), ) self._attr_device_class = device_class self._extra_state_attributes = None diff --git a/homeassistant/components/minecraft_server/config_flow.py b/homeassistant/components/minecraft_server/config_flow.py index 691aea0f75e..b402b7cfff0 100644 --- a/homeassistant/components/minecraft_server/config_flow.py +++ b/homeassistant/components/minecraft_server/config_flow.py @@ -8,6 +8,7 @@ import voluptuous as vol from homeassistant.config_entries import ConfigFlow from homeassistant.const import CONF_HOST, CONF_NAME, CONF_PORT +from homeassistant.data_entry_flow import FlowResult from . import MinecraftServer, helpers from .const import DEFAULT_HOST, DEFAULT_NAME, DEFAULT_PORT, DOMAIN @@ -18,7 +19,7 @@ class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN): VERSION = 1 - async def async_step_user(self, user_input=None): + async def async_step_user(self, user_input=None) -> FlowResult: """Handle the initial step.""" errors = {} @@ -117,7 +118,7 @@ class MinecraftServerConfigFlow(ConfigFlow, domain=DOMAIN): # form filled with user_input and eventually with errors otherwise). return self._show_config_form(user_input, errors) - def _show_config_form(self, user_input=None, errors=None): + def _show_config_form(self, user_input=None, errors=None) -> FlowResult: """Show the setup form to the user.""" if user_input is None: user_input = {} diff --git a/homeassistant/components/minecraft_server/manifest.json b/homeassistant/components/minecraft_server/manifest.json index b831e1eae90..27019cb80a8 100644 --- a/homeassistant/components/minecraft_server/manifest.json +++ b/homeassistant/components/minecraft_server/manifest.json @@ -7,5 +7,5 @@ "iot_class": "local_polling", "loggers": ["dnspython", "mcstatus"], "quality_scale": "silver", - "requirements": ["aiodns==3.0.0", "getmac==0.8.2", "mcstatus==6.0.0"] + "requirements": ["aiodns==3.0.0", "getmac==0.8.2", "mcstatus==11.0.0"] } diff --git a/requirements_all.txt b/requirements_all.txt index b15c4f97c88..394bf420796 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1175,7 +1175,7 @@ maxcube-api==0.4.3 mbddns==0.1.2 # homeassistant.components.minecraft_server -mcstatus==6.0.0 +mcstatus==11.0.0 # homeassistant.components.meater meater-python==0.0.8 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index a0b49ab41f0..2b84604df48 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -898,7 +898,7 @@ maxcube-api==0.4.3 mbddns==0.1.2 # homeassistant.components.minecraft_server -mcstatus==6.0.0 +mcstatus==11.0.0 # homeassistant.components.meater meater-python==0.0.8 diff --git a/tests/components/minecraft_server/test_config_flow.py b/tests/components/minecraft_server/test_config_flow.py index 25c0cec441b..ac5ae7dbc6e 100644 --- a/tests/components/minecraft_server/test_config_flow.py +++ b/tests/components/minecraft_server/test_config_flow.py @@ -4,7 +4,7 @@ import asyncio from unittest.mock import patch import aiodns -from mcstatus.pinger import PingResponse +from mcstatus.status_response import JavaStatusResponse from homeassistant.components.minecraft_server.const import ( DEFAULT_NAME, @@ -22,7 +22,7 @@ from tests.common import MockConfigEntry class QueryMock: """Mock for result of aiodns.DNSResolver.query.""" - def __init__(self): + def __init__(self) -> None: """Set up query result mock.""" self.host = "mc.dummyserver.com" self.port = 23456 @@ -31,7 +31,7 @@ class QueryMock: self.ttl = None -STATUS_RESPONSE_RAW = { +JAVA_STATUS_RESPONSE_RAW = { "description": {"text": "Dummy Description"}, "version": {"name": "Dummy Version", "protocol": 123}, "players": { @@ -103,8 +103,10 @@ async def test_same_host(hass: HomeAssistant) -> None: "aiodns.DNSResolver.query", side_effect=aiodns.error.DNSError, ), patch( - "mcstatus.server.MinecraftServer.status", - return_value=PingResponse(STATUS_RESPONSE_RAW), + "mcstatus.server.JavaServer.async_status", + return_value=JavaStatusResponse( + None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None + ), ): unique_id = "mc.dummyserver.com-25565" config_data = { @@ -158,7 +160,7 @@ async def test_connection_failed(hass: HomeAssistant) -> None: with patch( "aiodns.DNSResolver.query", side_effect=aiodns.error.DNSError, - ), patch("mcstatus.server.MinecraftServer.status", side_effect=OSError): + ), patch("mcstatus.server.JavaServer.async_status", side_effect=OSError): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT ) @@ -173,8 +175,10 @@ async def test_connection_succeeded_with_srv_record(hass: HomeAssistant) -> None "aiodns.DNSResolver.query", return_value=SRV_RECORDS, ), patch( - "mcstatus.server.MinecraftServer.status", - return_value=PingResponse(STATUS_RESPONSE_RAW), + "mcstatus.server.JavaServer.async_status", + return_value=JavaStatusResponse( + None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None + ), ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT_SRV @@ -192,8 +196,10 @@ async def test_connection_succeeded_with_host(hass: HomeAssistant) -> None: "aiodns.DNSResolver.query", side_effect=aiodns.error.DNSError, ), patch( - "mcstatus.server.MinecraftServer.status", - return_value=PingResponse(STATUS_RESPONSE_RAW), + "mcstatus.server.JavaServer.async_status", + return_value=JavaStatusResponse( + None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None + ), ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT @@ -211,8 +217,10 @@ async def test_connection_succeeded_with_ip4(hass: HomeAssistant) -> None: "aiodns.DNSResolver.query", side_effect=aiodns.error.DNSError, ), patch( - "mcstatus.server.MinecraftServer.status", - return_value=PingResponse(STATUS_RESPONSE_RAW), + "mcstatus.server.JavaServer.async_status", + return_value=JavaStatusResponse( + None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None + ), ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT_IPV4 @@ -230,8 +238,10 @@ async def test_connection_succeeded_with_ip6(hass: HomeAssistant) -> None: "aiodns.DNSResolver.query", side_effect=aiodns.error.DNSError, ), patch( - "mcstatus.server.MinecraftServer.status", - return_value=PingResponse(STATUS_RESPONSE_RAW), + "mcstatus.server.JavaServer.async_status", + return_value=JavaStatusResponse( + None, None, None, None, JAVA_STATUS_RESPONSE_RAW, None + ), ): result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER}, data=USER_INPUT_IPV6