Add binary sensor platform to Arcam FMJ#165272
Add binary sensor platform to Arcam FMJ#165272jgus wants to merge 3 commits intohome-assistant:devfrom
Conversation
Add incoming video interlaced binary sensor for each zone, disabled by default, using the coordinator pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hey there @elupus, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Adds a new Arcam FMJ binary sensor to expose whether the incoming video signal is interlaced (per zone), implemented via the coordinator pattern and disabled by default.
Changes:
- Introduces a new
binary_sensorplatform with anincoming_video_interlacedentity per zone (disabled by default). - Adds entity translation strings for the new binary sensor.
- Adds test coverage for creation, value states, and coordinator signal updates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
homeassistant/components/arcam_fmj/__init__.py |
Registers the new binary_sensor platform for config entry setup. |
homeassistant/components/arcam_fmj/binary_sensor.py |
Implements coordinator-backed binary sensor entity for interlaced input video status. |
homeassistant/components/arcam_fmj/strings.json |
Adds translated entity name template for the new binary sensor. |
tests/components/arcam_fmj/test_binary_sensor.py |
Adds tests for entity creation, state mapping, and coordinator-driven updates. |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Do those comments actually block these changes? Or can we just as easily clean them up once all actual functionality is in? |
- Replace getattr with direct None check for video parameters - Use coordinator.state instead of cached self._state to avoid stale data - Use entity_registry_enabled_by_default fixture in tests - Replace runtime_data access in tests with coordinator fixtures - Use state constants (STATE_ON, STATE_OFF, etc.) instead of raw strings - Fix conftest mock defaults to use (None, None) for audio format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nary-sensor # Conflicts: # tests/components/arcam_fmj/conftest.py
epenet
left a comment
There was a problem hiding this comment.
You need to fix merge conflicts, and open documentation PR
| @pytest.fixture(name="coordinator_1") | ||
| def coordinator_1_fixture( | ||
| hass: HomeAssistant, player_setup: str | ||
| ) -> ArcamFmjCoordinator: | ||
| """Get the coordinator for zone 1.""" | ||
| config_entry = hass.config_entries.async_entries("arcam_fmj")[0] | ||
| return config_entry.runtime_data.coordinators[1] | ||
|
|
||
|
|
||
| @pytest.fixture(name="coordinator_2") | ||
| def coordinator_2_fixture( | ||
| hass: HomeAssistant, player_setup: str | ||
| ) -> ArcamFmjCoordinator: | ||
| """Get the coordinator for zone 2.""" | ||
| config_entry = hass.config_entries.async_entries("arcam_fmj")[0] | ||
| return config_entry.runtime_data.coordinators[2] |
There was a problem hiding this comment.
Please remove these two fixtures.
You are using them to test internal and this is not allowed
Also, please keep one of the PRs in draft (eg. sensor) until the first one is merged (eg. binary_sensor)
| class ArcamFmjBinarySensorEntity( | ||
| CoordinatorEntity[ArcamFmjCoordinator], BinarySensorEntity | ||
| ): |
There was a problem hiding this comment.
Some of the code is duplicated with the media_player entity.
I suggest that you create a new entity.py module with a new class ArcamFmjCoordinatorEntity:
- it should set
has_entity_name - it should call
super().__init__(coordinator) - it should set
device_info
You can do this inside this PR, or you can do it in a preliminary PR
Add incoming video interlaced binary sensor for each zone, disabled by default, using the coordinator pattern.
This is a breakdown of #165098 per request, a follow-up to #165232, and sibling to #165271
Proposed change
Add a binary sensor that indicated whether the incoming video is interlaced
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: