RCT: add curtailment api

[Maschga]

Depends on #26323

\cc @andig

[andig]

decorate.go kennt das noch nicht

[Maschga]

Ich glaub, so geht das auch nicht.

[Maschga]

decorate.go kennt das noch nicht

wie kann man das ändern?

[sourcery-ai]

Hey - I've found 4 issues, and left some high level feedback:

• The decorateRCT switch has grown significantly with many near-identical cases; consider refactoring the generator or using a more data-driven approach to avoid this combinatorial explosion and make future extensions (like additional interfaces) less error-prone.
• In parseFunctions/splitTopLevel, malformed type specs (e.g. odd number of elements or unbalanced parentheses) are silently tolerated; adding validation with a clear error path would make misconfigured -t arguments fail fast instead of generating incorrect code.
• The RCT.curtailed flag is only maintained locally and never read back from the device, so Curtailed() may return stale information after process restarts or external changes; consider reading the actual curtailment state from RCT or documenting that this is a best-effort cached value.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `decorateRCT` switch has grown significantly with many near-identical cases; consider refactoring the generator or using a more data-driven approach to avoid this combinatorial explosion and make future extensions (like additional interfaces) less error-prone.
- In `parseFunctions`/`splitTopLevel`, malformed type specs (e.g. odd number of elements or unbalanced parentheses) are silently tolerated; adding validation with a clear error path would make misconfigured `-t` arguments fail fast instead of generating incorrect code.
- The `RCT.curtailed` flag is only maintained locally and never read back from the device, so `Curtailed()` may return stale information after process restarts or external changes; consider reading the actual curtailment state from RCT or documenting that this is a best-effort cached value.

## Individual Comments

### Comment 1
<location> `cmd/decorate/decorate.go:310-314` </location>
<code_context>
+	return res
+}
+
+func parseFunctions(iface string) []function {
+	parts := splitTopLevel(iface)
+
+	var res []function
+	for i := 0; i+1 < len(parts); i += 2 {
+		res = append(res, function{
+			function:  parts[i],
</code_context>

<issue_to_address>
**issue:** Consider validating that the function specification has an even number of elements instead of silently dropping a trailing token.

Because `parseFunctions` loops with `for i := 0; i+1 < len(parts); i += 2`, any trailing unmatched element in an odd-length `parts` slice is silently ignored, hiding configuration mistakes. Instead, explicitly check `len(parts)%2 == 0` and fail fast (e.g., return an error or panic) on malformed input rather than truncating it implicitly.
</issue_to_address>

### Comment 2
<location> `cmd/decorate/decorate.go:288-297` </location>
<code_context>
 	return scanner.Err()
 }

+func splitTopLevel(s string) []string {
+	var res []string
+	brackets := 0
+	start := 0
+
+	for i, r := range s {
+		switch r {
+		case '(':
+			brackets++
+		case ')':
+			brackets--
+		case ',':
+			if brackets == 0 {
+				res = append(res, strings.TrimSpace(s[start:i]))
+				start = i + 1
+			}
+		}
+	}
+	res = append(res, strings.TrimSpace(s[start:]))
+	return res
+}
</code_context>

<issue_to_address>
**suggestion:** splitTopLevel should probably detect mismatched parentheses or nested function types instead of assuming balanced input.

Because `splitTopLevel` always appends the final slice regardless of `brackets`, malformed signatures (e.g., missing `)`) will still produce output and shift errors downstream. It will also incorrectly split if you later support nested `func(...)` parameter types. Consider validating `brackets == 0` at the end and returning an error (or otherwise failing fast) for malformed input so issues surface at codegen time rather than later.

Suggested implementation:

```golang
func splitTopLevel(s string) []string {
	var res []string
	brackets := 0
	start := 0

	for i, r := range s {
		switch r {
		case '(':
			brackets++
		case ')':
			brackets--
			// Fail fast on mismatched closing parenthesis so we don't
			// silently produce an incorrect split.
			if brackets < 0 {
				panic("splitTopLevel: unmatched closing parenthesis in signature")
			}
		case ',':
			if brackets == 0 {
				res = append(res, strings.TrimSpace(s[start:i]))
				start = i + 1
			}
		}
	}

	// If brackets is not zero here, we have unbalanced parentheses and
	// should fail fast rather than returning a potentially misleading split.
	if brackets != 0 {
		panic("splitTopLevel: unbalanced parentheses in signature")
	}

	res = append(res, strings.TrimSpace(s[start:]))
	return res
}

```

If you prefer not to panic, you could instead change the signature to `func splitTopLevel(s string) ([]string, error)` and replace the `panic` calls with `return nil, errors.New("...")`, then propagate and handle the error at all call sites. This would require updating the function calls to check and handle the returned error.
</issue_to_address>

### Comment 3
<location> `meter/rct.go:25-28` </location>
<code_context>
 	conn          *rct.Connection // connection with the RCT device
 	usage         string          // grid, pv, battery
 	externalPower bool            // whether to query external power
+	curtailed     bool            // whether pv is currently curtailed
 }

</code_context>

<issue_to_address>
**issue (bug_risk):** The in-memory `curtailed` flag may become stale or racy; consider querying device state or synchronizing access.

`curtailed` is only set inside the local `curtail` closure and then read via `Curtailed()`, so:
- After a process restart it will always be `false`, regardless of the inverter’s real state.
- If another client curtails the device, this value will be stale.
- Concurrent `Curtail` / `Curtailed` calls can race on `m.curtailed`.
Depending on how `api.Curtailer` is used, consider either reading the live curtailment state from the device in `Curtailed()` or protecting `m.curtailed` with synchronization (or clearly documenting that this is only a best-effort local view).
</issue_to_address>

### Comment 4
<location> `meter/rct_decorators.go:9` </location>
<code_context>
 )

-func decorateRCT(base *RCT, meterEnergy func() (float64, error), battery func() (float64, error), batterySocLimiter func() (float64, float64), batteryController func(api.BatteryMode) error, batteryCapacity func() float64) api.Meter {
+func decorateRCT(base *RCT, meterEnergy func() (float64, error), curtailer0 func(bool) error, curtailer1 func() (bool, error), battery func() (float64, error), batterySocLimiter func() (float64, float64), batteryController func(api.BatteryMode) error, batteryCapacity func() float64) api.Meter {
 	switch {
-	case battery == nil && meterEnergy == nil:
</code_context>

<issue_to_address>
**issue (complexity):** Consider replacing the large combinatorial switch in decorateRCT with a linear sequence of conditional decorator wrappers to avoid enumerating every interface combination.

The new cases for Curtailer clearly amplify the existing combinatorial pattern. Since this file is generated, you can usually get the same behavior with a *linear* decorator composition instead of enumerating every combination in a switch.

Concretely, instead of:

```go
func decorateRCT(...) api.Meter {
	switch {
	case battery != nil && batteryCapacity == nil && batteryController == nil && batterySocLimiter == nil && curtailer0 != nil && curtailer1 != nil && meterEnergy != nil:
		return &struct {
			*RCT
			api.Battery
			api.Curtailer
			api.MeterEnergy
		}{
			RCT: base,
			Battery: &decorateRCTBatteryImpl{ battery: battery },
			Curtailer: &decorateRCTCurtailerImpl{ curtailer0: curtailer0, curtailer1: curtailer1 },
			MeterEnergy: &decorateRCTMeterEnergyImpl{ meterEnergy: meterEnergy },
		}
	// ... dozens more branches ...
	}
	return nil
}
```

you can generate a *single* linear composition that conditionally wraps the base with the needed interfaces:

```go
func decorateRCT(
	base *RCT,
	meterEnergy func() (float64, error),
	curtailer0 func(bool) error,
	curtailer1 func() (bool, error),
	battery func() (float64, error),
	batterySocLimiter func() (float64, float64),
	batteryController func(api.BatteryMode) error,
	batteryCapacity func() float64,
) api.Meter {
	var m api.Meter = base

	if battery != nil {
		m = &struct {
			api.Meter
			api.Battery
		}{
			Meter:   m,
			Battery: &decorateRCTBatteryImpl{battery: battery},
		}
	}

	if batteryCapacity != nil {
		m = &struct {
			api.Meter
			api.BatteryCapacity
		}{
			Meter:          m,
			BatteryCapacity: &decorateRCTBatteryCapacityImpl{batteryCapacity: batteryCapacity},
		}
	}

	if batteryController != nil {
		m = &struct {
			api.Meter
			api.BatteryController
		}{
			Meter:            m,
			BatteryController: &decorateRCTBatteryControllerImpl{batteryController: batteryController},
		}
	}

	if batterySocLimiter != nil {
		m = &struct {
			api.Meter
			api.BatterySocLimiter
		}{
			Meter:            m,
			BatterySocLimiter: &decorateRCTBatterySocLimiterImpl{batterySocLimiter: batterySocLimiter},
		}
	}

	if curtailer0 != nil && curtailer1 != nil {
		m = &struct {
			api.Meter
			api.Curtailer
		}{
			Meter:     m,
			Curtailer: &decorateRCTCurtailerImpl{curtailer0: curtailer0, curtailer1: curtailer1},
		}
	}

	if meterEnergy != nil {
		m = &struct {
			api.Meter
			api.MeterEnergy
		}{
			Meter:       m,
			MeterEnergy: &decorateRCTMeterEnergyImpl{meterEnergy: meterEnergy},
		}
	}

	return m
}
```

This preserves the current semantics:

- Only the interfaces for which funcs are non-nil are implemented (same as now).
- You still use anonymous structs embedding the relevant interfaces, so callers’ type assertions keep working.
- Adding another optional interface becomes a single `if` block instead of multiplying the switch cases.

The main change would be in the code generator: emit a linear sequence of `if <feature> != nil { wrap }` blocks instead of a full switch over all combinations.
</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.}

[andig]

Mit jedem weiteren Interface steigt die kombinatorische Komplexität. #26357 soll dazu dienen, diese nach Usage aufzusplitten und damit die Anzahl der Kombinationen und damit Codezeilen zu reduzieren.

[Maschga]

Don't know why Sourcery-Ai doesn't like my PR.
Anyway, awaiting your review. :)