From 6f93b96c39d3f47cc4b781653b3d4c2b2f1bd56c Mon Sep 17 00:00:00 2001
From: hmelder <service@hugomelder.com>
Date: Sun, 10 Nov 2024 06:19:01 +0100
Subject: [PATCH] succd: do not early return on error in modbusUpdate

When one device fails, this should not influence updates of the other
devices.  Thus, early return was the wrong strategy here.

Instead, when communication with a device fails, skip the process data
update and continue with the next device.
---
 succbone/succd/modbus.go | 67 ++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/succbone/succd/modbus.go b/succbone/succd/modbus.go
index e5f3b91..a4d05e0 100644
--- a/succbone/succd/modbus.go
+++ b/succbone/succd/modbus.go
@@ -2,11 +2,10 @@ package main
 
 import (
 	"context"
-	"errors"
-	"fmt"
 	"time"
 
 	"github.com/simonvetter/modbus"
+	"k8s.io/klog"
 )
 
 func modbusValuesToFloat(v uint16) float32 {
@@ -39,7 +38,7 @@ func (d *daemon) modbusConnect() error {
 // There are currently two devices connected to the modbus.
 // The first one (slave 1) is a temperature/humidity sensor.
 // The second one (slave 2) is a PTA8D08 transmitter
-func (d *daemon) modbusUpdate() error {
+func (d *daemon) modbusUpdate() {
 	var err error
 
 	// Switch to slave 1 (BTA1)
@@ -49,19 +48,16 @@ func (d *daemon) modbusUpdate() error {
 	var registersBTA1 []uint16 // temperature, humidity
 	registersBTA1, err = d.modbusClient.ReadRegisters(1, 2, modbus.INPUT_REGISTER)
 	if err != nil {
-		return err
+		klog.Warningf("error while reading registers from BTA1 %v", err)
+	} else if len(registersBTA1) != 2 {
+		klog.Warningf("expected two registers from modbus slave 1, but got %d", len(registersBTA1))
+	} else {
+		d.mu.Lock()
+		d.daemonState.tempSEM = modbusValuesToFloat(registersBTA1[0])
+		d.daemonState.humiditySEM = modbusValuesToFloat(registersBTA1[1])
+		d.mu.Unlock()
 	}
 
-	if len(registersBTA1) != 2 {
-		msg := fmt.Sprintf("Expected two registers from modbus slave 1, but got %d", len(registersBTA1))
-		return errors.New(msg)
-	}
-
-	d.mu.Lock()
-	d.daemonState.tempSEM = modbusValuesToFloat(registersBTA1[0])
-	d.daemonState.humiditySEM = modbusValuesToFloat(registersBTA1[1])
-	d.mu.Unlock()
-
 	// Switch to slave 2 (KEC2)
 	d.modbusClient.SetUnitId(2)
 
@@ -72,19 +68,17 @@ func (d *daemon) modbusUpdate() error {
 	var registersKEC2 []uint16 // temperatures dp
 	registersKEC2, err = d.modbusClient.ReadRegisters(0, 3, modbus.HOLDING_REGISTER)
 	if err != nil {
-		return err
+		klog.Warningf("error while reading registers from KEC2 %v", err)
+	} else if len(registersKEC2) != 3 {
+		klog.Warningf("expected three registers from modbus slave 2, but got %d", len(registersKEC2))
+	} else {
+		d.mu.Lock()
+		d.daemonState.tempDPBottom = modbusValuesToFloat(registersKEC2[0])
+		d.daemonState.tempDPInlet = modbusValuesToFloat(registersKEC2[1])
+		d.daemonState.tempDPTop = modbusValuesToFloat(registersKEC2[2])
+		d.mu.Unlock()
 	}
 
-	if len(registersKEC2) != 3 {
-		return fmt.Errorf("expected three registers from modbus slave 2, but got %d", len(registersKEC2))
-	}
-
-	d.mu.Lock()
-	d.daemonState.tempDPBottom = modbusValuesToFloat(registersKEC2[0])
-	d.daemonState.tempDPInlet = modbusValuesToFloat(registersKEC2[1])
-	d.daemonState.tempDPTop = modbusValuesToFloat(registersKEC2[2])
-	d.mu.Unlock()
-
 	// Switch to slave 3 (KEC1)
 	d.modbusClient.SetUnitId(3)
 
@@ -94,17 +88,18 @@ func (d *daemon) modbusUpdate() error {
 
 	digitalInputRegisters, err = d.modbusClient.ReadRegisters(0x81, 8, modbus.HOLDING_REGISTER)
 	if err != nil {
-		return err
-	}
-	// Convert MODBUS words into bools
-	for idx, value := range digitalInputRegisters {
-		if value != 0 {
-			digitalInputs[idx] = true
-		} else {
-			digitalInputs[idx] = false
+		klog.Warningf("error while reading digital inputs from KEC1 %v", err)
+	} else {
+		// Convert MODBUS words into bools
+		for idx, value := range digitalInputRegisters {
+			if value != 0 {
+				digitalInputs[idx] = true
+			} else {
+				digitalInputs[idx] = false
+			}
 		}
+		// TODO: Input mapping goes here
 	}
-	// TODO: Input mapping goes here
 
 	// KFA1-KFA8
 	var relayState [8]bool
@@ -140,10 +135,8 @@ func (d *daemon) modbusUpdate() error {
 
 	err = d.modbusClient.WriteRegisters(0x01, registerValuesKEC1[:])
 	if err != nil {
-		return err
+		klog.Warningf("error while updating registers %v", err)
 	}
-
-	return nil
 }
 
 // Call modbusUpdate every 100 milliseconds