Skip to content

Conversation

@VallahDieWaldfee
Copy link
Contributor

@VallahDieWaldfee VallahDieWaldfee commented Jan 2, 2026

Add support to setmaxpower

@evcc-bot evcc-bot added devices Specific device support enhancement New feature or request labels Jan 2, 2026
@VallahDieWaldfee VallahDieWaldfee marked this pull request as ready for review January 2, 2026 22:15
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In the new setmaxpower sequence, register 2741 is declared with type: input, which in other Modbus templates is used for read-only registers; consider changing this to the appropriate writable type (e.g. holding) for consistency with how write operations are modeled.
  • The first step in setmaxpower sets a constant value of 1 before writing to register 2741; if setmaxpower is meant to accept a dynamic power limit, consider wiring the requested value through instead of hardcoding 1.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the new `setmaxpower` sequence, register 2741 is declared with `type: input`, which in other Modbus templates is used for read-only registers; consider changing this to the appropriate writable type (e.g. `holding`) for consistency with how write operations are modeled.
- The first step in `setmaxpower` sets a constant value of `1` before writing to register 2741; if `setmaxpower` is meant to accept a dynamic power limit, consider wiring the requested value through instead of hardcoding `1`.

## Individual Comments

### Comment 1
<location> `templates/definition/charger/nibe-s-series.yaml:82` </location>
<code_context>
       type: input
       encoding: uint16
     scale: 0.1
</code_context>

<issue_to_address>
**issue (bug_risk):** Using an `input` register type for what appears to be a write operation may be incorrect

In the first `setmaxpower` step, the nested `modbus` block appears to write value `1` to register `2741`, but it’s declared as `type: input`. Writable Modbus registers are typically `holding`; if `input` is treated as read‑only by the template engine, this write may fail or behave unpredictably. Please confirm `2741` in the NIBE spec and switch to `type: holding` if it’s meant to be writable.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig
Copy link
Member

andig commented Jan 3, 2026

@VallahDieWaldfee lgtm. Hast Du ausprobiert, ob das auch tatsächlich einen Effekt hat?

@andig andig changed the title Add setmaxpower for Nibe-S Series Nibe-S Series: add max power Jan 3, 2026
@VallahDieWaldfee
Copy link
Contributor Author

@andig ja, manuell ausgeführt klappt das. Zugelassen sind Werte ab 1,1kw in 100w Schritten.

Schöner wäre es eigentlich noch das Register 2741 bei nicht vorhandener Begrenzung auf 0 zu setzen. Da hab ich aber keinen Weg zu gefunden.

@andig
Copy link
Member

andig commented Jan 3, 2026

Das müsste doch einfach passieren wenn mode=normal gesetzt wird?

@VallahDieWaldfee
Copy link
Contributor Author

also wird die maxpower nur angesteuert, wenn der mode auf blocked ist? dann wird es nicht wie von mir gedacht funktionieren.

Die WP sind so eingestellt, dass mode blocked immer den Verdichter + Zusatzheizung komplett blockiert. dann ist es auch egal, wie der Maximalstrom eingestellt ist.

unabhängig davon kann man über die beiden verwendeten register einen Maximalstrom einstellen. wäre für §14a imo sinnvoller.

@andig
Copy link
Member

andig commented Jan 3, 2026

also wird die maxpower nur angesteuert, wenn der mode auf blocked ist?

Blocked? Es wird beim ein/ausschalten aufgerufen:

// Enable implements the api.Charger interface
func (wb *Heatpump) Enable(enable bool) error {
	var power int64
	if enable {
		power = wb.power
	}
	return wb.setMaxPower(power)
}

oder

// Enable implements the api.Charger interface
func (wb *SgReady) Enable(enable bool) error {
	mode := map[bool]int64{false: Normal, true: Boost}[enable]

	if err := wb.modeS(mode); err != nil {
		return err
	}

	wb.mode = mode

	return wb.setMaxPower(wb.power)
}

D.h. bei disable wird Leistung=0 gesetzt. Ein/ausschalten i.S. von Wärmepumpen heißt boost oder nicht.

Die WP kann sich im "normal" Modus natürlich entscheiden das zu ignorieren. Statt 1/0 bei setMaxPower zu ändern könntest Du Dich- wenn das besser funktioniert- das auch jeweils in den Modbus normal/boost mit rein packen.

@VallahDieWaldfee
Copy link
Contributor Author

mit blocked meinte ich den reduced zustand (1).
Ich bin davon ausgegangen, dass der wert zur §14a Regelung genutzt wird, nicht zur Leistungsbegrenzung im boost. Daher die Verwirrung.

ich schau mir das nochmal näher an. Danke für die Aufklärung

@VallahDieWaldfee VallahDieWaldfee marked this pull request as draft January 3, 2026 16:29
@andig
Copy link
Member

andig commented Jan 3, 2026

Dim Modus wäre nochmal was Anderes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants