Skip to content

Fix HomeKit select accessories not marking the active option as on - #175603

Merged
bdraco merged 1 commit into
home-assistant:devfrom
mattias-arrelid:homekit-selectswitch-raw-option-compare
Jul 4, 2026
Merged

Fix HomeKit select accessories not marking the active option as on#175603
bdraco merged 1 commit into
home-assistant:devfrom
mattias-arrelid:homekit-selectswitch-raw-option-compare

Conversation

@mattias-arrelid

Copy link
Copy Markdown
Contributor

Breaking change

Proposed change

SelectSwitch.async_update_state ran the current state through cleanup_name_for_homekit() before comparing it against the raw option strings that key self.select_chars. Any option containing a character the cleanup translates (underscore being the common case, but also characters mapped to - and options longer than MAX_NAME_LENGTH) could therefore never match, so its outlet never showed as on in the Home app — and when two options collide after cleanup (e.g. always_on and always on), the wrong outlet lit up. Control in the HomeKit→HA direction was unaffected, since the setter closes over the raw option.

The fix compares raw against raw: a select's state is by contract exactly one of its options, so the cleanup has no business in the comparison. Name cleanup is display-only and is kept for CHAR_NAME/CHAR_CONFIGURED_NAME, which the new test also asserts.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies a diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

The state comparison in SelectSwitch.async_update_state ran the current
state through cleanup_name_for_homekit() before comparing it against the
raw option strings used as keys. Any option containing a character the
cleanup translates (e.g. underscores) could therefore never match, so
its outlet never showed as on. The cleanup is meant for display names
only; a select state is by contract exactly one of its raw options.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 4, 2026 11:07
@mattias-arrelid
mattias-arrelid requested a review from bdraco as a code owner July 4, 2026 11:07
@home-assistant home-assistant Bot added bugfix cla-signed has-tests integration: homekit small-pr PRs with less than 30 lines. Top 100 Integration is ranked within the top 100 by usage Top 200 Integration is ranked within the top 200 by usage Top 50 Integration is ranked within the top 50 by usage labels Jul 4, 2026
@home-assistant

home-assistant Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (homekit) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of homekit can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant mark-draft Mark the pull request as draft.
  • @home-assistant ready-for-review Remove the draft status from the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign homekit Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant update-branch Update the pull request branch with the base branch.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) on the pull request.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a display-only bug in the HomeKit Bridge SelectSwitch accessory (which renders select/input_select entities as a power strip of outlets). async_update_state compared the entity's state after passing it through cleanup_name_for_homekit() against self.select_chars keys that are the raw option strings. Any option containing a character the cleanup translates (e.g. underscores), or longer than MAX_NAME_LENGTH, could never match, so its outlet never showed as on; colliding options could also light the wrong outlet. The fix removes the cleanup from the comparison so raw is compared against raw, which is correct since a select's state is by contract exactly one of its options. Name cleanup remains for the display-only CHAR_NAME/CHAR_CONFIGURED_NAME.

Changes:

  • In async_update_state, compare the raw new_state.state against the raw option keys instead of the cleaned-up name.
  • Add a parametrized test (input_select/select) covering options altered by name cleanup, including a post-cleanup collision, and asserting CHAR_NAME/CHAR_CONFIGURED_NAME cleanup is preserved and the HomeKit→HA setter still works.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
homeassistant/components/homekit/type_switches.py Compares raw state against raw option keys in async_update_state, fixing outlets never lighting for cleaned/colliding options.
tests/components/homekit/test_type_switches.py Adds a parametrized regression test for options needing name cleanup, verifying state sync, display-name cleanup, and setter behavior.

@bluetoothbot

Copy link
Copy Markdown

PR Review — Fix HomeKit select accessories not marking the active option as on

Correct, minimal bugfix with strong regression coverage. Merge-ready.

What's solid:

  • Root cause is precisely diagnosed: select_chars is keyed by the raw option (type_switches.py:539) and the setter closes over the raw option (type_switches.py:542), so async_update_state comparing a cleaned state against raw keys could never match. Removing cleanup_name_for_homekit from the comparison makes it raw-vs-raw, which is right since a select's state is by contract one of its options.

  • I confirmed the failure path independently: char 95 (_) maps to space in HOMEKIT_CHAR_TRANSLATIONS (util.py:314), so always_on cleans to always on — exactly the never-matches / wrong-outlet-collision scenario described.

  • Display-only cleanup is correctly preserved for CHAR_NAME/CHAR_CONFIGURED_NAME (type_switches.py:536-537), and the new test asserts that so a future regression can't silently drop it.

  • The parametrized test covers both input_select and select, the post-cleanup collision (always_on vs always on), state sync in both states, and the HomeKit→HA setter direction.

  • No blocking issues found.



Checklist

  • Bugfix addresses stated root cause
  • Regression test covers the fix and edge cases
  • Display-name cleanup behavior preserved
  • No unintended scope creep

Automated review by Kōan (Claude) HEAD=82a5ca1 35s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@bdraco

bdraco commented Jul 4, 2026

Copy link
Copy Markdown
Member

Thanks @mattias-arrelid

@bdraco
bdraco merged commit 669dd93 into home-assistant:dev Jul 4, 2026
33 checks passed
@mattias-arrelid
mattias-arrelid deleted the homekit-selectswitch-raw-option-compare branch July 4, 2026 15:09
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix cla-signed has-tests integration: homekit Quality Scale: No score small-pr PRs with less than 30 lines. Top 50 Integration is ranked within the top 50 by usage Top 100 Integration is ranked within the top 100 by usage Top 200 Integration is ranked within the top 200 by usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HomeKit Bridge select entities never show the active option when it contains characters altered by name cleanup (e.g. underscores)

4 participants