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

Improve energy settings dialog #15205

Merged
merged 3 commits into from Jan 30, 2023
Merged

Improve energy settings dialog #15205

merged 3 commits into from Jan 30, 2023

Conversation

emontnemery
Copy link
Collaborator

@emontnemery emontnemery commented Jan 25, 2023

Proposed change

Improve energy settings dialogs:

  • Remove hard coded units from translation strings and instead fetch units from backend
  • Don't include units in input labels
  • Fix bugs related to setting up costs for external statistics

Screenshot:
image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration


Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

)}
>
<ha-radio
value="statistic"
name="costs"
.checked=${this._costs === "statistic"}
.disabled=${externalSource}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was wrong: A sensor or external statistics which tracks the total cost is supported both when the consumed gas is tracked by an external statistics or by a sensor. A price entity or a fixed price is only supported when the consumed gas is tracked by a sensor.

Same change for for grid and water.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @emontnemery,

I'm trying to understand why I cannot use a fixed price on external statistics but I cannot find any related documentation.

Could you please tell me where I can find more information about it? I doesn't make sense to me

Thanks!

Comment on lines +104 to +105
this._source.stat_energy_from &&
isExternalStatistic(this._source.stat_energy_from);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check if the gas statistic is external, not if the cost statistic is external.

Copy link
Member

@piitaya piitaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with the change.
I didn't have the answer for your questions about external statistics. May be @bramkragten ?

@emontnemery
Copy link
Collaborator Author

I didn't have the answer for your questions about external statistics. May be @bramkragten ?

I just added the comment to explain the change, the old code was clearly wrong.

Co-authored-by: Paul Bottein <paul.bottein@gmail.com>
@bramkragten bramkragten merged commit 8212a5a into dev Jan 30, 2023
@bramkragten bramkragten deleted the improve_energy_dialogs branch January 30, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants