-
Notifications
You must be signed in to change notification settings - Fork 114
feat(heat pump): add COP, compressor sensors, refrigerant circuit and heating rod stats #689
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
base: master
Are you sure you want to change the base?
feat(heat pump): add COP, compressor sensors, refrigerant circuit and heating rod stats #689
Conversation
eb51a59 to
91f78d6
Compare
CFenner
left a comment
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.
Please separate the PR into dedicated PRs for:
- heating rod
- cooling circuit
- heating circuit
- COP
- compressors
- etc
Also consider adding the testate in a dedicated PR upfront.
Sorry, this is to much to review.
- Remove test data (split to PR openviess#697) - Rename getTemperature -> getTargetTemperature per review - Rename getTemperatureUnit -> getTargetTemperatureUnit per review
|
Thanks for the feedback @CFenner! Changes made:
Converted to draft until #697 is merged (tests depend on that data). Review guide for the remaining ~365 lines of code changes: The changes are grouped into repetitive boilerplate patterns. Here's a quick overview:
Non-boilerplate changes:
I'm happy to split this further if you prefer. That said, my concern is that splitting into many small PRs might actually increase the review burden, and these features only provide full Vitocal 300G support when merged together. But I appreciate your time reviewing this and am glad to support your efforts however works best for you. |
|
Note to self: Once #697 is merged, we can remove |
Minimal changes to make existing tests pass with new Vitocal300G_CU401B.json: - Update expected values (compressor hours, starts, temperatures) - Use circuit 1 instead of 0 (device configuration difference) - Skip load class tests (need statistics.load fallback from PR openviess#689) - Remove outdated Vitocal300G.json (from 2021) Comprehensive tests for new features (COP, cooling circuits, heating rod, refrigerant sensors, etc.) will be added with PR openviess#689.
* test: add Vitocal 300G CU401B response data Adds API response dump for Vitocal 300G with CU401B controller. This test data will be used by upcoming feature PRs. * test: add heating.configuration.dhwHeater to ignore list This property is exposed by Vitocal 300G but not yet implemented. * Update test setup to use new response file * Update existing Vitocal 300G tests for fresh data Minimal changes to make existing tests pass with new Vitocal300G_CU401B.json: - Update expected values (compressor hours, starts, temperatures) - Use circuit 1 instead of 0 (device configuration difference) - Skip load class tests (need statistics.load fallback from PR #689) - Remove outdated Vitocal300G.json (from 2021) Comprehensive tests for new features (COP, cooling circuits, heating rod, refrigerant sensors, etc.) will be added with PR #689. * fix: use correct circuit list index in Vitocal300G tests The device only has circuit "1" enabled, so getAvailableCircuits() returns ["1"]. The library creates circuits[0] for this single circuit, not circuits[1]. Fix index and clarify comment. * fix: correct expected modes in Vitocal300G test Available modes are read from API constraints and vary by device configuration. This device reports 3 modes: dhw, dhwAndHeating, standby. --------- Co-authored-by: Christopher Fenner <9592452+CFenner@users.noreply.github.com>
… heating rod stats
Add comprehensive heat pump monitoring for Vitocal 300-G and similar devices:
HeatPump class:
- COP methods: getCoefficientOfPerformance{Heating,DHW,Total,Cooling,Green}
- Compressor: getPower, getModulation (with units)
- Refrigerant sensors: getHotGas/SuctionGas pressure and temperature, getLiquidGasTemperature
- Runtime: getMainECURuntime, getHeatingRodRuntimeLevel{One,Two}
- Configuration: buffer temp max, damping factor, heater approvals
- Heating rod power consumption summary (DHW and heating)
HeatingDevice class:
- Primary circuit pump: getPrimaryCircuitPumpRotation (with unit)
Compressor class:
- Load class methods now support fallback to statistics.load path
- Sensor methods: getInlet/Outlet/Overheat temperature, getInletPressure
New CoolingCircuit class:
- getType, getReverseActive
Closes openviess#677
ce02406 to
037276e
Compare
|
While exploring the issue tracker, I noticed this PR addresses several open issues:
That's quite a few birds with one stone! 🪨🐦 No pressure of course - just thought it might be helpful to know the coverage. Happy to adjust anything if needed. |
|
Hi @CFenner, Just checking in on this PR. Is there anything I can do to help move it forward? Regarding the split — I hope the review guide in the description helps: 240 of 290 lines are boilerplate getters, leaving only the load class fallback (~35 lines) and CoolingCircuit (~15 lines) as non-trivial code. I'm happy to split if you still prefer, though the individual PRs would only be fully useful once all are merged. I've been running these features as a custom HA component for over a week now, everything works reliably. This PR addresses several open issues:
Older issues asked for similar features too: #265, #16. There's also a downstream HA Core PR (home-assistant/core#161422) building on this. Let me know how you'd like to proceed! |
|
I simply do not have enough time, sorry, but I really appreciate your work, thanks! I'm fine with the target temp and circulation pump flow addition. In think the cooling circuit could be a or of its own. For some of the other changes, they might be deprecated already (buffer, cop), have to look again. |
- Remove getCoefficientOfPerformanceGreen (heating.cop.green is deprecated, replaced by heating.cop.photovoltaic) - Move heating.cop.green to deprecated properties list - Remove CoolingCircuit class (will be in separate PR)
|
Thanks for taking the time to review this, @CFenner — I really appreciate the guidance, especially given your limited time. It helps a lot! I now understand why splitting things up makes sense. The cooling circuit is a distinct feature area, so I've moved it into its own PR: #706. Other changes based on your feedback:
The buffer paths were already cleaned up in an earlier commit. This PR no longer adds any code targeting deprecated API endpoints. |
|
Thanks, I think I should have added a test case to avoid that deprecated and removed datapoint ts are added again 😀 There is already an SFP for heating she and total, do you see any for cooling in sour device? Would you mind adding that to be consistent? |
|
Thanks! The deprecation database PR (#707) should help with that going forward. Regarding COP for cooling — it's already included in this PR: Or did you mean something else by "SFP"? |
|
Yes I meant SFP. |
|
Checked our test data — no device has Happy to add |
PyViCare/PyViCareHeatPump.py
Outdated
|
|
||
| # Heating rod power consumption summary for DHW: | ||
| @handleNotSupported | ||
| def getHeatingRodPowerConsumptionSummaryDHWUnit(self) -> str: |
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.
HeatingRod is a device component of its own, so using this like this looks a bit odd:
heatingRod1 = ..
heatingRod.GetHeatingRodPowerConsumption()
I would recommend to remove the heatingRod prefix.
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.
Ah, no it's not, sorry. So next question, would it not make sense to have this as a component like we have for compressors and so on?
Extract all getHeatingRod* methods from HeatPump into a dedicated HeatingRod class, consistent with Compressor/Condensor/Inverter. Access via device.heatingRod.getStarts() etc.
|
You're right, made it a component class now — |
Summary
Add comprehensive heat pump monitoring methods, primarily for Vitocal 300-G but applicable to other models exposing these API features.
New features:
statistics.loadpathCoolingCircuitclass with type and reverse active statusCloses #677
Review guide
Total: ~290 lines of library code, ~280 lines of tests
Compressor(lines 547-585) - triesstatisticsthenstatistics.loadCoolingCircuitclass (lines 529-543)@handleNotSupporteddecorator + single-line property accessThe boilerplate getters all follow the exact same pattern as existing methods like
getSeasonalPerformanceFactorHeating(). You can skim these quickly - they're just exposing new API properties.Notes
Test plan
test_Vitocal300G.pycovering all new methodstest_Vitocal250A.pyfor heating rod power consumption summarytest_TestForMissingProperties.py