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

Refactor green share calculation #4760

Closed
wants to merge 1 commit into from

Conversation

naltatis
Copy link
Member

@naltatis naltatis commented Oct 8, 2022

  • moved calculation from savings.go to site.go to make result reusable in savings, telemetry and sessions (later)
  • simplified tests
  • publish greenShare value
    • 0 means currently no self-produced energy used
    • 1 means all usage is covered by pv or battery

@naltatis naltatis requested a review from andig October 8, 2022 15:54
@naltatis naltatis added the infrastructure Basic functionality label Oct 8, 2022
Copy link
Contributor

@Hofyyy Hofyyy left a comment

Choose a reason for hiding this comment

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

Hi @naltatis,

Ich habe alles einmal im Detail durchgelesen und kann erstmal nichts negatives finden.
Auch wenn ich noch keinen sehr großen Einblick in die EVCC Architektur habe.

Ein Punkt den ich anmerken könnte:

  • Refactoring wenn möglich entweder "Form" oder "Function".

In dem Fall wären zwei Commits mega. Ein Commit der alles Verschiebt, aber kein Verhalten ändert (Tests bleiben gleich). Und ein zweiter Commit der auf den Greenshare umstellt. Das macht das Review einfacher und reduziert Fehler.

@naltatis
Copy link
Member Author

@andig rein damit?

// greenShare returns the percentage of self produced energy currently used at the site. 0 = full-grid; 1 = full pv/battery.
func (site *Site) greenShare() float64 {
batteryDischarge := math.Max(0, site.batteryPower)
batteryCharge := math.Min(0, site.batteryPower) * -1
Copy link
Member

Choose a reason for hiding this comment

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

:= -...

Copy link
Member

Choose a reason for hiding this comment

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

Magst die komplizierte Minusrechnung noch anpassen?

Copy link
Member

Choose a reason for hiding this comment

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

ping

@github-actions github-actions bot added stale Outdated and ready to close and removed stale Outdated and ready to close labels Dec 28, 2022
@github-actions github-actions bot added the stale Outdated and ready to close label Jan 29, 2023
@github-actions github-actions bot closed this Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Basic functionality stale Outdated and ready to close
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants