Skip to content
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

Record IQS for Switchbot #136058

Merged
merged 7 commits into from
Jan 22, 2025
Merged

Record IQS for Switchbot #136058

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
67f9387
Record IQS for Switchbot
huyuwei1996 Jan 20, 2025
4b85a97
Update quality scale for Switchbot integration with status and comments
huyuwei1996 Jan 21, 2025
557ebc2
Update discovery-update-info comment
huyuwei1996 Jan 21, 2025
9677540
Merge branch 'dev' into feat/switchbot-iqs
huyuwei1996 Jan 21, 2025
8013c1e
update comment
huyuwei1996 Jan 21, 2025
2cd4062
Update homeassistant/components/switchbot/quality_scale.yaml
joostlek Jan 22, 2025
32eeb42
Update homeassistant/components/switchbot/quality_scale.yaml
joostlek Jan 22, 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
67 changes: 52 additions & 15 deletions homeassistant/components/switchbot/quality_scale.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
rules:
Copy link
Member

Choose a reason for hiding this comment

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

One general note: I believe there are quite a few issues where users try to set up their new device (which isnt supported by HA yet at that time), and then it gets detected as led strip, but doesn't actually work. I think that is something that can be improved on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a known issue.Does it need to be done under the gold standard? We have some ideas, but it will take some time to develop

Copy link
Member

Choose a reason for hiding this comment

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

This was fixed in sblibs/pySwitchbot#265

Copy link
Contributor Author

@huyuwei1996 huyuwei1996 Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This was fixed in sblibs/pySwitchbot#265

As far as I know, we probably have a way to correctly determine the device type during passive scanning, which is what we hope for. And it can be done without modifying the firmware

Copy link
Member

Choose a reason for hiding this comment

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

There currently isn't sblibs/pySwitchbot#284

Also confirmed by Switchbot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There currently isn't sblibs/pySwitchbot#284

Also confirmed by Switchbot

Yes, I just modified my comment. There was a grammatical error in what I said just now.

# Bronze
action-setup: done
action-setup:
status: exempt
comment: |
No custom actions
appropriate-polling: done
brands: todo
brands: done
common-modules: done
config-flow-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.

Consider making

def patch_async_setup_entry(return_value=True):
    """Patch async setup entry to return True."""
    return patch(
        "homeassistant.components.switchbot.async_setup_entry",
        return_value=return_value,
    )

a proper generator

@pytest.fixture
def mock_setup_entry() -> Generator[AsyncMock]:
    """Override async_setup_entry."""
    with patch(
        "homeassistant.components.switchbot.async_setup_entry",
        return_value=True,
    ) as mock_setup_entry:
        yield mock_setup_entry

This way you only have to include it as a fixture and you can remove all the with patch_async_setup_entry() as mock_setup_entry from the tests

Copy link
Member

Choose a reason for hiding this comment

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

Let's also test the unique id in test_bluetooth_discovery and test_bluetooth_discovery_requires_password and test_bluetooth_discovery_encrypted_key

Copy link
Member

Choose a reason for hiding this comment

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

Consider parametrizing tests like test_user_setup_wohand, test_user_setup_wocurtain

Copy link
Member

Choose a reason for hiding this comment

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

Only patch the library where you use them

    with patch(
        "switchbot.SwitchbotLock.verify_encryption_key",
        return_value=False,
    ):

so rather do homeassistant.components.switchbot.....

config-flow: done
Expand All @@ -16,7 +19,7 @@ rules:
docs-removal-instructions: todo
entity-event-setup: done
entity-unique-id: done
has-entity-name: todo
has-entity-name: done
runtime-data: done
test-before-configure: done
test-before-setup: done
Expand All @@ -30,35 +33,69 @@ rules:
entity-unavailable: done
integration-owner: done
log-when-unavailable: todo
parallel-updates: done
reauthentication-flow: todo
test-coverage: todo
parallel-updates:
status: todo
comment: |
set `PARALLEL_UPDATES` in lock.py
reauthentication-flow:
status: todo
comment: |
Consider using snapshots for fixating all the entities a device creates.
E.g., reconfiguring device encryption key or password
test-coverage:
status: todo
comment: |
To be completed.
joostlek marked this conversation as resolved.
Show resolved Hide resolved

# Gold
devices: done
diagnostics: todo
discovery-update-info:
status: exempt
comment: |
Not possible to discover these devices.
discovery: done
No network discovery.
discovery:
status: done
comment: |
Can be improved: Device type scan filtering is applied to only show devices that are actually supported.
docs-data-update: todo
docs-examples: todo
docs-known-limitations: todo
docs-supported-devices: done
docs-supported-functions: todo
docs-troubleshooting: done
docs-use-cases: todo
dynamic-devices: todo
dynamic-devices:
status: exempt
comment: |
Only one device per config entry. New devices are set up as new entries.
entity-category: done
entity-device-class: done
entity-disabled-by-default: done
entity-translations: done
exception-translations: todo
icon-translations: todo
reconfiguration-flow: todo
repair-issues: todo
stale-devices: todo
entity-translations:
status: todo
comment: |
Needs to provide translations for hub2 temperature entity
exception-translations:
status: exempt
comment: |
No custom exceptions.
joostlek marked this conversation as resolved.
Show resolved Hide resolved
icon-translations:
status: exempt
comment: |
No custom icons.
reconfiguration-flow:
status: exempt
comment: |
No need for reconfiguration flow.
repair-issues:
status: exempt
comment: |
No repairs/issues.
stale-devices:
status: exempt
comment: |
Device type integration.

# Platinum
async-dependency: done
Expand Down