Skip to content

Add quality scale for Sonos #144928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft

Add quality scale for Sonos #144928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
953d9bf
Sonos current integration quality scale
PeteRager May 7, 2025
1fe23be
fix: update quality_scale
PeteRager May 14, 2025
20ef219
fix: update quality_scale
PeteRager May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions homeassistant/components/sonos/quality_scale.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
rules:
Copy link
Member

Choose a reason for hiding this comment

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

    @property
    def extra_state_attributes(self) -> dict[str, Any]:
        """Return entity specific state attributes."""
        return {
            ATTR_BATTERY_POWER_SOURCE: self.speaker.power_source,
        }

in binary sensor, could we make this a full fledged sensor? I have a roam with both wireless charging and usb-c, so I think we can create a complete list for that one

Copy link
Member

Choose a reason for hiding this comment

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

What about the media player attributes?

Copy link
Member

Choose a reason for hiding this comment

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

We don't want to store the entities in another place other than passing them onto async_add_entities

        self.hass.data[DATA_SONOS].entity_id_mappings[self.entity_id] = self.speaker

Copy link
Contributor Author

@PeteRager PeteRager May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This data is used in async_join_players to go from the entity_id to the associated speaker object. I believe the speaker is the device; so instead we could go to the entity_registry and get the device and lookup the speaker from the device_id.

I have a branch where I'm moving all of DATA_SONOS to runtime data on the config_entry. Tests are passing and starting to do run-time validation. There alot of small changes.

A sequence could be
a) move to runtime data
b) address this comment in a seperate PR
c) address the tests that access hass.data (which will now be runtime data) in a seperate PR

or alternatively we could address this comment first; then do the move to runtime data.

Copy link
Member

Choose a reason for hiding this comment

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

I think either way works

Copy link
Contributor Author

@PeteRager PeteRager May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ok starting with moving to run-time data. #145512 - which is showing some codecov gaps - so we'll need to add some additional tests first in PR #145602

# Bronze
action-setup:
status: todo
comment: |
Needs complete refactor
Comment on lines +3 to +6
Copy link
Member

Choose a reason for hiding this comment

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

I think a few entity service could be made into entities, like the sleep timer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree - rather than just porting them all - we'll see where we can make entities and then deprecate the service call

appropriate-polling: done
brands: done
common-modules: done
config-flow-test-coverage: done
config-flow: done
dependency-transparency: done
docs-actions: done
docs-high-level-description: done
docs-installation-instructions: done
docs-removal-instructions: todo
entity-event-setup: done
entity-unique-id: done
has-entity-name: done
runtime-data: todo
test-before-configure: done
test-before-setup:
status: exempt
comment: |
Offline devices are automatically reconnected
unique-config-entry:
status: exempt
comment: |
Only allows a single entry
Comment on lines +26 to +29
Copy link
Member

Choose a reason for hiding this comment

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

You can also set single_config_entry in the manifest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. will try that. when I marked it as done the CI fails since it doesn't detect the right exception being raised.

# Silver
action-exceptions: todo
config-entry-unloading: done
docs-configuration-parameters: done
docs-installation-parameters: done
entity-unavailable: done
integration-owner: done
log-when-unavailable: done
parallel-updates: todo
reauthentication-flow:
status: exempt
comment: |
No authentication
test-coverage: todo
Copy link
Member

Choose a reason for hiding this comment

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

Some tests are using internals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That needs to be fixed.

Copy link
Member

Choose a reason for hiding this comment

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

test_async_poll_manual_hosts_5 why do we set the total second to 0.5 to only sleep 0.5 seconds. Can't we then just sleep the original time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can fix this.

Looks like a change was made to add async_fire_time_changed (probably to make the test more reliable) and the interval was left.

Copy link
Member

Choose a reason for hiding this comment

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

test_play_media_library ideally we don't have if statements in the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree

Copy link
Member

Choose a reason for hiding this comment

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

For some reason test_sensor is testing both binary sensor and sensor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can break it apart

# Gold
devices: done
diagnostics: done
discovery-update-info: todo
discovery: done
docs-data-update: todo
docs-examples: done
docs-known-limitations: done
docs-supported-devices: todo
docs-supported-functions: todo
docs-troubleshooting: todo
docs-use-cases: done
dynamic-devices: done
entity-category: done
entity-device-class: done
entity-disabled-by-default: done
entity-translations: todo
exception-translations: todo
icon-translations: todo
reconfiguration-flow:
status: exempt
comment: |
No configurable options
repair-issues: todo
stale-devices: todo

# Platinum
async-dependency: todo
inject-websession: todo
strict-typing: todo
1 change: 0 additions & 1 deletion script/hassfest/quality_scale.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,6 @@ class Rule:
"somfy_mylink",
"sonarr",
"songpal",
"sonos",
"sony_projector",
"soundtouch",
"spaceapi",
Expand Down