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.
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
Add support for arcam fmj receivers #24621
Conversation
.coveragerc
Outdated
@@ -38,6 +38,7 @@ omit = | |||
homeassistant/components/apple_tv/* | |||
homeassistant/components/aqualogic/* | |||
homeassistant/components/aquostv/media_player.py | |||
homeassistant/components/arcam_fmj/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure you don't exclude the tests for your config flow...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted below, there is not real config flow. Only import. Since device registry is non working anyway for this integration (can't get unique id) i could just drop it and move back to old way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still test the config flow. That module needs 100% coverage.
"""Initialize the config flow.""" | ||
self._config = OrderedDict() | ||
|
||
async def async_step_import(self, import_config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So why only the import? That seems a bit inconvenient for the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because without configuring a turn_on service call with an IR blaster or similar it's quite limited in use. And there is no good way to do that from config flows at the moment. Perhaps once option flows are in place...
Otherwise they won't be called in loop
Currently tests are failing since lib is 3.6 only. depends on yields in async functions. I will see if can fix that, but i oh so wish we could speed up the 3.5 removal :) edit: No long the case |
Anything more i need to look at before I press the merge button on this? |
Please don't merge until another member has approved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
… into hive_water_heater * 'hive_water_heater' of github.com:Rendili/home-assistant: (21 commits) Sensibo, add HVAC_MODE_OFF (home-assistant#25016) Add support for arcam fmj receivers (home-assistant#24621) Enphase envoy individual inverter production (home-assistant#24445) Implement Twilio SMS notify MediaUrl support (home-assistant#24971) Climate 1.0 (home-assistant#23899) Correct socket use in cert_expiry platform (home-assistant#25011) Added missing yeelight models mapping (home-assistant#24963) Install requirements for integrations in packages before importing them. (home-assistant#25005) Upgrade insteonplm to 0.16.0 and add INSTEON scene triggering (home-assistant#24765) Upgrade hdate==0.8.8 (home-assistant#25008) upgrade switchmate to latest lib (home-assistant#25006) Test dependency updates (home-assistant#25004) Add support for aurora ABB Powerone solar photovoltaic inverter (home-assistant#24809) Sleepiq single sleeper crash (home-assistant#24941) Changes as per code review of home-assistant#24646 (home-assistant#24917) Upgrade mypy to 0.711, drop no longer needed workarounds (home-assistant#24998) Adds Stale Probot for issues (home-assistant#24985) Adds Lock Threads Probot (home-assistant#24984) Switched from tuyapy to tuyaha as 1st one is not maintained (home-assistant#24821) Fix errors if rest source becomes unavailable (home-assistant#24986) ... # Conflicts: # homeassistant/components/hive/__init__.py # homeassistant/components/hive/climate.py
* Add arcam_fmj support * Just use use state in player avoid direct client access * Avoid leaking exceptions on invalid data * Fix return value for volume in case of 0 * Mark component as having no coverage * Add new requirement * Add myself as maintainer * Correct linting errors * Use async_create_task instead of async_add_job * Use new style string format instead of concat * Don't call init of base class without init * Annotate callbacks with @callback Otherwise they won't be called in loop * Reduce log level to debug * Use async_timeout instead of wait_for * Bump to version of arcam_fmj supporting 3.5 * Fix extra spaces * Drop somewhat flaky unique_id * Un-blackify ident to satisy pylint * Un-blackify ident to satisy pylint * Move default name calculation to config validation * Add test folder * Drop unused code * Add tests for config flow import
Description:
Add support for arcam fmj receivers
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#9660
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices: