Record IQS for Switchbot #136058
Record IQS for Switchbot #136058
Conversation
home-assistant
bot
added
cla-signed
code-quality
integration: switchbot
quality-scale
labels
|
Hey there @Danielhiversen, @RenierM26, @murtas, @Eloston, @dsypniewski, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
ok, thanks. |
huyuwei1996
force-pushed
the
feat/switchbot-iqs
branch
from
8c3e0b7 to
313023f
Compare
huyuwei1996
force-pushed
the
feat/switchbot-iqs
branch
from
349822a to
67f9387
Compare
| @@ -0,0 +1,66 @@ | |||
| rules: | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
This was fixed in sblibs/pySwitchbot#265
There was a problem hiding this comment.
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
There was a problem hiding this comment.
There currently isn't sblibs/pySwitchbot#284
Also confirmed by Switchbot
There was a problem hiding this comment.
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.
| entity-category: done | ||
| entity-device-class: done | ||
| entity-disabled-by-default: done | ||
| entity-translations: done |
There was a problem hiding this comment.
One thing on entity names, I happen to have a Hub 2, and the temperature sensor's name is set to None (because there is also a thermometer or another device where the temperature sensor is the mail feature of the device), but that looks weird on the Hub 2.
There was a problem hiding this comment.
I understand. I also found it when I used it myself. Because I was just developing it for my own use, I simply added the sensor reading of hub2. I will make changes later to change the entity name of hub2 temperature sensor to xxx_temperature.
| appropriate-polling: done | ||
| brands: todo | ||
| common-modules: done | ||
| config-flow-test-coverage: todo |
There was a problem hiding this comment.
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_entryThis 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
| appropriate-polling: done | ||
| brands: todo | ||
| common-modules: done | ||
| config-flow-test-coverage: todo |
There was a problem hiding this comment.
Let's also test the unique id in test_bluetooth_discovery and test_bluetooth_discovery_requires_password and test_bluetooth_discovery_encrypted_key
| appropriate-polling: done | ||
| brands: todo | ||
| common-modules: done | ||
| config-flow-test-coverage: todo |
There was a problem hiding this comment.
Consider parametrizing tests like test_user_setup_wohand, test_user_setup_wocurtain
Breaking change
Proposed change
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: