Improve stability of homekit media players (#35080)

Author: bdraco

homeassistant/components/homekit/__init__.py

@@ -275,13 +275,12 @@ def get_accessory(hass, driver, state, aid, config):
 
     elif state.domain == "media_player":
         device_class = state.attributes.get(ATTR_DEVICE_CLASS)
-        feature_list = config.get(CONF_FEATURE_LIST)
+        feature_list = config.get(CONF_FEATURE_LIST, [])
 
         if device_class == DEVICE_CLASS_TV:
             a_type = "TelevisionMediaPlayer"
-        else:
-            if feature_list and validate_media_player_features(state, feature_list):
-                a_type = "MediaPlayer"
+        elif validate_media_player_features(state, feature_list):
+            a_type = "MediaPlayer"
 
     elif state.domain == "sensor":
         device_class = state.attributes.get(ATTR_DEVICE_CLASS)

homeassistant/components/homekit/type_media_players.py

@@ -65,6 +65,7 @@
     SERV_TELEVISION,
     SERV_TELEVISION_SPEAKER,
 )
+from .util import get_media_player_features
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -84,10 +85,12 @@
     # 15: "Information",
 }
 
+# Names may not contain special characters
+# or emjoi (/ is a special character for Apple)
 MODE_FRIENDLY_NAME = {
     FEATURE_ON_OFF: "Power",
-    FEATURE_PLAY_PAUSE: "Play/Pause",
-    FEATURE_PLAY_STOP: "Play/Stop",
+    FEATURE_PLAY_PAUSE: "Play-Pause",
+    FEATURE_PLAY_STOP: "Play-Stop",
     FEATURE_TOGGLE_MUTE: "Mute",
 }
 
@@ -106,7 +109,9 @@ def __init__(self, *args):
             FEATURE_PLAY_STOP: None,
             FEATURE_TOGGLE_MUTE: None,
         }
-        feature_list = self.config[CONF_FEATURE_LIST]
+        feature_list = self.config.get(
+            CONF_FEATURE_LIST, get_media_player_features(state)
+        )
 
         if FEATURE_ON_OFF in feature_list:
             name = self.generate_service_name(FEATURE_ON_OFF)
@@ -214,7 +219,7 @@ def update_state(self, new_state):
                 self.chars[FEATURE_PLAY_STOP].set_value(hk_state)
 
         if self.chars[FEATURE_TOGGLE_MUTE]:
-            current_state = new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED)
+            current_state = bool(new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED))
             _LOGGER.debug(
                 '%s: Set current state for "toggle_mute" to %s',
                 self.entity_id,
@@ -240,9 +245,7 @@ def __init__(self, *args):
         # Add additional characteristics if volume or input selection supported
         self.chars_tv = []
         self.chars_speaker = []
-        features = self.hass.states.get(self.entity_id).attributes.get(
-            ATTR_SUPPORTED_FEATURES, 0
-        )
+        features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
 
         if features & (SUPPORT_PLAY | SUPPORT_PAUSE):
             self.chars_tv.append(CHAR_REMOTE_KEY)
@@ -253,7 +256,8 @@ def __init__(self, *args):
             if features & SUPPORT_VOLUME_SET:
                 self.chars_speaker.append(CHAR_VOLUME)
 
-        if features & SUPPORT_SELECT_SOURCE:
+        source_list = state.attributes.get(ATTR_INPUT_SOURCE_LIST, [])
+        if source_list and features & SUPPORT_SELECT_SOURCE:
             self.support_select_source = True
 
         serv_tv = self.add_preload_service(SERV_TELEVISION, self.chars_tv)
@@ -298,9 +302,7 @@ def __init__(self, *args):
                 )
 
         if self.support_select_source:
-            self.sources = self.hass.states.get(self.entity_id).attributes.get(
-                ATTR_INPUT_SOURCE_LIST, []
-            )
+            self.sources = source_list
             self.char_input_source = serv_tv.configure_char(
                 CHAR_ACTIVE_IDENTIFIER, setter_callback=self.set_input_source
             )
@@ -380,35 +382,30 @@ def update_state(self, new_state):
         hk_state = 0
         if current_state not in ("None", STATE_OFF, STATE_UNKNOWN):
             hk_state = 1
-
         _LOGGER.debug("%s: Set current active state to %s", self.entity_id, hk_state)
         if self.char_active.value != hk_state:
             self.char_active.set_value(hk_state)
 
         # Set mute state
         if CHAR_VOLUME_SELECTOR in self.chars_speaker:
-            current_mute_state = new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED)
+            current_mute_state = bool(new_state.attributes.get(ATTR_MEDIA_VOLUME_MUTED))
             _LOGGER.debug(
                 "%s: Set current mute state to %s", self.entity_id, current_mute_state,
             )
             if self.char_mute.value != current_mute_state:
                 self.char_mute.set_value(current_mute_state)
 
         # Set active input
-        if self.support_select_source:
+        if self.support_select_source and self.sources:
             source_name = new_state.attributes.get(ATTR_INPUT_SOURCE)
-            if self.sources:
-                _LOGGER.debug(
-                    "%s: Set current input to %s", self.entity_id, source_name
+            _LOGGER.debug("%s: Set current input to %s", self.entity_id, source_name)
+            if source_name in self.sources:
+                index = self.sources.index(source_name)
+                if self.char_input_source.value != index:
+                    self.char_input_source.set_value(index)
+            else:
+                _LOGGER.warning(
+                    "%s: Sources out of sync. Restart Home Assistant", self.entity_id,
                 )
-                if source_name in self.sources:
-                    index = self.sources.index(source_name)
-                    if self.char_input_source.value != index:
-                        self.char_input_source.set_value(index)
-                else:
-                    _LOGGER.warning(
-                        "%s: Sources out of sync. Restart Home Assistant",
-                        self.entity_id,
-                    )
-                    if self.char_input_source.value != 0:
-                        self.char_input_source.set_value(0)
+                if self.char_input_source.value != 0:
+                    self.char_input_source.set_value(0)

homeassistant/components/homekit/util.py

@@ -96,6 +96,40 @@
 )
 
 
+HOMEKIT_CHAR_TRANSLATIONS = {
+    0: " ",  # nul
+    10: " ",  # nl
+    13: " ",  # cr
+    33: "-",  # !
+    34: " ",  # "
+    36: "-",  # $
+    37: "-",  # %
+    40: "-",  # (
+    41: "-",  # )
+    42: "-",  # *
+    43: "-",  # +
+    47: "-",  # /
+    58: "-",  # :
+    59: "-",  # ;
+    60: "-",  # <
+    61: "-",  # =
+    62: "-",  # >
+    63: "-",  # ?
+    64: "-",  # @
+    91: "-",  # [
+    92: "-",  # \
+    93: "-",  # ]
+    94: "-",  # ^
+    95: " ",  # _
+    96: "-",  # `
+    123: "-",  # {
+    124: "-",  # |
+    125: "-",  # }
+    126: "-",  # ~
+    127: "-",  # del
+}
+
+
 def validate_entity_config(values):
     """Validate config entry for CONF_ENTITY."""
     if not isinstance(values, dict):
@@ -133,8 +167,8 @@ def validate_entity_config(values):
     return entities
 
 
-def validate_media_player_features(state, feature_list):
-    """Validate features for media players."""
+def get_media_player_features(state):
+    """Determine features for media players."""
     features = state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
 
     supported_modes = []
@@ -148,14 +182,30 @@ def validate_media_player_features(state, feature_list):
         supported_modes.append(FEATURE_PLAY_STOP)
     if features & media_player.const.SUPPORT_VOLUME_MUTE:
         supported_modes.append(FEATURE_TOGGLE_MUTE)
+    return supported_modes
+
+
+def validate_media_player_features(state, feature_list):
+    """Validate features for media players."""
+    supported_modes = get_media_player_features(state)
+
+    if not supported_modes:
+        _LOGGER.error("%s does not support any media_player features", state.entity_id)
+        return False
+
+    if not feature_list:
+        # Auto detected
+        return True
 
     error_list = []
     for feature in feature_list:
         if feature not in supported_modes:
             error_list.append(feature)
 
     if error_list:
-        _LOGGER.error("%s does not support features: %s", state.entity_id, error_list)
+        _LOGGER.error(
+            "%s does not support media_player features: %s", state.entity_id, error_list
+        )
         return False
     return True
 
@@ -247,6 +297,16 @@ def convert_to_float(state):
         return None
 
 
+def cleanup_name_for_homekit(name):
+    """Ensure the name of the device will not crash homekit."""
+    #
+    # This is not a security measure.
+    #
+    # UNICODE_EMOJI is also not allowed but that
+    # likely isn't a problem
+    return name.translate(HOMEKIT_CHAR_TRANSLATIONS)
+
+
 def temperature_to_homekit(temperature, unit):
     """Convert temperature to Celsius for HomeKit."""
     return round(temp_util.convert(temperature, unit, TEMP_CELSIUS), 1)

tests/components/homekit/test_type_media_players.py

@@ -368,6 +368,26 @@ async def test_media_player_television_basic(hass, hk_driver, events, caplog):
     assert not caplog.messages or "Error" not in caplog.messages[-1]
 
 
+async def test_media_player_television_supports_source_select_no_sources(
+    hass, hk_driver, events, caplog
+):
+    """Test if basic tv that supports source select but is missing a source list."""
+    entity_id = "media_player.television"
+
+    # Supports turn_on', 'turn_off'
+    hass.states.async_set(
+        entity_id,
+        None,
+        {ATTR_DEVICE_CLASS: DEVICE_CLASS_TV, ATTR_SUPPORTED_FEATURES: 3469},
+    )
+    await hass.async_block_till_done()
+    acc = TelevisionMediaPlayer(hass, hk_driver, "MediaPlayer", entity_id, 2, None)
+    await acc.run_handler()
+    await hass.async_block_till_done()
+
+    assert acc.support_select_source is False
+
+
 async def test_tv_restore(hass, hk_driver, events):
     """Test setting up an entity from state in the event registry."""
     hass.state = CoreState.not_running

tests/components/homekit/test_util.py

@@ -22,6 +22,7 @@
 from homeassistant.components.homekit.util import (
     HomeKitSpeedMapping,
     SpeedRange,
+    cleanup_name_for_homekit,
     convert_to_float,
     density_to_air_quality,
     dismiss_setup_message,
@@ -172,6 +173,19 @@ def test_convert_to_float():
     assert convert_to_float(None) is None
 
 
+def test_cleanup_name_for_homekit():
+    """Ensure name sanitize works as expected."""
+
+    assert cleanup_name_for_homekit("abc") == "abc"
+    assert cleanup_name_for_homekit("a b c") == "a b c"
+    assert cleanup_name_for_homekit("ab_c") == "ab c"
+    assert (
+        cleanup_name_for_homekit('ab!@#$%^&*()-=":.,><?//\\ frog')
+        == "ab--#---&----- -.,------ frog"
+    )
+    assert cleanup_name_for_homekit("の日本_語文字セット") == "の日本 語文字セット"
+
+
 def test_temperature_to_homekit():
     """Test temperature conversion from HA to HomeKit."""
     assert temperature_to_homekit(20.46, TEMP_CELSIUS) == 20.5