From c991503e33f7e46d54dc0f24d406ebbd0c3784a6 Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Sat, 19 Sep 2020 18:38:00 +0200 Subject: [PATCH 1/4] Add mode-2 to scale factors --- mk2driver/mk2.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index ba0af6e..c8d0fc8 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -236,9 +236,21 @@ func (m *mk2Ser) reqScaleFactor(in byte) { // Decode the scale factor frame. func (m *mk2Ser) scaleDecode(frame []byte) { tmp := scaling{} + logrus.Infof("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 if len(frame) == 6 { + tmp.supported = true + scl := uint16(frame[2])<<8 + uint16(frame[1]) + ofs := int16(uint16(frame[4])<<8 + uint16(frame[3])) + + tmp.offset = float64(ofs) + if scl >= 0x4000 { + tmp.scale = math.Abs(1 / (0x8000 - float64(scl))) + } else { + tmp.scale = math.Abs(float64(scl)) + } } else { tmp.supported = true scl := uint16(frame[2])<<8 + uint16(frame[1]) From 86f3f0c8e3958b93a869f5e16acf606dc0c300e5 Mon Sep 17 00:00:00 2001 From: Hendrik van Wyk Date: Fri, 25 Sep 2020 15:03:26 +0200 Subject: [PATCH 2/4] 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. --- mk2driver/mk2.go | 75 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 25 deletions(-) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index c8d0fc8..7a34be7 100644 --- a/mk2driver/mk2.go +++ b/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 } @@ -233,6 +233,13 @@ 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{} @@ -240,27 +247,26 @@ func (m *mk2Ser) scaleDecode(frame []byte) { if len(frame) < 6 { tmp.supported = false logrus.Warnf("Skiping scaling factors for: %d", m.scaleCount) - } else if len(frame) == 6 { - tmp.supported = true - scl := uint16(frame[2])<<8 + uint16(frame[1]) - ofs := int16(uint16(frame[4])<<8 + uint16(frame[3])) - - tmp.offset = float64(ofs) - if scl >= 0x4000 { - tmp.scale = math.Abs(1 / (0x8000 - float64(scl))) - } else { - tmp.scale = math.Abs(float64(scl)) - } } else { tmp.supported = true - scl := uint16(frame[2])<<8 + uint16(frame[1]) - ofs := int16(uint16(frame[5])<<8 + uint16(frame[4])) - - tmp.offset = float64(ofs) - if scl >= 0x4000 { - tmp.scale = math.Abs(1 / (0x8000 - float64(scl))) + 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 { - tmp.scale = math.Abs(float64(scl)) + 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) } } m.scales = append(m.scales, tmp) @@ -292,6 +298,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 { @@ -305,6 +325,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) @@ -312,7 +337,7 @@ func getUnsigned(data []byte) float64 { // 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) @@ -329,10 +354,10 @@ 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 @@ -348,7 +373,7 @@ 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) m.updateReport() } From 157736a99d08209829042c4b9140498e09bd05e3 Mon Sep 17 00:00:00 2001 From: Hendrik van Wyk Date: Fri, 25 Sep 2020 15:03:49 +0200 Subject: [PATCH 3/4] Add optional debug logging for frame decoding. --- README.md | 7 +++---- cmd/invertergui/config.go | 1 + cmd/invertergui/main.go | 5 +++++ mk2driver/mk2.go | 9 ++++++++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 38e469c..1be2004 100644 --- 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 diff --git a/cmd/invertergui/config.go b/cmd/invertergui/config.go index 79de9a2..9cdad85 100644 --- 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) { diff --git a/cmd/invertergui/main.go b/cmd/invertergui/main.go index 4bb57c3..84f5cb0 100644 --- 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 { diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index 7a34be7..5c7d3b8 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -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: @@ -243,7 +244,7 @@ func int16Abs(in int16) uint16 { // Decode the scale factor frame. func (m *mk2Ser) scaleDecode(frame []byte) { tmp := scaling{} - logrus.Infof("Scale frame(%d): 0x%x", len(frame), frame) + 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) @@ -269,6 +270,7 @@ func (m *mk2Ser) scaleDecode(frame []byte) { 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 { @@ -280,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++ { @@ -344,6 +347,7 @@ func (m *mk2Ser) dcDecode(frame []byte) { 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) @@ -364,6 +368,7 @@ func (m *mk2Ser) acDecode(frame []byte) { } else { m.info.InFrequency = 10 / (m.applyScale(float64(frame[13]), ramVarMainPeriod)) } + logrus.Debugf("acDecode %#v", m.info) // Send status request cmd := make([]byte, 1) @@ -374,6 +379,7 @@ func (m *mk2Ser) acDecode(frame []byte) { // Decode charge state of battery. func (m *mk2Ser) stateDecode(frame []byte) { m.info.ChargeState = m.applyScaleAndSign(frame[1:3], ramVarChargeState) + logrus.Debugf("battery state decode %#v", m.info) m.updateReport() } @@ -420,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)) From 49be089a231d848b56d53a6dda5c7879b46589fd Mon Sep 17 00:00:00 2001 From: Hendrik van Wyk Date: Sat, 26 Sep 2020 13:35:11 +0200 Subject: [PATCH 4/4] 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. --- Makefile | 3 -- plugins/munin/munin.go | 76 ++++++++++++++++++------------------- plugins/munin/munin_test.go | 29 ++++++++++++++ 3 files changed, 65 insertions(+), 43 deletions(-) create mode 100644 plugins/munin/munin_test.go diff --git a/Makefile b/Makefile index be161b4..3f95a73 100644 --- a/Makefile +++ b/Makefile @@ -39,9 +39,6 @@ gofmt: gofmt -l -s -w . test: - go test -v ./... - -test-race: go test -v -race ./... docker: diff --git a/plugins/munin/munin.go b/plugins/munin/munin.go index d85ba16..ba2d333 100644 --- 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 { diff --git a/plugins/munin/munin_test.go b/plugins/munin/munin_test.go new file mode 100644 index 0000000..c34b85b --- /dev/null +++ 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) + } +}