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

Enable Statistics Graph card to integrate with Energy Dashboard #21105

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

madsciencetist
Copy link

@madsciencetist madsciencetist commented Jun 17, 2024

Proposed change

Adds boolean option energy_date_selection to Statistics Graph card. If
true, the graph will set its time range to that specified by an Energy
Date Picker card on the same dashboard, similar to the plots on the
Energy Dashboard. Otherwise, days_to_show will be used as before.

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

- type: custom:energy-period-selector-plus
  card_background: false
  today_button: true
  prev_next_buttons: true
  compare_button_type: ''
  today_button_type: text
  period_buttons:
    - day
    - week
    - month
    - year
    - custom
  default_period: day
  rolling_periods: true
- type: statistics-graph
  chart_type: bar
  period: hour
  entities:
    - sensor.main_123_1d
  stat_types:
    - change
  days_to_show: 1
  title: Statistics - Hourly
  hide_legend: true
  energy_date_selection: true

Additional information

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:

Summary by CodeRabbit

  • New Features
    • Added energy data handling and subscription management in the statistics graph card.
    • Introduced an optional "Energy Date Picker" for more flexible date selection in energy statistics.
  • Enhancements
    • Updated user interface to support the new "Energy Date Picker" feature.
  • Localization
    • Added English translation for the "Energy Date Picker" option.

Copy link
Contributor

coderabbitai bot commented Jun 17, 2024

Walkthrough

Walkthrough

The changes enhance the HuiStatisticsGraphCard class in the Lovelace UI to support energy data management and user subscription handling. The StatisticsGraphCardConfig interface now includes an option for energy date selection, and the corresponding editor and translations have been updated accordingly. These modifications enable more refined control over energy data display and user preferences within the statistics graph card.

Changes

File Path Change Summary
src/panels/lovelace/cards/hui-statistics-graph-card.ts Added imports for energy data handling, extended HuiStatisticsGraphCard with subscription capabilities, updated methods to incorporate energy data, and introduced a new hassSubscribe method.
src/panels/lovelace/cards/types.ts Modified StatisticsGraphCardConfig to extend EnergyCardBaseConfig and added a new property energy_date_selection.
src/panels/lovelace/editor/config-elements/hui-statistics-graph-card-editor.ts Enhanced to include the new energy_date_selection property in cardConfigStruct and updated switch case logic in HuiStatisticsGraphCardEditor class to handle this property.
src/translations/en.json Added a new translation key energy_date_selection for an option to use the Energy Date Picker instead of Days to Show.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HuiStatisticsGraphCard
    participant EnergyAPI
    participant SubscriptionService
    
    User->>HuiStatisticsGraphCard: Modify card configuration
    note right of HuiStatisticsGraphCard: Handles new property `energy_date_selection`
    
    HuiStatisticsGraphCard->>SubscriptionService: Subscribe to updates
    SubscriptionService-->>HuiStatisticsGraphCard: Provide updates
    
    HuiStatisticsGraphCard->>EnergyAPI: Request energy data (with optional date selection)
    EnergyAPI-->>HuiStatisticsGraphCard: Return energy data
    
    HuiStatisticsGraphCard-->>User: Display updated statistics graph
Loading

Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 95cebdb and 7458a88.

Files selected for processing (2)
  • src/panels/lovelace/cards/hui-statistics-graph-card.ts (8 hunks)
  • src/panels/lovelace/cards/types.ts (2 hunks)
Additional context used
Biome
src/panels/lovelace/cards/hui-statistics-graph-card.ts

[error] 88-88: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 254-254: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)


[error] 259-259: This variable implicitly has the any type.

Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.

(lint/suspicious/noImplicitAnyLet)


[error] 260-260: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)


[error] 281-281: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 285-285: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)

Additional comments not posted (2)
src/panels/lovelace/cards/hui-statistics-graph-card.ts (1)

Line range hint 190-195: Verify timer management logic.

Ensure the timer is cleared before setting a new one to avoid multiple timers running simultaneously.

src/panels/lovelace/cards/types.ts (1)

Line range hint 338-348: Verify consistency with related documentation and usage.

Ensure that the changes to the StatisticsGraphCardConfig interface, including extending EnergyCardBaseConfig and adding the energy_date_selection property, are consistent with the related documentation and usage.


Tip

Early access features: enabled

We are currently testing the following features in early access:

  • OpenAI gpt-4o model for code reviews and chat: OpenAI claims that this model is better at understanding and generating code than the previous models. We seek your feedback over the next few weeks before making it generally available.

Note:

  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
  • Please join our Discord Community to provide feedback and report issues.
  • OSS projects are currently opted into early access features by default.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@madsciencetist madsciencetist force-pushed the statistics_plot_use_energy_dates branch from 0cc1302 to 16e6129 Compare June 17, 2024 20:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Comment on lines +82 to +85
public hassSubscribe() {
if (!this._config?.energy_date_selection) {
return [];
}
Copy link
Member

Choose a reason for hiding this comment

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

If the config changes, the subscriptions are now not updated, so behaviour in the editor will be wrong.

Copy link
Author

Choose a reason for hiding this comment

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

Can you explain more? Are you saying that if a user uses the editor to uncheck energy_date_selection, nothing will force the plot to be redrawn with the now-static time period?

Is this moot now that I reverted the editor changes?

Copy link
Member

Choose a reason for hiding this comment

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

The subscription is created on init of the element, so if some one enables the energy_date_selection option after the element was initialised, it will not add the subscription and thus miss data/not work.

This is not just for the visible editor, but also for the YAML editor.

Copy link
Member

Choose a reason for hiding this comment

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

I still think we can not use hassSubscribe here, it will not work in the card editor.

@bramkragten
Copy link
Member

Why is energy_date_selection the config key and not collection_key?

Adds boolean option `energy_date_selection` to Statistics Graph card. If
true, the graph will set its time range to that specified by an Energy
Date Picker card on the same dashboard, similar to the plots on the
Energy Dashboard.
PR reviewer suggested it was too confusing, especially without
dynamically hiding the overridden days_to_show option.
and instead activate when collection_key is specified
@madsciencetist madsciencetist force-pushed the statistics_plot_use_energy_dates branch from 16e6129 to 95cebdb Compare June 24, 2024 18:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

@madsciencetist
Copy link
Author

madsciencetist commented Jun 24, 2024

@bramkragten

Why is energy_date_selection the config key and not collection_key?

95cebdb changes the behavior to trigger when collection_key is set rather then when energy_date_selection is true. However...this removes the possibility of using the "default" collection key, i.e. what happens when getEnergyDataCollection is called with no key.

What is the intended behavior? Is it acceptable to call getEnergyDataCollection with no key? It's quite awkward because if called without a key, the key is implicitly set to "energy", but if you try to getEnergyDataCollection(key = "energy"), it complains that Key needs to start with energy_ (with trailing underscore). Or is that a bug?

I'm ultimately trying to interop with two other community integrations here, energy-period-selector-plus and ha-sankey-chart, so if collection_key is supposed to be required, then those two are bugged (in fact the latter doesn't support collection_key at all). So unless the Statistics Graph collection_key is allowed to be empty, there is no way to make a Statistics Graph and Sankey Chart show data from the same time period, which is ultimately my goal here.

Thus, I'm pretty sure I should revert 95cebdb and restore the energy_date_selection config param, but I'll wait for your advice.

@bramkragten
Copy link
Member

Yeah collection_key should be able to be empty, as it would use the default time picker used in the energy dashboard. So reverting seems fair.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

getEnergyDataCollection(this.hass!, {
key: this._config?.collection_key,
}).subscribe((data) => {
this._setFetchStatisticsTimer(data);
Copy link
Member

Choose a reason for hiding this comment

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

You should not pass the data to this function, as the function is also called in other places, save the data you need (the dates) in the class and then call the _setFetchStatisticsTimer function.

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

2 participants