Collect 15min energy metrics

[andig]

@naltatis hier der erste Aufschlag für 15min Metriken. Daten werden jeweils für 15min gesammelt und dann in die metrics Tabelle geschrieben.

TODO

• entities (CRUD)
• add meter usage column (grid, pv, ...)
• split consumption and production
• use energy where possible
• add reading metrics from DB

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• The Collector.process method only triggers persistence when slotStart.Sub(c.started) == SlotDuration which may skip slots if calls are delayed; consider using >= to catch full slots robustly.
• pvEnergy accumulators are initialized with a fresh clock.New() in Boot instead of the shared site clock, leading to mismatched time bases; use the same clock instance throughout.
• The updateHomeConsumption method is no longer used after inlining AddPower in update; remove it or refactor update to call it to avoid dead code and duplicated error handling.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- The Collector.process method only triggers persistence when slotStart.Sub(c.started) == SlotDuration which may skip slots if calls are delayed; consider using >= to catch full slots robustly.
- pvEnergy accumulators are initialized with a fresh clock.New() in Boot instead of the shared site clock, leading to mismatched time bases; use the same clock instance throughout.
- The updateHomeConsumption method is no longer used after inlining AddPower in update; remove it or refactor update to call it to avoid dead code and duplicated error handling.

## Individual Comments

### Comment 1
<location> `core/site.go:257` </location>
<code_context>

 // NewSite creates a Site with sane defaults
 func NewSite() *Site {
+	clock := clock.New()
+
</code_context>

<issue_to_address>
Multiple clock.New() calls may lead to inconsistent time sources.

Previously, each meterEnergy had its own clock. If you need consistent timekeeping, share the same clock instance across all metrics objects.

Suggested implementation:

```golang
 // NewSite creates a Site with sane defaults
 func NewSite() *Site {
 	clock := clock.New()

```

```golang
		site.pvMeters = append(site.pvMeters, dev)

		// accumulator
		site.pvEnergy[ref] = metrics.NewAccumulator(clock)
	}

```
</issue_to_address>

### Comment 2
<location> `core/metrics/collector.go:22` </location>
<code_context>
+	}
+}
+
+func (c *Collector) process(fun func()) error {
+	now := c.accu.clock.Now()
+
</code_context>

<issue_to_address>
Consider removing the process closure and flattening slot logic by extracting a helper and inlining steps in each Add* method.

```suggestion
You can remove the closure indirection and flatten the slot‐boundary logic by:

1. Extracting slot rollover into a simple helper (`handleSlot`).
2. Inlining `Now()` + zero‐check + accumulator call in each `Add*` method.

This keeps all behavior but removes the extra abstraction:

```go
func (c *Collector) handleSlot(now time.Time) error {
    slot := now.Truncate(SlotDuration)
    if slot.After(c.started) {
        // exactly one full slot elapsed ⇒ persist
        if slot.Sub(c.started) == SlotDuration {
            if err := Persist(c.started, c.accu.AccumulatedEnergy()); err != nil {
                return err
            }
        }
        c.started = slot
        c.accu.Accumulated = 0
    }
    return nil
}

func (c *Collector) AddEnergy(v float64) error {
    now := c.accu.clock.Now()
    if c.accu.updated.IsZero() {
        c.started = now
    }
    c.accu.AddEnergy(v)
    return c.handleSlot(now)
}

func (c *Collector) AddMeterTotal(v float64) error {
    now := c.accu.clock.Now()
    if c.accu.updated.IsZero() {
        c.started = now
    }
    c.accu.AddMeterTotal(v)
    return c.handleSlot(now)
}

func (c *Collector) AddPower(v float64) error {
    now := c.accu.clock.Now()
    if c.accu.updated.IsZero() {
        c.started = now
    }
    c.accu.AddPower(v)
    return c.handleSlot(now)
}
```

This removes the `process` closure, flattens the nested `if`s, and makes each step explicit.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[naltatis]

I'd suggest keeping it simpler and not introduce a foreign key relation. This is something we'd then have to manually manage. My suggestion would be to introduce a mandatory "type" (or category) and an optional "name" (device).

Types: home, pv, battery_import, battery_export, grid, feedin, charger, aux, ext, solar_forecast, solar_forecast_plus_24h

The types are similar to your existing meter types but they dont have to be limited to that (see home).

Name: db:12, charger_goe, ...

Advantages:

• fast to query and group (one table, no joins)
• query by specific device (charger_goe) or by type (pv)
• no need to manage entity table (what happens if user removes charger_goe from yaml or renames)?
• overall metrics (this year pv production) stay complete, even if you remove/replace a device
• easy to read and manually cleanup (human)

Disadvantages:

• slightly more data stored (strings instead of ids). SQLite is good in optimizing/compressing this! Possible optimization: We could store type as number from Go type enum. Similar to configs.class field.

Naming: Lets use energy instead of val. We'll likely want to add additional (optional) fields like cost, co2, ... later.

[andig]

Es wäre gut, wenn wir die Diskussion von den Anforderungen statt von der Implementierung her führen würden. Fwiw:

I'd suggest keeping it simpler and not introduce a foreign key relation. This is something we'd then have to manually manage.

That's already done in the current code.

My suggestion would be to introduce a mandatory "type" (or category) and an optional "name" (device).

Mir ist nicht klar, wie wir die verwenden würden bzw. zu welchem Zweck wir Beide Attribute brauchen?

no need to manage entity table (what happens if user removes charger_goe from yaml or renames)?

Gleicher Punkt wie drüber- ich habe noch kein klares Bild, wie wir die verwenden würden.

overall metrics (this year pv production) stay complete, even if you remove/replace a device

Super use case! Wäre ein weiterer Grund für eine entity Tabelle- dort würde dann auch die Kategorie hin gehören.

Naming: Lets use energy instead of val. We'll likely want to add additional (optional) fields like cost, co2, ... later.

In einem orthogonalen Schema nicht.

fast to query and group (one table, no joins)

Joins sind perfekt für Geschwindigkeit ;)

[andig]

battery_import, battery_export, grid, feedin

Wollen wir Import/Export separat modellieren oder sind das zwei Seiten einer Medaille? Letzteres hätte den Vorteil, dass der Accumulator das intern handlen könnte (=2 Spalten pos/neg). Anderenfalls brauchst Du immer zwei Updates damit die "andere" Seite weiss, dass sich der Timestamp des letzten Updates geändert hat, auch wenn es keine Wertänderung gibt.

[naltatis]

Naming: Lets use energy instead of val. We'll likely want to add additional (optional) fields like cost, co2, ... later.

In einem orthogonalen Schema nicht.

Ich weiß nicht, was du mit ortohonalem schema meinst, aber die Kosten für diese geloggt Energiemenge würde ich damit direkt verbunden sehen und nicht noch eine allgemein Kostenloggingtabelle aufmachen.

[naltatis]

Naming: Pos/Neg ist sehr abstrakt/technisch. Um hier ne saubere Datenbasis zu bekommen werden wir ja auch bei den den Netz-/Batteriezählern das jeweils andere Meter hinzufügen müssen (zumindest mittelfristig). Wie würden wir denn das nennen (energy[In|Out], import|export, energy & energyReverse)? Ich fänd gut, wenn wir das Naming dann hier bei den Metriken "kompatibel" haben.

[andig]

Naming: Pos/Neg ist sehr abstrakt/technisch.

Es ist auch eine Datenbanktabelle! Das Naming entspricht 1:1 der evcc Logik der Energieflüsse. Ich wollte da keine neuen Begriffe erfinden. Import/Export sind mindestens bei der Batterie zweideutig.

Was wir aktuell übrigens (leider) nicht können ist hier mit Zählerständen zu agieren da wir die für 2-Richtungszähler mangels APIs nicht kennen sondern immer nur eine Richtung ausgeprägt haben...

[premultiply]

Zwei Einwürfe:

• sollten wir die Erzeugung perspektivisch auf die einzelnen MPPTs aufsplitten um so einzele Strings besser loggen/visualisieren/vergleichen zu können?
• Import/Export wäre das imho das Gescheiteste, bräuchte aber eine Migration der aktuellen "energy-API" bei der wir uns immer auf die für uns primäre Flussrichtung festgelegt haben.

Aktuell erwartetes Energy-Register-Mapping:

• pv: Export
• battery: Export
• grid: Import
• charger: Import

[naltatis]

My suggestion would be to introduce a mandatory "type" (or category) and an optional "name" (device).

Mir ist nicht klar, wie wir die verwenden würden bzw. zu welchem Zweck wir Beide Attribute brauchen?

Unterschiedliche Arten von Auswertung. Einmal das klassische "Gibt mir mal ne Jahresübersicht meiner PV Produktion". Dann würde ich einfach nach Kategorie "pv" filtern, aufsummieren und fertig. Egal wie viele WRs an der Produktion beteiligt sind oder waren. Keine joins, einfach super simples group und sum.

Ich kann mir aber auch im Breakdown pro Wechselrichter vorstellen. Dann würde ich zusätzlich noch nach Gerät gruppieren.

Geht beides auch mit Joins, die Frage ist nur was wir dadurch wirklich gewinnen.

[andig]

Schau mal, ist schon umgesetzt. Aktuell:

• grid/device name
• pv/device name
• virtual/home

Für home habe ich eine Gruppe "virtueller" Zähler angedacht, die einen errechneten Meßwert repräsentieren (auch wenn an der Stelle egtl. nur "home" relevant ist). Lässt sich das Schema durchziehen?