Merge pull request #20 from diebietse/scalefixes

Fix scaling decoding and munin race condition
Fix race condition in munin output.
2020-10-08 12:29:58 +02:00 · 2020-10-08 12:25:39 +02:00 · 2020-10-08 12:25:33 +02:00 · 2020-09-25 15:03:26 +02:00 · 2020-09-19 18:38:00 +02:00 · 2020-09-13 21:58:57 +02:00
8 changed files with 308 additions and 165 deletions
--- a/3
+++ b/3
@@ -39,9 +39,6 @@ gofmt:
 	gofmt -l -s -w .

 test:
-	go test -v ./...
-
-test-race:
 	go test -v -race ./...

 docker:
--- a/README.md
+++ b/README.md
@@ -27,10 +27,8 @@ Usage:
  invertergui [OPTIONS]

 Application Options:
-      --address=        The IP/DNS and port of the machine that the application is running on. (default: :8080)
-                        [$ADDRESS]
-      --data.source=    Set the source of data for the inverter gui. "serial", "tcp" or "mock" (default: serial)
-                        [$DATA_SOURCE]
+      --address=        The IP/DNS and port of the machine that the application is running on. (default: :8080) [$ADDRESS]
+      --data.source=    Set the source of data for the inverter gui. "serial", "tcp" or "mock" (default: serial) [$DATA_SOURCE]
      --data.host=      Host to connect when source is set to tcp. (default: localhost:8139) [$DATA_HOST]
      --data.device=    TTY device to use when source is set to serial. (default: /dev/ttyUSB0) [$DATA_DEVICE]
      --cli.enabled     Enable CLI output. [$CLI_ENABLED]
@@ -40,6 +38,7 @@ Application Options:
      --mqtt.topic=     Set the MQTT topic updates published to. (default: invertergui/updates) [$MQTT_TOPIC]
      --mqtt.username=  Set the MQTT username [$MQTT_USERNAME]
      --mqtt.password=  Set the MQTT password [$MQTT_PASSWORD]
+      --loglevel=       The log level to generate logs at. ("panic", "fatal", "error", "warn", "info", "debug", "trace") (default: info) [$LOGLEVEL]

 Help Options:
  -h, --help            Show this help message
--- a/cmd/invertergui/config.go
+++ b/cmd/invertergui/config.go
@@ -22,6 +22,7 @@ type config struct {
 		Username string `long:"mqtt.username" env:"MQTT_USERNAME" default:"" description:"Set the MQTT username"`
 		Password string `long:"mqtt.password" env:"MQTT_PASSWORD" default:"" description:"Set the MQTT password"`
 	}
+	Loglevel string `long:"loglevel" env:"LOGLEVEL" default:"info" description:"The log level to generate logs at. (\"panic\", \"fatal\", \"error\", \"warn\", \"info\", \"debug\", \"trace\")"`
 }

 func parseConfig() (*config, error) {
--- a/cmd/invertergui/main.go
+++ b/cmd/invertergui/main.go
@@ -58,6 +58,11 @@ func main() {
 		os.Exit(1)
 	}
 	log.Info("Starting invertergui")
+	logLevel, err := logrus.ParseLevel(conf.Loglevel)
+	if err != nil {
+		log.Fatalf("Could not parse log level: %v", err)
+	}
+	logrus.SetLevel(logLevel)

 	mk2, err := getMk2Device(conf.Data.Source, conf.Data.Host, conf.Data.Device)
 	if err != nil {
--- a/mk2driver/mk2.go
+++ b/mk2driver/mk2.go
@@ -4,17 +4,72 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
-	"math"
 	"sync"
 	"time"
+
+	"github.com/sirupsen/logrus"
 )

 type scaling struct {
 	scale     float64
 	offset    float64
+	signed    bool
+	supported bool
 }

+//nolint:deadcode,varcheck
+const (
+	ramVarVMains = iota
+	ramVarIMains
+	ramVarVInverter
+	ramVarIInverter
+	ramVarVBat
+	ramVarIBat
+	ramVarVBatRipple
+	ramVarInverterPeriod
+	ramVarMainPeriod
+	ramVarIACLoad
+	ramVarVirSwitchPos
+	ramVarIgnACInState
+	ramVarMultiFuncRelay
+	ramVarChargeState
+	ramVarInverterPower1
+	ramVarInverterPower2
+	ramVarOutPower
+
+	ramVarMaxOffset = 14
+)
+
+const (
+	infoFrameHeader = 0x20
+	frameHeader     = 0xff
+)
+
+const (
+	acL1InfoFrame  = 0x08
+	dcInfoFrame    = 0x0C
+	setTargetFrame = 0x41
+	infoReqFrame   = 0x46
+	ledFrame       = 0x4C
+	vFrame         = 0x56
+	winmonFrame    = 0x57
+)
+
+// info frame types
+const (
+	infoReqAddrDC   = 0x00
+	infoReqAddrACL1 = 0x01
+)
+
+// winmon frame commands
+const (
+	commandReadRAMVar    = 0x30
+	commandGetRAMVarInfo = 0x36
+
+	commandReadRAMResponse       = 0x85
+	commandGetRAMVarInfoResponse = 0x8E
+)
+
 type mk2Ser struct {
 	info       *Mk2Info
 	p          io.ReadWriter
@@ -32,7 +87,7 @@ func NewMk2Connection(dev io.ReadWriter) (Mk2, error) {
 	mk2.info = &Mk2Info{}
 	mk2.scaleCount = 0
 	mk2.frameLock = false
-	mk2.scales = make([]scaling, 0, 14)
+	mk2.scales = make([]scaling, 0, ramVarMaxOffset)
 	mk2.setTarget()
 	mk2.run = make(chan struct{})
 	mk2.infochan = make(chan *Mk2Info)
@@ -43,9 +98,8 @@ func NewMk2Connection(dev io.ReadWriter) (Mk2, error) {

 // Locks to incoming frame.
 func (m *mk2Ser) frameLocker() {
-
 	frame := make([]byte, 256)
-	var size byte
+	var frameLength byte
 	for {
 		select {
 		case <-m.run:
@@ -54,34 +108,36 @@ func (m *mk2Ser) frameLocker() {
 		default:
 		}
 		if m.frameLock {
-			size = m.readByte()
-			l, err := io.ReadFull(m.p, frame[0:int(size)+1])
+			frameLength = m.readByte()
+			frameLengthOffset := int(frameLength) + 1
+			l, err := io.ReadFull(m.p, frame[:frameLengthOffset])
 			if err != nil {
 				m.addError(fmt.Errorf("Read Error: %v", err))
 				m.frameLock = false
-			} else if l != int(size)+1 {
+			} else if l != frameLengthOffset {
 				m.addError(errors.New("Read Length Error"))
 				m.frameLock = false
 			} else {
-				m.handleFrame(size, frame[0:int(size+1)])
+				m.handleFrame(frameLength, frame[:frameLengthOffset])
 			}
 		} else {
 			tmp := m.readByte()
-			if tmp == 0xff || tmp == 0x20 {
-				l, err := io.ReadFull(m.p, frame[0:int(size)])
+			frameLengthOffset := int(frameLength)
+			if tmp == frameHeader || tmp == infoFrameHeader {
+				l, err := io.ReadFull(m.p, frame[:frameLengthOffset])
 				if err != nil {
 					m.addError(fmt.Errorf("Read Error: %v", err))
 					time.Sleep(1 * time.Second)
-				} else if l != int(size) {
+				} else if l != frameLengthOffset {
 					m.addError(errors.New("Read Length Error"))
 				} else {
-					if checkChecksum(size, tmp, frame[0:int(size)]) {
+					if checkChecksum(frameLength, tmp, frame[:frameLengthOffset]) {
 						m.frameLock = true
-						log.Printf("Locked")
+						logrus.Info("Locked")
 					}
 				}
 			}
-			size = tmp
+			frameLength = tmp
 		}
 	}
 }
@@ -127,34 +183,35 @@ func (m *mk2Ser) updateReport() {

 // Checks for valid frame and chooses decoding.
 func (m *mk2Ser) handleFrame(l byte, frame []byte) {
+	logrus.Debugf("frame %#v", frame)
 	if checkChecksum(l, frame[0], frame[1:]) {
 		switch frame[0] {
-		case 0xff:
+		case frameHeader:
 			switch frame[1] {
-			case 0x56: // V
+			case vFrame:
 				m.versionDecode(frame[2:])
-			case 0x57:
+			case winmonFrame:
 				switch frame[2] {
-				case 0x8e:
+				case commandGetRAMVarInfoResponse:
 					m.scaleDecode(frame[2:])
-				case 0x85:
+				case commandReadRAMResponse:
 					m.stateDecode(frame[2:])
 				}

-			case 0x4C: // L
+			case ledFrame:
 				m.ledDecode(frame[2:])
 			}

-		case 0x20:
+		case infoFrameHeader:
 			switch frame[5] {
-			case 0x0C:
+			case dcInfoFrame:
 				m.dcDecode(frame[1:])
-			case 0x08:
+			case acL1InfoFrame:
 				m.acDecode(frame[1:])
 			}
 		}
 	} else {
-		log.Printf("Invalid incoming frame checksum: %x", frame)
+		logrus.Errorf("Invalid incoming frame checksum: %x", frame)
 		m.frameLock = false
 	}
 }
@@ -162,7 +219,7 @@ func (m *mk2Ser) handleFrame(l byte, frame []byte) {
 // Set the target VBus device.
 func (m *mk2Ser) setTarget() {
 	cmd := make([]byte, 3)
-	cmd[0] = 0x41 // A
+	cmd[0] = setTargetFrame
 	cmd[1] = 0x01
 	cmd[2] = 0x00
 	m.sendCommand(cmd)
@@ -171,57 +228,98 @@ func (m *mk2Ser) setTarget() {
 // Request the scaling factor for entry 'in'.
 func (m *mk2Ser) reqScaleFactor(in byte) {
 	cmd := make([]byte, 4)
-	cmd[0] = 0x57 // W
-	cmd[1] = 0x36
+	cmd[0] = winmonFrame
+	cmd[1] = commandGetRAMVarInfo
 	cmd[2] = in
 	m.sendCommand(cmd)
 }

+func int16Abs(in int16) uint16 {
+	if in < 0 {
+		return uint16(-in)
+	}
+	return uint16(in)
+}
+
 // Decode the scale factor frame.
 func (m *mk2Ser) scaleDecode(frame []byte) {
-	scl := uint16(frame[2])<<8 + uint16(frame[1])
-	ofs := int16(uint16(frame[5])<<8 + uint16(frame[4]))
-
 	tmp := scaling{}
-	tmp.offset = float64(ofs)
-	if scl >= 0x4000 {
-		tmp.scale = math.Abs(1 / (0x8000 - float64(scl)))
+	logrus.Debugf("Scale frame(%d): 0x%x", len(frame), frame)
+	if len(frame) < 6 {
+		tmp.supported = false
+		logrus.Warnf("Skiping scaling factors for: %d", m.scaleCount)
 	} else {
-		tmp.scale = math.Abs(float64(scl))
+		tmp.supported = true
+		var scl int16
+		var ofs int16
+		if len(frame) == 6 {
+			scl = int16(frame[2])<<8 + int16(frame[1])
+			ofs = int16(uint16(frame[4])<<8 + uint16(frame[3]))
+		} else {
+			scl = int16(frame[2])<<8 + int16(frame[1])
+			ofs = int16(uint16(frame[5])<<8 + uint16(frame[4]))
 		}
+		if scl < 0 {
+			tmp.signed = true
+		}
+		tmp.offset = float64(ofs)
+		scale := int16Abs(scl)
+		if scale >= 0x4000 {
+			tmp.scale = 1 / (0x8000 - float64(scale))
+		} else {
+			tmp.scale = float64(scale)
+		}
+	}
+	logrus.Debugf("scalecount %v: %#v \n", m.scaleCount, tmp)
 	m.scales = append(m.scales, tmp)
-
 	m.scaleCount++
-	if m.scaleCount < 14 {
+	if m.scaleCount < ramVarMaxOffset {
 		m.reqScaleFactor(byte(m.scaleCount))
 	} else {
-		log.Print("Monitoring starting.")
+		logrus.Info("Monitoring starting.")
 	}
-
 }

 // Decode the version number
 func (m *mk2Ser) versionDecode(frame []byte) {
+	logrus.Debugf("versiondecode %v", frame)
 	m.info.Version = 0
 	m.info.Valid = true
 	for i := 0; i < 4; i++ {
 		m.info.Version += uint32(frame[i]) << uint(i) * 8
 	}

-	if m.scaleCount < 14 {
-		log.Print("Get scaling factors.")
+	if m.scaleCount < ramVarMaxOffset {
+		logrus.Info("Get scaling factors.")
 		m.reqScaleFactor(byte(m.scaleCount))
 	} else {
 		// Send DC status request
 		cmd := make([]byte, 2)
-		cmd[0] = 0x46 //F
-		cmd[1] = 0
+		cmd[0] = infoReqFrame
+		cmd[1] = infoReqAddrDC
 		m.sendCommand(cmd)
 	}
 }

+// Decode with correct signedness and apply scale
+func (m *mk2Ser) applyScaleAndSign(data []byte, scale int) float64 {
+	var value float64
+	if !m.scales[scale].supported {
+		return 0
+	}
+	if m.scales[scale].signed {
+		value = getSigned(data)
+	} else {
+		value = getUnsigned16(data)
+	}
+	return m.applyScale(value, scale)
+}
+
 // Apply scaling to float
 func (m *mk2Ser) applyScale(value float64, scale int) float64 {
+	if !m.scales[scale].supported {
+		return value
+	}
 	return m.scales[scale].scale * (value + m.scales[scale].offset)
 }

@@ -230,6 +328,11 @@ func getSigned(data []byte) float64 {
 	return float64(int16(data[0]) + int16(data[1])<<8)
 }

+// Convert bytes->int16->float
+func getUnsigned16(data []byte) float64 {
+	return float64(uint16(data[0]) + uint16(data[1])<<8)
+}
+
 // Convert bytes->uint32->float
 func getUnsigned(data []byte) float64 {
 	return float64(uint32(data[0]) + uint32(data[1])<<8 + uint32(data[2])<<16)
@@ -237,43 +340,46 @@ func getUnsigned(data []byte) float64 {

 // Decodes DC frame.
 func (m *mk2Ser) dcDecode(frame []byte) {
-	m.info.BatVoltage = m.applyScale(getSigned(frame[5:7]), 4)
+	m.info.BatVoltage = m.applyScaleAndSign(frame[5:7], ramVarVBat)

-	usedC := m.applyScale(getUnsigned(frame[7:10]), 5)
-	chargeC := m.applyScale(getUnsigned(frame[10:13]), 5)
+	usedC := m.applyScale(getUnsigned(frame[7:10]), ramVarIBat)
+	chargeC := m.applyScale(getUnsigned(frame[10:13]), ramVarIBat)
 	m.info.BatCurrent = usedC - chargeC

-	m.info.OutFrequency = 10 / (m.applyScale(float64(frame[13]), 7))
+	m.info.OutFrequency = 10 / (m.applyScale(float64(frame[13]), ramVarInverterPeriod))
+	logrus.Debugf("dcDecode %#v", m.info)

 	// Send L1 status request
 	cmd := make([]byte, 2)
-	cmd[0] = 0x46 //F
-	cmd[1] = 1
+	cmd[0] = infoReqFrame
+	cmd[1] = infoReqAddrACL1
 	m.sendCommand(cmd)
 }

 // Decodes AC frame.
 func (m *mk2Ser) acDecode(frame []byte) {
-	m.info.InVoltage = m.applyScale(getSigned(frame[5:7]), 0)
-	m.info.InCurrent = m.applyScale(getSigned(frame[7:9]), 1)
-	m.info.OutVoltage = m.applyScale(getSigned(frame[9:11]), 2)
-	m.info.OutCurrent = m.applyScale(getSigned(frame[11:13]), 3)
+	m.info.InVoltage = m.applyScaleAndSign(frame[5:7], ramVarVMains)
+	m.info.InCurrent = m.applyScaleAndSign(frame[7:9], ramVarIMains)
+	m.info.OutVoltage = m.applyScaleAndSign(frame[9:11], ramVarVInverter)
+	m.info.OutCurrent = m.applyScaleAndSign(frame[11:13], ramVarIInverter)

 	if frame[13] == 0xff {
 		m.info.InFrequency = 0
 	} else {
-		m.info.InFrequency = 10 / (m.applyScale(float64(frame[13]), 8))
+		m.info.InFrequency = 10 / (m.applyScale(float64(frame[13]), ramVarMainPeriod))
 	}
+	logrus.Debugf("acDecode %#v", m.info)

 	// Send status request
 	cmd := make([]byte, 1)
-	cmd[0] = 0x4C //F
+	cmd[0] = ledFrame
 	m.sendCommand(cmd)
 }

 // Decode charge state of battery.
 func (m *mk2Ser) stateDecode(frame []byte) {
-	m.info.ChargeState = m.applyScale(getSigned(frame[1:3]), 13)
+	m.info.ChargeState = m.applyScaleAndSign(frame[1:3], ramVarChargeState)
+	logrus.Debugf("battery state decode %#v", m.info)
 	m.updateReport()
 }

@@ -283,9 +389,9 @@ func (m *mk2Ser) ledDecode(frame []byte) {
 	m.info.LEDs = getLEDs(frame[0], frame[1])
 	// Send charge state request
 	cmd := make([]byte, 4)
-	cmd[0] = 0x57 //W
-	cmd[1] = 0x30
-	cmd[2] = 13
+	cmd[0] = winmonFrame
+	cmd[1] = commandReadRAMVar
+	cmd[2] = ramVarChargeState
 	m.sendCommand(cmd)
 }

@@ -312,7 +418,7 @@ func (m *mk2Ser) sendCommand(data []byte) {
 	l := len(data)
 	dataOut := make([]byte, l+3)
 	dataOut[0] = byte(l + 1)
-	dataOut[1] = 0xff
+	dataOut[1] = frameHeader
 	cr := -dataOut[0] - dataOut[1]
 	for i := 0; i < len(data); i++ {
 		cr = cr - data[i]
@@ -320,6 +426,7 @@ func (m *mk2Ser) sendCommand(data []byte) {
 	}
 	dataOut[l+2] = cr

+	logrus.Debugf("sendCommand %#v", dataOut)
 	_, err := m.p.Write(dataOut)
 	if err != nil {
 		m.addError(fmt.Errorf("Write error: %v", err))
--- a/mk2driver/mk2_test.go
+++ b/mk2driver/mk2_test.go
@@ -1,45 +1,10 @@
-/**
-write out: []byte{0x04, 0xff, 0x41, 0x01, 0x00, 0xbb, }
-read byte: []byte{0x04, }
-read byte: []byte{0xff, }
-read unlocked: []byte{0x41, 0x01, 0x00, 0xbb, }
-2019/03/17 16:24:17 Locked
-
-
-
-write out: []byte{0x04, 0xff, 0x41, 0x01, 0x00, 0xbb, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x00, 0x00, 0x6f, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x01, 0x00, 0x6e, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x02, 0x00, 0x6d, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x03, 0x00, 0x6c, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x04, 0x00, 0x6b, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x05, 0x00, 0x6a, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x06, 0x00, 0x69, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x07, 0x00, 0x68, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x08, 0x00, 0x67, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x09, 0x00, 0x66, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x0a, 0x00, 0x65, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x0b, 0x00, 0x64, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x0c, 0x00, 0x63, }
-write out: []byte{0x05, 0xff, 0x57, 0x36, 0x0d, 0x00, 0x62, }
-write out: []byte{0x03, 0xff, 0x46, 0x00, 0xb8, }
-write out: []byte{0x03, 0xff, 0x46, 0x01, 0xb7, }
-write out: []byte{0x02, 0xff, 0x4c, 0xb3, }
-write out: []byte{0x05, 0xff, 0x57, 0x30, 0x0d, 0x00, 0x68, }
-write out: []byte{0x03, 0xff, 0x46, 0x00, 0xb8, }
-write out: []byte{0x03, 0xff, 0x46, 0x01, 0xb7, }
-write out: []byte{0x02, 0xff, 0x4c, 0xb3, }
-write out: []byte{0x05, 0xff, 0x57, 0x30, 0x0d, 0x00, 0x68, }
-*/
-
-package mk2driver_test
+package mk2driver

 import (
 	"bytes"
 	"io"
 	"testing"

-	"github.com/diebietse/invertergui/mk2driver"
 	"github.com/stretchr/testify/assert"
 )

@@ -67,6 +32,10 @@ var knownWrites = []byte{

 var writeBuffer = bytes.NewBuffer(nil)

+const (
+	testEpsilon = 0.00000001
+)
+
 type testIo struct {
 	io.Reader
 	io.Writer
@@ -105,18 +74,18 @@ func TestSync(t *testing.T) {
 		0x05, 0xff, 0x57, 0x85, 0xc8, 0x00, 0x58,
 	}

-	expectedLEDs := map[mk2driver.Led]mk2driver.LEDstate{
-		mk2driver.LedMain:        mk2driver.LedOn,
-		mk2driver.LedAbsorption:  mk2driver.LedOn,
-		mk2driver.LedBulk:        mk2driver.LedOff,
-		mk2driver.LedFloat:       mk2driver.LedOff,
-		mk2driver.LedInverter:    mk2driver.LedOff,
-		mk2driver.LedOverload:    mk2driver.LedOff,
-		mk2driver.LedLowBattery:  mk2driver.LedOff,
-		mk2driver.LedTemperature: mk2driver.LedOff,
+	expectedLEDs := map[Led]LEDstate{
+		LedMain:        LedOn,
+		LedAbsorption:  LedOn,
+		LedBulk:        LedOff,
+		LedFloat:       LedOff,
+		LedInverter:    LedOff,
+		LedOverload:    LedOff,
+		LedLowBattery:  LedOff,
+		LedTemperature: LedOff,
 	}
 	testIO := NewIOStub(knownReadBuffer)
-	mk2, err := mk2driver.NewMk2Connection(testIO)
+	mk2, err := NewMk2Connection(testIO)
 	assert.NoError(t, err, "Could not open MK2")

 	event := <-mk2.C()
@@ -128,14 +97,54 @@ func TestSync(t *testing.T) {
 	assert.Equal(t, 0, len(event.Errors), "Reported errors not empty")
 	assert.Equal(t, expectedLEDs, event.LEDs, "Reported LEDs incorrect")

-	epsilon := 0.00000001
-	assert.InEpsilon(t, 14.41, event.BatVoltage, epsilon, "BatVoltage conversion failed")
-	assert.InEpsilon(t, -0.4, event.BatCurrent, epsilon, "BatCurrent conversion failed")
-	assert.InEpsilon(t, 226.98, event.InVoltage, epsilon, "InVoltage conversion failed")
-	assert.InEpsilon(t, 1.71, event.InCurrent, epsilon, "InCurrent conversion failed")
-	assert.InEpsilon(t, 50.10256410256411, event.InFrequency, epsilon, "InFrequency conversion failed")
-	assert.InEpsilon(t, 226.980, event.OutVoltage, epsilon, "OutVoltage conversion failed")
-	assert.InEpsilon(t, 1.54, event.OutCurrent, epsilon, "OutCurrent conversion failed")
-	assert.InEpsilon(t, 50.025510204081634, event.OutFrequency, epsilon, "OutFrequency conversion failed")
-	assert.InEpsilon(t, 1, event.ChargeState, epsilon, "ChargeState conversion failed")
+	assert.InEpsilon(t, 14.41, event.BatVoltage, testEpsilon, "BatVoltage conversion failed")
+	assert.InEpsilon(t, -0.4, event.BatCurrent, testEpsilon, "BatCurrent conversion failed")
+	assert.InEpsilon(t, 226.98, event.InVoltage, testEpsilon, "InVoltage conversion failed")
+	assert.InEpsilon(t, 1.71, event.InCurrent, testEpsilon, "InCurrent conversion failed")
+	assert.InEpsilon(t, 50.10256410256411, event.InFrequency, testEpsilon, "InFrequency conversion failed")
+	assert.InEpsilon(t, 226.980, event.OutVoltage, testEpsilon, "OutVoltage conversion failed")
+	assert.InEpsilon(t, 1.54, event.OutCurrent, testEpsilon, "OutCurrent conversion failed")
+	assert.InEpsilon(t, 50.025510204081634, event.OutFrequency, testEpsilon, "OutFrequency conversion failed")
+	assert.InEpsilon(t, 1, event.ChargeState, testEpsilon, "ChargeState conversion failed")
+}
+
+func Test_mk2Ser_scaleDecode(t *testing.T) {
+	tests := []struct {
+		name            string
+		frame           []byte
+		expectedScaling scaling
+	}{
+		{
+			name:  "Valid scale",
+			frame: []byte{0x57, 0x8e, 0x9c, 0x7f, 0x8f, 0x00, 0x00, 0x6a},
+			expectedScaling: scaling{
+				scale:     0.00013679890560875513,
+				offset:    143,
+				supported: true,
+			},
+		},
+		{
+			name:  "Unsupported frame",
+			frame: []byte{0x57, 0x00},
+			expectedScaling: scaling{
+				supported: false,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := &mk2Ser{
+				scales: make([]scaling, 0, ramVarMaxOffset),
+				p:      NewIOStub([]byte{}),
+			}
+			m.scaleDecode(tt.frame)
+			assert.Equal(t, 1, len(m.scales))
+			assert.Equal(t, 1, m.scaleCount)
+			assert.Equal(t, tt.expectedScaling.supported, m.scales[0].supported)
+			if tt.expectedScaling.supported {
+				assert.InEpsilon(t, tt.expectedScaling.offset, m.scales[0].offset, testEpsilon)
+				assert.InEpsilon(t, tt.expectedScaling.scale, m.scales[0].scale, testEpsilon)
+			}
+		})
+	}
 }
--- a/plugins/munin/munin.go
+++ b/plugins/munin/munin.go
@@ -44,18 +44,18 @@ var log = logrus.WithField("ctx", "inverter-gui-munin")

 type Munin struct {
 	mk2driver.Mk2
-	muninResponse chan *muninData
+	muninResponse chan muninData
 }

 type muninData struct {
-	status       *mk2driver.Mk2Info
+	status       mk2driver.Mk2Info
 	timesUpdated int
 }

 func NewMunin(mk2 mk2driver.Mk2) *Munin {
 	m := &Munin{
 		Mk2:           mk2,
-		muninResponse: make(chan *muninData),
+		muninResponse: make(chan muninData),
 	}

 	go m.run()
@@ -71,10 +71,10 @@ func (m *Munin) ServeMuninHTTP(rw http.ResponseWriter, r *http.Request) {
 		_, _ = rw.Write([]byte("No data to return.\n"))
 		return
 	}
-	calcMuninAverages(muninDat)
+	calcMuninAverages(&muninDat)

 	status := muninDat.status
-	tmpInput := buildTemplateInput(status)
+	tmpInput := buildTemplateInput(&status)
 	outputBuf := &bytes.Buffer{}
 	fmt.Fprintf(outputBuf, "multigraph in_batvolt\n")
 	fmt.Fprintf(outputBuf, "volt.value %s\n", tmpInput.BatVoltage)
@@ -113,65 +113,61 @@ func (m *Munin) ServeMuninConfigHTTP(rw http.ResponseWriter, r *http.Request) {

 func (m *Munin) run() {
 	muninValues := &muninData{
-		status: &mk2driver.Mk2Info{},
+		status: mk2driver.Mk2Info{},
 	}
 	for {
 		select {
 		case e := <-m.C():
 			if e.Valid {
 				calcMuninValues(muninValues, e)
-
 			}
-		case m.muninResponse <- muninValues:
+		case m.muninResponse <- *muninValues:
 			zeroMuninValues(muninValues)
 		}
 	}
 }

 //Munin only samples once every 5 minutes so averages have to be calculated for some values.
-func calcMuninValues(muninDat *muninData, newStatus *mk2driver.Mk2Info) {
-	muninDat.timesUpdated++
-	muninVal := muninDat.status
-	muninVal.OutCurrent += newStatus.OutCurrent
-	muninVal.InCurrent += newStatus.InCurrent
-	muninVal.BatCurrent += newStatus.BatCurrent
+func calcMuninValues(m *muninData, newStatus *mk2driver.Mk2Info) {
+	m.timesUpdated++
+	m.status.OutCurrent += newStatus.OutCurrent
+	m.status.InCurrent += newStatus.InCurrent
+	m.status.BatCurrent += newStatus.BatCurrent

-	muninVal.OutVoltage += newStatus.OutVoltage
-	muninVal.InVoltage += newStatus.InVoltage
-	muninVal.BatVoltage += newStatus.BatVoltage
+	m.status.OutVoltage += newStatus.OutVoltage
+	m.status.InVoltage += newStatus.InVoltage
+	m.status.BatVoltage += newStatus.BatVoltage

-	muninVal.InFrequency = newStatus.InFrequency
-	muninVal.OutFrequency = newStatus.OutFrequency
+	m.status.InFrequency = newStatus.InFrequency
+	m.status.OutFrequency = newStatus.OutFrequency

-	muninVal.ChargeState = newStatus.ChargeState
+	m.status.ChargeState = newStatus.ChargeState
 }

-func calcMuninAverages(muninDat *muninData) {
-	muninVal := muninDat.status
-	muninVal.OutCurrent /= float64(muninDat.timesUpdated)
-	muninVal.InCurrent /= float64(muninDat.timesUpdated)
-	muninVal.BatCurrent /= float64(muninDat.timesUpdated)
+func calcMuninAverages(m *muninData) {
+	m.status.OutCurrent /= float64(m.timesUpdated)
+	m.status.InCurrent /= float64(m.timesUpdated)
+	m.status.BatCurrent /= float64(m.timesUpdated)

-	muninVal.OutVoltage /= float64(muninDat.timesUpdated)
-	muninVal.InVoltage /= float64(muninDat.timesUpdated)
-	muninVal.BatVoltage /= float64(muninDat.timesUpdated)
+	m.status.OutVoltage /= float64(m.timesUpdated)
+	m.status.InVoltage /= float64(m.timesUpdated)
+	m.status.BatVoltage /= float64(m.timesUpdated)
 }

-func zeroMuninValues(muninDat *muninData) {
-	muninDat.timesUpdated = 0
-	muninVal := muninDat.status
-	muninVal.OutCurrent = 0
-	muninVal.InCurrent = 0
-	muninVal.BatCurrent = 0
+func zeroMuninValues(m *muninData) {
+	m.timesUpdated = 0
+	m.status.OutCurrent = 0
+	m.status.InCurrent = 0
+	m.status.BatCurrent = 0

-	muninVal.OutVoltage = 0
-	muninVal.InVoltage = 0
-	muninVal.BatVoltage = 0
+	m.status.OutVoltage = 0
+	m.status.InVoltage = 0
+	m.status.BatVoltage = 0

-	muninVal.InFrequency = 0
-	muninVal.OutFrequency = 0
+	m.status.InFrequency = 0
+	m.status.OutFrequency = 0

-	muninVal.ChargeState = 0
+	m.status.ChargeState = 0
 }

 type templateInput struct {
--- a/plugins/munin/munin_test.go
+++ b/plugins/munin/munin_test.go
@@ -0,0 +1,29 @@
+package munin
+
+import (
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/diebietse/invertergui/mk2driver"
+)
+
+func TestServer(t *testing.T) {
+
+	mockMk2 := mk2driver.NewMk2Mock()
+	muninServer := NewMunin(mockMk2)
+
+	ts := httptest.NewServer(http.HandlerFunc(muninServer.ServeMuninHTTP))
+	defer ts.Close()
+
+	res, err := http.Get(ts.URL)
+	if err != nil {
+		log.Fatal(err)
+	}
+	_, err = ioutil.ReadAll(res.Body)
+	res.Body.Close()
+	if err != nil {
+		log.Fatal(err)
+	}
+}
Author	SHA1	Message	Date
Hendrik van Wyk	cc8fa9d611	Merge pull request #20 from diebietse/scalefixes Fix scaling decoding and munin race condition	2020-10-08 12:29:58 +02:00
Hendrik van Wyk	49be089a23	Fix race condition in munin output. The munin server used the same structure in two goroutines at once causing possible data corruption. A copy of the structure is now used by the second goroutine instead.	2020-10-08 12:25:39 +02:00
Hendrik van Wyk	157736a99d	Add optional debug logging for frame decoding.	2020-10-08 12:25:33 +02:00
Hendrik van Wyk	86f3f0c8e3	Fix scaling to more closely match the Victron documentation. We were decoding the scale as unsigned while it is signed. We were also ignoring the fact that the sign of the scale determines the signedness of the value it scales.	2020-09-25 15:03:26 +02:00
Nicholas Thompson	c991503e33	Add mode-2 to scale factors	2020-09-19 18:38:00 +02:00
Nicholas Thompson	55ae241d92	Merge pull request #17 from diebietse/scale-factor-fixes Fix scale factor issue #16	2020-09-13 21:58:57 +02:00
Nicholas Thompson	ab346bcf90	Disable dead code check for RAM IDs	2020-09-13 21:14:24 +02:00
Nicholas Thompson	4c6df96051	Add unit test to scaleDecode	2020-09-13 20:56:26 +02:00
Nicholas Thompson	2a56dd24e4	Cleanup mk2 logging	2020-09-13 20:56:22 +02:00
Nicholas Thompson	65d9429a12	Add constants to frame decoder	2020-09-13 20:56:15 +02:00
Nicholas Thompson	5fb5ce5f12	Add check to scaling factors Fix scaling append	2020-09-13 20:52:39 +02:00