From d8a467a0c451edd11a7a61212670bbca845b465c Mon Sep 17 00:00:00 2001
From: hmelder <service@hugomelder.com>
Date: Sun, 10 Nov 2024 05:45:32 +0100
Subject: [PATCH] succd: Split scope lock into multiple blocks

We noticed huge load spikes with the latest changes.  This was caused
by the modbus goroutine blocking the entire daemon for long periods of
time while doing its data transfer.

Fix this by only holding the lock while performing data accesses.
---
 succbone/succd/modbus.go | 45 +++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/succbone/succd/modbus.go b/succbone/succd/modbus.go
index ed33f7e..e5f3b91 100644
--- a/succbone/succd/modbus.go
+++ b/succbone/succd/modbus.go
@@ -40,28 +40,27 @@ func (d *daemon) modbusConnect() error {
 // The first one (slave 1) is a temperature/humidity sensor.
 // The second one (slave 2) is a PTA8D08 transmitter
 func (d *daemon) modbusUpdate() error {
-	d.mu.Lock()
-	defer d.mu.Unlock()
-
 	var err error
-	var registers []uint16 // temperature, humidity
 
-	// Switch to slave 1
+	// Switch to slave 1 (BTA1)
 	d.modbusClient.SetUnitId(1)
 
 	// Read temperature and humidity
-	registers, err = d.modbusClient.ReadRegisters(1, 2, modbus.INPUT_REGISTER)
+	var registersBTA1 []uint16 // temperature, humidity
+	registersBTA1, err = d.modbusClient.ReadRegisters(1, 2, modbus.INPUT_REGISTER)
 	if err != nil {
 		return err
 	}
 
-	if len(registers) != 2 {
-		msg := fmt.Sprintf("Expected two registers from modbus slave 1, but got %d", len(registers))
+	if len(registersBTA1) != 2 {
+		msg := fmt.Sprintf("Expected two registers from modbus slave 1, but got %d", len(registersBTA1))
 		return errors.New(msg)
 	}
 
-	d.daemonState.tempSEM = modbusValuesToFloat(registers[0])
-	d.daemonState.humiditySEM = modbusValuesToFloat(registers[1])
+	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)
@@ -70,18 +69,21 @@ func (d *daemon) modbusUpdate() error {
 	// Channel 0: Cable -WGA6, Sensor "dp bottom"
 	// Channel 1: Cable -WGA8, Sensor "dp inlet"
 	// Channel 2: Cable WGA7, Sensor "dp top"
-	registers, err = d.modbusClient.ReadRegisters(0, 3, modbus.HOLDING_REGISTER)
+	var registersKEC2 []uint16 // temperatures dp
+	registersKEC2, err = d.modbusClient.ReadRegisters(0, 3, modbus.HOLDING_REGISTER)
 	if err != nil {
 		return err
 	}
 
-	if len(registers) != 3 {
-		return fmt.Errorf("expected three registers from modbus slave 2, but got %d", len(registers))
+	if len(registersKEC2) != 3 {
+		return fmt.Errorf("expected three registers from modbus slave 2, but got %d", len(registersKEC2))
 	}
 
-	d.daemonState.tempDPBottom = modbusValuesToFloat(registers[0])
-	d.daemonState.tempDPInlet = modbusValuesToFloat(registers[1])
-	d.daemonState.tempDPTop = modbusValuesToFloat(registers[2])
+	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)
@@ -106,7 +108,7 @@ func (d *daemon) modbusUpdate() error {
 
 	// KFA1-KFA8
 	var relayState [8]bool
-
+	d.mu.Lock()
 	// -KFA1 Roughing Pump (normally closed contact)
 	relayState[0] = !d.daemonState.rpOn
 	// -KFA2 Diffusion Pump
@@ -119,23 +121,24 @@ func (d *daemon) modbusUpdate() error {
 	relayState[5] = !d.aboveRough.output
 	// -KFA7 Fake-Pirani High (normally closed contact)
 	relayState[6] = !d.aboveHigh.output
+	d.mu.Unlock()
 
 	// The KEC1 module uses a non-standard MODBUS interface
 	// instead of coils
 	// 0x0100 is the open command
 	// 0x0200 is the close command
 	// We write 8 words (16-bit) to address 0x01 to update the relays
-	var registerValues [8]uint16
+	var registerValuesKEC1 [8]uint16
 	// Convert the boolean values to the commands
 	for idx, state := range relayState {
 		if state {
-			registerValues[idx] = 0x0100
+			registerValuesKEC1[idx] = 0x0100
 		} else {
-			registerValues[idx] = 0x0200
+			registerValuesKEC1[idx] = 0x0200
 		}
 	}
 
-	err = d.modbusClient.WriteRegisters(0x01, registerValues[:])
+	err = d.modbusClient.WriteRegisters(0x01, registerValuesKEC1[:])
 	if err != nil {
 		return err
 	}