Merge pull request #20 from diebietse/scalefixes

Fix scaling decoding and munin race condition

Makefile

@@ -39,9 +39,6 @@ gofmt:
 	gofmt -l -s -w .
 
 test:
-	go test -v ./...
-
-test-race:
 	go test -v -race ./...
 
 docker:

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

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) {

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 {

mk2driver/mk2.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"math"
 	"sync"
 	"time"
 
@@ -14,6 +13,7 @@ import (
 type scaling struct {
 	scale     float64
 	offset    float64
+	signed    bool
 	supported bool
 }
 
@@ -183,6 +183,7 @@ 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 frameHeader:
@@ -233,24 +234,43 @@ func (m *mk2Ser) reqScaleFactor(in byte) {
 	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) {
 	tmp := scaling{}
+	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.supported = true
-		scl := uint16(frame[2])<<8 + uint16(frame[1])
-		ofs := int16(uint16(frame[5])<<8 + uint16(frame[4]))
-
+		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)
-		if scl >= 0x4000 {
-			tmp.scale = math.Abs(1 / (0x8000 - float64(scl)))
+		scale := int16Abs(scl)
+		if scale >= 0x4000 {
+			tmp.scale = 1 / (0x8000 - float64(scale))
 		} else {
-			tmp.scale = math.Abs(float64(scl))
+			tmp.scale = float64(scale)
 		}
 	}
+	logrus.Debugf("scalecount %v: %#v \n", m.scaleCount, tmp)
 	m.scales = append(m.scales, tmp)
 	m.scaleCount++
 	if m.scaleCount < ramVarMaxOffset {
@@ -262,6 +282,7 @@ func (m *mk2Ser) scaleDecode(frame []byte) {
 
 // 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++ {
@@ -280,6 +301,20 @@ func (m *mk2Ser) versionDecode(frame []byte) {
 	}
 }
 
+// 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 {
@@ -293,20 +328,26 @@ 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)
 }
 
 // Decodes DC frame.
 func (m *mk2Ser) dcDecode(frame []byte) {
-	m.info.BatVoltage = m.applyScale(getSigned(frame[5:7]), ramVarVBat)
+	m.info.BatVoltage = m.applyScaleAndSign(frame[5:7], ramVarVBat)
 
 	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]), ramVarInverterPeriod))
+	logrus.Debugf("dcDecode %#v", m.info)
 
 	// Send L1 status request
 	cmd := make([]byte, 2)
@@ -317,16 +358,17 @@ func (m *mk2Ser) dcDecode(frame []byte) {
 
 // Decodes AC frame.
 func (m *mk2Ser) acDecode(frame []byte) {
-	m.info.InVoltage = m.applyScale(getSigned(frame[5:7]), ramVarVMains)
-	m.info.InCurrent = m.applyScale(getSigned(frame[7:9]), ramVarIMains)
-	m.info.OutVoltage = m.applyScale(getSigned(frame[9:11]), ramVarVInverter)
-	m.info.OutCurrent = m.applyScale(getSigned(frame[11:13]), ramVarIInverter)
+	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]), ramVarMainPeriod))
 	}
+	logrus.Debugf("acDecode %#v", m.info)
 
 	// Send status request
 	cmd := make([]byte, 1)
@@ -336,7 +378,8 @@ func (m *mk2Ser) acDecode(frame []byte) {
 
 // Decode charge state of battery.
 func (m *mk2Ser) stateDecode(frame []byte) {
-	m.info.ChargeState = m.applyScale(getSigned(frame[1:3]), ramVarChargeState)
+	m.info.ChargeState = m.applyScaleAndSign(frame[1:3], ramVarChargeState)
+	logrus.Debugf("battery state decode %#v", m.info)
 	m.updateReport()
 }
 
@@ -383,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))

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 {

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)
+	}
+}