From 6aa6e27cb403e76bc1c57fe56e1aa9e5473b9a67 Mon Sep 17 00:00:00 2001
From: Philip Porto Schiffer <admin@psdev.de>
Date: Tue, 29 Apr 2025 15:33:07 +0200
Subject: [PATCH] Sungrow: fix charger Enabled and phase-switch logic (#20857)

---
 charger/sungrow.go | 82 ++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/charger/sungrow.go b/charger/sungrow.go
index be824f5a1..b3e757867 100644
--- a/charger/sungrow.go
+++ b/charger/sungrow.go
@@ -31,9 +31,10 @@ import (
 
 // Sungrow charger implementation
 type Sungrow struct {
-	log  *util.Logger
-	conn *modbus.Connection
-	curr uint16
+	log     *util.Logger
+	conn    *modbus.Connection
+	curr    uint16
+	enabled bool
 }
 
 const (
@@ -41,19 +42,19 @@ const (
 	sgRegPhase             = 21224 // uint16 [1: Single-phase,	3: Three-phase]
 	sgRegWorkMode          = 21262 // uint16 [0: Network, 2: Plug&Play, 6: EMS]
 	sgRegRemCtrlStatus     = 21267 // uint16 [0: Disable, 1: Enable]
-	sgRegPhaseSwitchStatus = 21269 // uint16
+	sgRegPhaseSwitchStatus = 21269 // uint16 [0: Three-phase, 1: Single-phase]
 	sgRegTotalEnergy       = 21299 // uint32s 1Wh
 	sgRegActivePower       = 21307 // uint32s 1W
 	sgRegChargedEnergy     = 21309 // uint32s 1Wh
 	sgRegStartMode         = 21313 // uint16 [1: Started by EMS, 2: Started by swiping card]
 	sgRegPowerRequest      = 21314 // uint16 [0: Enable, 1: Close]
 	sgRegPowerFlag         = 21315 // uint16 [0: Charging or power regulation is not allowed; 1: Charging or power regulation is allowed]
-	sgRegState             = 21316 // uint16
+	sgRegState             = 21316 // uint16 [1: Idle, 2: Standby, 3: Charging, 4: Charging suspended (pile side), 5: Charging suspended (vehicle side), 6: Charging completed, 7: Reserved, 8: Unavailable, 9: Faulted]
 
 	// holding
 	sgRegSetOutI       = 21202 // uint16 0.01A
 	sgRegPhaseSwitch   = 21203 // uint16 [0: Three-phase, 1: Single-phase]
-	sgRegUnavailable   = 21210 // uint16
+	sgRegAvailability  = 21210 // uint16 [0: Unavailable, 1: Available]
 	sgRegRemoteControl = 21211 // uint16 [0: Start, 1: Stop]
 )
 
@@ -127,14 +128,20 @@ func (wb *Sungrow) Status() (api.ChargeStatus, error) {
 	switch s := binary.BigEndian.Uint16(b); s {
 	case 1: // Idle
 		return api.StatusA, nil
-	case
-		2, // Standby
-		4, // SuspendedEVSE
-		5, // SuspendedEV
-		6: // Completed
+	case 2: // Standby
 		return api.StatusB, nil
 	case 3: // Charging
+		wb.enabled = true
 		return api.StatusC, nil
+	case 4: // SuspendedEVSE
+		wb.enabled = false
+		return api.StatusB, nil
+	case 5: // SuspendedEV
+		wb.enabled = true
+		return api.StatusB, nil
+	case 6: // Completed
+		wb.enabled = false
+		return api.StatusB, nil
 	default:
 		return api.StatusNone, fmt.Errorf("invalid status: %d", s)
 	}
@@ -142,28 +149,35 @@ func (wb *Sungrow) Status() (api.ChargeStatus, error) {
 
 // Enabled implements the api.Charger interface
 func (wb *Sungrow) Enabled() (bool, error) {
-	b, err := wb.conn.ReadInputRegisters(sgRegStartMode, 1)
-	if err != nil {
-		return false, err
-	}
-
-	return binary.BigEndian.Uint16(b) != 0, nil
+	return wb.enabled, nil
 }
 
 // Enable implements the api.Charger interface
 func (wb *Sungrow) Enable(enable bool) error {
-	var u uint16
-	if !enable {
-		u = 1
+	var u uint16 = 1 // Stop
+
+	if enable {
+		u = 0 // Start
+
+		// Make sure the charger is available, otherwise sgRegRemoteControl is not usable
+		if _, err := wb.conn.WriteSingleRegister(sgRegAvailability, 1); err != nil {
+			return err
+		}
 	}
 
-	_, err := wb.conn.WriteSingleRegister(sgRegRemoteControl, u)
-
-	if err == nil && enable {
-		_, err = wb.conn.WriteSingleRegister(sgRegSetOutI, wb.curr)
+	if _, err := wb.conn.WriteSingleRegister(sgRegRemoteControl, u); err != nil {
+		return err
 	}
 
-	return err
+	if enable {
+		if _, err := wb.conn.WriteSingleRegister(sgRegSetOutI, wb.curr); err != nil {
+			return err
+		}
+	}
+
+	wb.enabled = enable
+
+	return nil
 }
 
 // MaxCurrent implements the api.Charger interface
@@ -249,20 +263,8 @@ func (wb *Sungrow) Phases1p3p(phases int) error {
 		u = 1
 	}
 
-	enabled, err := wb.Enabled()
-	if err == nil && enabled {
-		if err = wb.Enable(false); err != nil {
-			return err
-		}
-	}
-
 	// Switch phases
-	_, err = wb.conn.WriteSingleRegister(sgRegPhaseSwitch, u)
-
-	// Re-enable charging if it was previously enabled
-	if err == nil && enabled {
-		err = wb.Enable(true)
-	}
+	_, err := wb.conn.WriteSingleRegister(sgRegPhaseSwitch, u)
 
 	return err
 }
@@ -293,8 +295,8 @@ func (wb *Sungrow) Diagnose() {
 	if b, err := wb.conn.ReadHoldingRegisters(sgRegPhaseSwitch, 1); err == nil {
 		fmt.Printf("\tPhaseSwitch:\t%d\n", binary.BigEndian.Uint16(b))
 	}
-	if b, err := wb.conn.ReadHoldingRegisters(sgRegUnavailable, 1); err == nil {
-		fmt.Printf("\tUnavailable:\t%d\n", binary.BigEndian.Uint16(b))
+	if b, err := wb.conn.ReadHoldingRegisters(sgRegAvailability, 1); err == nil {
+		fmt.Printf("\tAvailability:\t%d\n", binary.BigEndian.Uint16(b))
 	}
 	if b, err := wb.conn.ReadHoldingRegisters(sgRegRemoteControl, 1); err == nil {
 		fmt.Printf("\tRemoteControl:\t%d\n", binary.BigEndian.Uint16(b))