Skip to content
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

Adds COP for heating and hot water #55

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Adds COP for heating and hot water #55

merged 1 commit into from
Nov 18, 2024

Conversation

kr0ner
Copy link
Owner

@kr0ner kr0ner commented Nov 16, 2024

No description provided.

@kr0ner kr0ner added the enhancement New feature or request label Nov 16, 2024
@kr0ner kr0ner self-assigned this Nov 16, 2024
yaml/common.yaml Outdated Show resolved Hide resolved
@mkaiser
Copy link

mkaiser commented Nov 16, 2024

I find this a bit confusing with the SUMME_KWH vs. SUMME_MWH endings - can't you just remove the postfix (marked in bold) here for the "logical" sensors in the yaml? (see https://github.com/mkaiser/OneESP32ToRuleThemAll/blob/5d22afd6af31382cffb12075dc4e55950c0c8c01/yaml/ttf07.yaml#L226)

"WAERMEERTRAG_2WE_WW_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_2WE_WW_SUMME_MWH" , scaled_property: "WAERMEERTRAG_2WE_WW_SUM_KWH" , property: "WAERMEERTRAG_2WE_WW_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }}
WAERMEERTRAG_2WE_HEIZ_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_2WE_HEIZ_SUMME_MWH" , scaled_property: "WAERMEERTRAG_2WE_HEIZ_SUM_KWH", property: "WAERMEERTRAG_2WE_HEIZ_SUM_MWH", unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }}
WAERMEERTRAG_WW_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_WW_SUMME_MWH" , scaled_property: "WAERMEERTRAG_WW_SUM_KWH" , property: "WAERMEERTRAG_WW_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }}
WAERMEERTRAG_HEIZ_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_HEIZ_SUMME_MWH" , scaled_property: "WAERMEERTRAG_HEIZ_SUM_KWH" , property: "WAERMEERTRAG_HEIZ_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }}

... and the same for the electrical energy EL_AUFNAHMELEISTUNG (yeah, I promised the PR to rename it to EL_AUFNAHMELEISTUNG, but I like to get my stuff working before)

Also maybe change SUMME to TOTAL?

I am really confused :)

@mkaiser
Copy link

mkaiser commented Nov 16, 2024

Another thing coming to my mind:

For the template update interval "$interval_very_slow" is used.

wp_daily_energy_combined.yaml has no interval set and uses the default update interval of 60s.

I suggest to skip the interval for the templates and use the default of 60 s

@kr0ner
Copy link
Owner Author

kr0ner commented Nov 16, 2024

I find this a bit confusing with the SUMME_KWH vs. SUMME_MWH endings - can't you just remove the postfix (marked in bold) here for the "logical" sensors in the yaml? (see https://github.com/mkaiser/OneESP32ToRuleThemAll/blob/5d22afd6af31382cffb12075dc4e55950c0c8c01/yaml/ttf07.yaml#L226)

"WAERMEERTRAG_2WE_WW_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_2WE_WW_SUMME_MWH" , scaled_property: "WAERMEERTRAG_2WE_WW_SUM_KWH" , property: "WAERMEERTRAG_2WE_WW_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }} WAERMEERTRAG_2WE_HEIZ_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_2WE_HEIZ_SUMME_MWH" , scaled_property: "WAERMEERTRAG_2WE_HEIZ_SUM_KWH", property: "WAERMEERTRAG_2WE_HEIZ_SUM_MWH", unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }} WAERMEERTRAG_WW_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_WW_SUMME_MWH" , scaled_property: "WAERMEERTRAG_WW_SUM_KWH" , property: "WAERMEERTRAG_WW_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }} WAERMEERTRAG_HEIZ_SUMME_MWH: !include { file: wp_generic_combined.yaml, vars: { sensor_name: "WAERMEERTRAG_HEIZ_SUMME_MWH" , scaled_property: "WAERMEERTRAG_HEIZ_SUM_KWH" , property: "WAERMEERTRAG_HEIZ_SUM_MWH" , unit: "MWh", accuracy_decimals: "3", icon: "mdi:fire" }}

... and the same for the electrical energy EL_AUFNAHMELEISTUNG (yeah, I promised the PR to rename it to EL_AUFNAHMELEISTUNG, but I like to get my stuff working before)

Also maybe change SUMME to TOTAL?

I am really confused :)

Yes, makes sense to remove them ... they anyways have a unit ... makes no sense to put it in the name again.

Let's not change too many things at once ... removing the postfix and renaming AUFNAHMELEISTUNG to ENERGIEAUFNAHME should be good enough for now. The code itself is written in english, just some variables have german names so that it reflects what is visible in the heatpump display .... not even sure if those are sold outside Germany ^^^
If we go with english names, I'd do this in a central place like property.h

Edit: got it wrong ... SUM is the property name ... SUMME is the resulting sensor 😉

@kr0ner
Copy link
Owner Author

kr0ner commented Nov 16, 2024

Another thing coming to my mind:

For the template update interval "$interval_very_slow" is used.

wp_daily_energy_combined.yaml has no interval set and uses the default update interval of 60s.

I suggest to skip the interval for the templates and use the default of 60 s

Actually it should be set to "never" for the sake of making it obvious. #56
Since there is no lambda, nothing will happen anyways.
The value is updated every 5 minutes via schedule.

Every hour, every 5 minutes except midnight.

@kr0ner
Copy link
Owner Author

kr0ner commented Nov 18, 2024

/test

Copy link

The GitHub Actions build has completed successfully. Check the workflow run details for more information.

@kr0ner kr0ner merged commit 310e265 into master Nov 18, 2024
@github-actions github-actions bot deleted the cop branch December 25, 2024 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants