Support for Hoymiles HMT/HMS DTU PRO S Modbus TCP

[jonilehtola]

Implementation of support of Hoymiles HMT/HMS microinverter own DTU PRO S. Requires that Modbus TCP is enabled in DTUs settings and probably requires using of LAN connection to the DTU.

Enables using Hoymiles own DTU instead of OpenDTU/AhoyDTU with frequent power intervals.

[sourcery-ai]

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

• In PanelPower, ReadHoldingRegisters is called with PORT_LENGTH defined as a byte length, but most Modbus clients expect a quantity in 16‑bit registers; double-check whether 0x28 should actually be half that value (e.g. 0x14) to avoid over-reading and index panics.
• The PanelPower method takes a conn *modbus.Connection parameter even though the struct already stores m.conn; consider dropping the parameter and using m.conn directly to simplify the API and avoid confusion.
• For noMoreHoymilesPanels, using errors.As on the error chain instead of a direct type assertion (if _, ok := err.(noMoreHoymilesPanels); ok) would make the sentinel error more robust if it ever gets wrapped.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `PanelPower`, `ReadHoldingRegisters` is called with `PORT_LENGTH` defined as a byte length, but most Modbus clients expect a quantity in 16‑bit registers; double-check whether `0x28` should actually be half that value (e.g. `0x14`) to avoid over-reading and index panics.
- The `PanelPower` method takes a `conn *modbus.Connection` parameter even though the struct already stores `m.conn`; consider dropping the parameter and using `m.conn` directly to simplify the API and avoid confusion.
- For `noMoreHoymilesPanels`, using `errors.As` on the error chain instead of a direct type assertion (`if _, ok := err.(noMoreHoymilesPanels); ok`) would make the sentinel error more robust if it ever gets wrapped.

## Individual Comments

### Comment 1
<location path="meter/hoymiles-dtu-mbtcp.go" line_range="108-113" />
<code_context>
+		m.log.TRACE.Printf("Panel %d: No more panels to read", panelIndex)
+		return 0, 0, noMoreHoymilesPanels{}
+	}
+	// power in 0x10 - 0x11, hex to int, divided by 10 to get actual power in watts
+	power := binary.BigEndian.Uint16(results[16:18]) / 10 // power is located at bytes 16-17 of the panel data, and is diveded by 10 to get the actual power in watts
+	// totalCumulativeProduction in 20 - 24
+	totalCumulativeProduction := binary.BigEndian.Uint32(results[20:24]) // total cumulative production is located at bytes 20-23 of the panel data, and is in Wh
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Preserve decimal precision when scaling the power value

The division is currently done in integer space (`Uint16(...) / 10`), so you lose the fractional digit before converting to `float64`. If the protocol defines one decimal place of precision, cast first, e.g. `power := float64(binary.BigEndian.Uint16(results[16:18])) / 10.0`, to preserve that precision.

```suggestion
	// power in 0x10 - 0x11, hex to float, divided by 10 to get actual power in watts (one decimal place precision)
	power := float64(binary.BigEndian.Uint16(results[16:18])) / 10.0 // power is located at bytes 16–17 of the panel data, and is divided by 10 to get the actual power in watts
	// totalCumulativeProduction in 20 - 24
	totalCumulativeProduction := binary.BigEndian.Uint32(results[20:24]) // total cumulative production is located at bytes 20–23 of the panel data, and is in Wh
	m.log.TRACE.Printf("Panel %d: Inverter Serial: %s, Port Number: %d, Power: %.1f W, Total Cumulative Production: %d Wh", panelIndex, inverterSerial, portNumber, power, totalCumulativeProduction)
	return power, float64(totalCumulativeProduction), nil
```
</issue_to_address>

### Comment 2
<location path="meter/hoymiles-dtu-mbtcp.go" line_range="94-103" />
<code_context>
+	results, err := conn.ReadHoldingRegisters(startRegister, PORT_LENGTH)
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against short Modbus responses before slicing fixed indices

This assumes `results` is at least 24 bytes and will panic on a short response (e.g. DTU glitch or protocol error). Please check `len(results)` before slicing and return a clear error if it’s too short for the expected layout.
</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]

Why can't this be implemented yaml-only?

[jonilehtola]

Why can't this be implemented yaml-only?

Because we need to sum an unknown N number of inverter port power values, and there can be up to 99 ports in one controller, which would also flood the DTU-controller, while most have only couple of panels installed. I didn't find a way to do this while-loop behavior in yaml-only. The Hoymiles DTU PRO Modbus TCP does not allow access to the Sunspec registers, only on RS485 mode.

[andig]

Because we need to sum an unknown N number of inverter port power values

Why?

[jonilehtola]

Because we need to sum an unknown N number of inverter port power values

Why?

In the Hoymiles DTU Modbus registers there are no place where the total power exists. I have been referring the documentation of (e.g. https://www.mikrocontroller.net/attachment/552319/Technical-Note-Modbus-implementation-using-3Gen-DTU-Pro-V1.2.pdf). Only place where the total power exists, which is required for evcc, is in Sunspec registers, which are unaccessible from Modbus TCP. The consistent way to get the total power is summing the power of microinverters.

[jonilehtola]

And also the HomeAssistant ModbusTCP integration does the power calculation exactly in similar manner accessing separate inverter powers.

[sourcery-ai]

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

• Consider using a context with timeout instead of context.Background() for the Modbus connection and/or per-register reads to avoid potential hangs if the DTU becomes unresponsive.
• The Modbus register layout in PanelPower (start address, port length, and byte offsets for serial, port, power, energy) is currently embedded as magic numbers; extracting these into well-named constants or a small struct would improve readability and make future protocol changes easier.
• The hardcoded upper bound of 99 panels in readCurrentValues would benefit from being a named constant or being derived from DTU capabilities, so the intent and any protocol/device limits are clearer.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider using a context with timeout instead of context.Background() for the Modbus connection and/or per-register reads to avoid potential hangs if the DTU becomes unresponsive.
- The Modbus register layout in PanelPower (start address, port length, and byte offsets for serial, port, power, energy) is currently embedded as magic numbers; extracting these into well-named constants or a small struct would improve readability and make future protocol changes easier.
- The hardcoded upper bound of 99 panels in readCurrentValues would benefit from being a named constant or being derived from DTU capabilities, so the intent and any protocol/device limits are clearer.

## Individual Comments

### Comment 1
<location path="meter/hoymiles-dtu-mbtcp.go" line_range="81-86" />
<code_context>
+// and modbus session, and returns the power in watts, or an error if the read fails
+// returns the power in watts, totalCumulativeProduction in Wh, a flag indicating whether a panel exists, or an error if the read fails
+func (m *HoymilesDTUModbusTcp) PanelPower(panelIndex int) (float64, float64, bool, error) {
+	PORT_LENGTH := uint16(0x28)      // length of the data for each panel in bytes
+	START_REGISTER := uint16(0x1000) // starting register for the first panel
+	// calculate the starting register for the panel based on the index
+	startRegister := START_REGISTER + uint16(panelIndex)*PORT_LENGTH
+	// read PORT_LENGTH bytes from the DTU starting at startRegister
+	results, err := m.conn.ReadHoldingRegisters(startRegister, PORT_LENGTH)
+	if err != nil {
+		return 0, 0, false, fmt.Errorf("Failed to read hoymiles-dtu panel %d: %w", panelIndex, err)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clarify whether PORT_LENGTH is in registers or bytes to avoid protocol confusion

`ReadHoldingRegisters` typically takes a register count (16‑bit values), not a byte length. If this API follows that convention, `0x28` would be 40 registers (80 bytes), not the 24 bytes you mention. Please either (a) treat `PORT_LENGTH` as a register count (and, if needed, introduce a separate byte-length constant), or (b) clearly document that this implementation uses a byte count so future readers don’t misinterpret it.

Suggested implementation:

```golang
func (m *HoymilesDTUModbusTcp) PanelPower(panelIndex int) (float64, float64, bool, error) {
	// PORT_LENGTH is a Modbus holding-register *count* (16‑bit values), not a byte length.
	// 0x28 registers = 40 registers = 80 bytes returned in the Modbus response.
	PORT_LENGTH := uint16(0x28)      // number of holding registers to read per panel
	START_REGISTER := uint16(0x1000) // starting register for the first panel
	// calculate the starting register for the panel based on the index
	startRegister := START_REGISTER + uint16(panelIndex)*PORT_LENGTH
	// read PORT_LENGTH holding registers from the DTU starting at startRegister
	results, err := m.conn.ReadHoldingRegisters(startRegister, PORT_LENGTH)

```

```golang
	// results is a raw byte slice: each register read above corresponds to 2 bytes here.
	// Check that results has at least 24 bytes (to read power and total cumulative production).
	if len(results) < 24 {

```
</issue_to_address>

### Comment 2
<location path="meter/hoymiles-dtu-mbtcp.go" line_range="114-116" />
<code_context>
+
+func (m *HoymilesDTUModbusTcp) readCurrentValues() (hoymilesDTUValues, error) {
+	var values hoymilesDTUValues
+	for i := 0; i < 99; i++ {
+		power, cumulative, found, err := m.PanelPower(i)
+		if err != nil {
</code_context>
<issue_to_address>
**suggestion:** Avoid magic number for maximum panel count

Using the hard-coded upper bound `99` makes the limit less clear and harder to update if the protocol or device constraints change. Please introduce a named constant (e.g. `const maxPanels = 99`) and use it in the loop so the limit is explicit and easier to adjust in one place.

```suggestion
const maxPanels = 99

func (m *HoymilesDTUModbusTcp) readCurrentValues() (hoymilesDTUValues, error) {
	var values hoymilesDTUValues
	for i := 0; i < maxPanels; i++ {
```
</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]

I understand the design of the HMT/HMS requires summing about a range, so this is ok. However, the code and needs be aligned to how it's done for the other devices before we can merge.

[jonilehtola]

Could you please clarify what you mean by your comment. I didn't fully understand your latter sentence.

[andig]

This PR does many things different than any other device- the implementations must be aligned