Add heat pump sensors to ViCare integration#161422
Add heat pump sensors to ViCare integration#161422joostlek merged 1 commit intohome-assistant:devfrom
Conversation
|
Hey there @CFenner, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
epenet
left a comment
There was a problem hiding this comment.
This PR depends on corresponding changes to the PyViCare library: openviess/PyViCare#688
If this is not actionnable, please keep in draft until the library has been updated and bumped inside Home Assistant
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f98ce40d14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceHeating(), |
There was a problem hiding this comment.
Bump PyViCare to match new COP APIs
These new sensors call api.getCoefficientOfPerformanceHeating() (and the other COP getters added here), but homeassistant/components/vicare/manifest.json still pins PyViCare==2.55.1. The commit message notes these APIs come from a newer PyViCare PR, so with the currently pinned version this call will raise AttributeError during ViCareSensor.update, breaking the new entities on a standard install. Please update the PyViCare requirement (and regenerate requirements_all.txt) to a version that includes these methods.
Useful? React with 👍 / 👎.
f98ce40 to
770cba8
Compare
770cba8 to
70ee425
Compare
| ViCareSensorEntityDescription( | ||
| key="cop_green", | ||
| translation_key="cop_green", | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceGreen(), | ||
| entity_registry_enabled_default=False, | ||
| ), |
There was a problem hiding this comment.
We did not implement this.
| ViCareSensorEntityDescription( | |
| key="cop_green", | |
| translation_key="cop_green", | |
| state_class=SensorStateClass.MEASUREMENT, | |
| entity_category=EntityCategory.DIAGNOSTIC, | |
| value_getter=lambda api: api.getCoefficientOfPerformanceGreen(), | |
| entity_registry_enabled_default=False, | |
| ), |
| "cop_green": { | ||
| "name": "Coefficient of performance - green" | ||
| }, |
There was a problem hiding this comment.
| "cop_green": { | |
| "name": "Coefficient of performance - green" | |
| }, |
| native_unit_of_measurement=PERCENTAGE, | ||
| value_getter=lambda api: api.getModulation(), | ||
| unit_getter=lambda api: api.getModulationUnit(), |
There was a problem hiding this comment.
Just out of curiosity, not saying that it is wrong, but why is there both native_unit_of_measurement and unit_getter for this? Can the unit change in the API?
There was a problem hiding this comment.
The unit_getter is a pattern used across several ViCare sensors (e.g. supply_pressure, primary_circuit_pump_rotation). It queries the device for the unit it reports in, allowing the sensor to adapt if different models/regions use different units. For compressor_power specifically, it likely always returns kilowatt, but we follow the same defensive pattern for consistency.
There was a problem hiding this comment.
Pull request overview
This pull request adds 15 new heat pump-specific sensors to the ViCare integration for Viessmann heat pumps, enhancing monitoring capabilities for heat pump operations. The changes depend on PyViCare 2.58.0 which has already been merged in PR #163686.
Changes:
- Added 5 Coefficient of Performance (COP) sensors tracking efficiency metrics for heating, DHW, total, cooling, and green modes
- Added 2 compressor sensors (power and modulation) to monitor compressor operation
- Added 6 refrigerant circuit sensors tracking hot gas, suction gas, and liquid gas temperatures and pressures
- Added 1 primary circuit pump rotation sensor
- Updated translations in strings.json for all new sensor keys
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| homeassistant/components/vicare/sensor.py | Added 15 new sensor definitions across GLOBAL_SENSORS and COMPRESSOR_SENSORS tuples with appropriate entity categories, state classes, and device classes |
| homeassistant/components/vicare/strings.json | Added translation keys for all new sensors with descriptive names following integration conventions |
| tests/components/vicare/snapshots/test_sensor.ambr | Added snapshot test data for new sensors validating entity registry and state attributes |
| value_getter=lambda api: api.getPower(), | ||
| unit_getter=lambda api: api.getPowerUnit(), | ||
| device_class=SensorDeviceClass.POWER, | ||
| state_class=SensorStateClass.MEASUREMENT, |
There was a problem hiding this comment.
The compressor_power sensor should have entity_category set to EntityCategory.DIAGNOSTIC for consistency with the integration's diagnostic sensor patterns. Looking at similar power sensors in the codebase (e.g., inverter_power at line 1462), and considering this is a component-level metric similar to compressor_starts and compressor_hours (which are DIAGNOSTIC), this sensor should be marked as diagnostic. Currently, it's set to None, which makes it appear as a primary entity.
| state_class=SensorStateClass.MEASUREMENT, | |
| state_class=SensorStateClass.MEASUREMENT, | |
| entity_category=EntityCategory.DIAGNOSTIC, |
| ViCareSensorEntityDescription( | ||
| key="compressor_modulation", | ||
| translation_key="compressor_modulation", | ||
| native_unit_of_measurement=PERCENTAGE, | ||
| value_getter=lambda api: api.getModulation(), | ||
| unit_getter=lambda api: api.getModulationUnit(), | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| ), |
There was a problem hiding this comment.
The compressor_modulation sensor is defined in the code but is not present in the test fixture data (Vitocal222G_Vitovent300W.json). While this is not necessarily incorrect (the sensor may only be available on certain heat pump models), it would be beneficial to verify this is intentional. If this sensor is expected to be available on the test device, the fixture data should be updated to include it. Otherwise, consider adding a comment explaining why this sensor is not present in all heat pump models.
| # name: test_all_entities[sensor.model2_compressor_power-entry] | ||
| EntityRegistryEntrySnapshot({ | ||
| 'aliases': set({ | ||
| }), | ||
| 'area_id': None, | ||
| 'capabilities': dict({ | ||
| 'state_class': <SensorStateClass.MEASUREMENT: 'measurement'>, | ||
| }), | ||
| 'config_entry_id': <ANY>, | ||
| 'config_subentry_id': <ANY>, | ||
| 'device_class': None, | ||
| 'device_id': <ANY>, | ||
| 'disabled_by': None, | ||
| 'domain': 'sensor', | ||
| 'entity_category': None, | ||
| 'entity_id': 'sensor.model2_compressor_power', | ||
| 'has_entity_name': True, | ||
| 'hidden_by': None, | ||
| 'icon': None, | ||
| 'id': <ANY>, | ||
| 'labels': set({ | ||
| }), | ||
| 'name': None, | ||
| 'object_id_base': 'Compressor power', | ||
| 'options': dict({ | ||
| 'sensor': dict({ | ||
| 'suggested_display_precision': 2, | ||
| }), | ||
| }), | ||
| 'original_device_class': <SensorDeviceClass.POWER: 'power'>, | ||
| 'original_icon': None, | ||
| 'original_name': 'Compressor power', | ||
| 'platform': 'vicare', | ||
| 'previous_unique_id': None, | ||
| 'suggested_object_id': None, | ||
| 'supported_features': 0, | ||
| 'translation_key': 'compressor_power', | ||
| 'unique_id': 'gateway2_################-compressor_power-0', | ||
| 'unit_of_measurement': <UnitOfPower.KILO_WATT: 'kW'>, | ||
| }) |
There was a problem hiding this comment.
The test snapshot shows entity_category as None for this sensor, but based on the code review at lines 1289-1297 in sensor.py, this should be EntityCategory.DIAGNOSTIC. After fixing the entity_category in the sensor definition, this snapshot will need to be regenerated to reflect the correct entity_category value of 'diagnostic'.
70ee425 to
dcde619
Compare
dcde619 to
445f30d
Compare
lackas
left a comment
There was a problem hiding this comment.
Thanks for catching this! getCoefficientOfPerformanceGreen() doesn't exist in PyViCare. Removed the sensor and its strings.json entry. Also made compressor_power EntityCategory.DIAGNOSTIC since it's a fixed rated value, not a live measurement.
| ViCareSensorEntityDescription( | ||
| key="cop_heating", | ||
| translation_key="cop_heating", | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceHeating(), | ||
| ), | ||
| ViCareSensorEntityDescription( | ||
| key="cop_dhw", | ||
| translation_key="cop_dhw", | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceDHW(), | ||
| ), | ||
| ViCareSensorEntityDescription( | ||
| key="cop_total", | ||
| translation_key="cop_total", | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceTotal(), | ||
| ), | ||
| ViCareSensorEntityDescription( | ||
| key="cop_cooling", | ||
| translation_key="cop_cooling", | ||
| state_class=SensorStateClass.MEASUREMENT, | ||
| entity_category=EntityCategory.DIAGNOSTIC, | ||
| value_getter=lambda api: api.getCoefficientOfPerformanceCooling(), | ||
| entity_registry_enabled_default=False, | ||
| ), |
There was a problem hiding this comment.
The PR description states there are 15 new sensors and mentions a "green COP" sensor, but the code only adds 12 sensors (4 refrigerant circuit temperatures/pressures, 4 COP sensors, 1 pump rotation, 2 compressor sensors, and 1 primary circuit pump). The "green COP" sensor is not implemented. Please update the PR description to accurately reflect the 12 sensors actually being added: COP (heating, DHW, total, cooling), compressor (power, modulation), refrigerant circuit (hot gas/suction gas/liquid gas temperatures and hot gas/suction gas pressures), and primary circuit pump rotation.
| value_getter=lambda api: api.getPower(), | ||
| unit_getter=lambda api: api.getPowerUnit(), | ||
| device_class=SensorDeviceClass.POWER, | ||
| state_class=SensorStateClass.MEASUREMENT, |
There was a problem hiding this comment.
@lackas you say this is no measurement but a fixed value? Then I think we should change this.
There was a problem hiding this comment.
Yes, looking at the available test data, heating.compressors.0.power is a fixed rated value across all fixtures (8 kW on 222G/333G, 10 kW on 200S, 12 kW on 300-G). Removed SensorStateClass.MEASUREMENT and kept it as EntityCategory.DIAGNOSTIC — it's still useful as a spec/reference value. Pushed.
Add support for additional heat pump sensors: - Coefficient of Performance (COP): heating, DHW, total, cooling, green - Compressor: power output and modulation percentage - Refrigerant circuit: hot gas, suction gas, and liquid gas pressures/temperatures - Primary circuit pump rotation
445f30d to
3de4ec5
Compare
|
Please avoid force pushed to ease review. |
Proposed change
Add heat pump sensors to the ViCare integration for Viessmann heat pumps (e.g. Vitocal 300-G).
New sensors (14 total):
These sensors are only created for devices that report as heat pumps (
type:heatpump). All sensors useEntityCategory.DIAGNOSTICandSensorStateClass.MEASUREMENT(exceptcompressor_powerwhich is a fixed rated value).Depends on PyViCare methods added in PyViCare#689 (merged), included in PyViCare 2.58.0 which was bumped in #163686 (merged).
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: