From 5fb5ce5f12beb063fd93902138a3334ce597c3d1 Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Wed, 9 Sep 2020 18:11:57 +0200 Subject: [PATCH 1/5] Add check to scaling factors Fix scaling append --- mk2driver/mk2.go | 69 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index 5c3c33e..7079bdd 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -11,10 +11,31 @@ import ( ) type scaling struct { - scale float64 - offset float64 + scale float64 + offset float64 + supported bool } +const ( + ramVarVMains = iota + ramVarIMains + ramVarVInverter + ramVarIInverter + ramVarVBat + ramVarIBat + ramVarVBatRipple + ramVarInverterPeriod + ramVarMainPeriod + ramVarIACLoad + ramVarVirSwitchPos + ramVarIgnACInState + ramVarMultiFuncRelay + ramVarChargeState + ramVarInverterPower1 + ramVarInverterPower2 + ramVarOutPower +) + type mk2Ser struct { info *Mk2Info p io.ReadWriter @@ -179,18 +200,23 @@ func (m *mk2Ser) reqScaleFactor(in byte) { // 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))) + if len(frame) <= 2 { + tmp.supported = false + log.Printf("Skiping scaling factors for: %d", m.scaleCount) } else { - tmp.scale = math.Abs(float64(scl)) + 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))) + } else { + tmp.scale = math.Abs(float64(scl)) + } } m.scales = append(m.scales, tmp) - m.scaleCount++ if m.scaleCount < 14 { m.reqScaleFactor(byte(m.scaleCount)) @@ -222,6 +248,9 @@ func (m *mk2Ser) versionDecode(frame []byte) { // 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) } @@ -237,13 +266,13 @@ 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.applyScale(getSigned(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)) // Send L1 status request cmd := make([]byte, 2) @@ -254,15 +283,15 @@ func (m *mk2Ser) dcDecode(frame []byte) { // 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.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) 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)) } // Send status request @@ -273,7 +302,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]), 13) + m.info.ChargeState = m.applyScale(getSigned(frame[1:3]), ramVarChargeState) m.updateReport() } From 65d9429a12b902d7496d10d87ab5539a5e825f40 Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Sun, 13 Sep 2020 15:59:33 +0200 Subject: [PATCH 2/5] Add constants to frame decoder --- mk2driver/mk2.go | 105 +++++++++++++++++++++++++++++++---------------- 1 file changed, 69 insertions(+), 36 deletions(-) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index 7079bdd..df83cb8 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -34,6 +34,38 @@ const ( 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 { @@ -53,7 +85,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) @@ -64,9 +96,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: @@ -75,34 +106,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") } } } - size = tmp + frameLength = tmp } } } @@ -150,27 +183,27 @@ func (m *mk2Ser) updateReport() { func (m *mk2Ser) handleFrame(l byte, frame []byte) { 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:]) } } @@ -183,7 +216,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) @@ -192,8 +225,8 @@ 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) } @@ -218,7 +251,7 @@ func (m *mk2Ser) scaleDecode(frame []byte) { } 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.") @@ -234,14 +267,14 @@ func (m *mk2Ser) versionDecode(frame []byte) { 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) } } @@ -276,8 +309,8 @@ func (m *mk2Ser) dcDecode(frame []byte) { // Send L1 status request cmd := make([]byte, 2) - cmd[0] = 0x46 //F - cmd[1] = 1 + cmd[0] = infoReqFrame + cmd[1] = infoReqAddrACL1 m.sendCommand(cmd) } @@ -296,7 +329,7 @@ func (m *mk2Ser) acDecode(frame []byte) { // Send status request cmd := make([]byte, 1) - cmd[0] = 0x4C //F + cmd[0] = ledFrame m.sendCommand(cmd) } @@ -312,9 +345,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) } @@ -341,7 +374,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] From 2a56dd24e417c9e56cfbaf908f7db90f45c599a3 Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Sun, 13 Sep 2020 15:59:49 +0200 Subject: [PATCH 3/5] Cleanup mk2 logging --- mk2driver/mk2.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index df83cb8..324ff34 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -4,10 +4,11 @@ import ( "errors" "fmt" "io" - "log" "math" "sync" "time" + + "github.com/sirupsen/logrus" ) type scaling struct { @@ -131,7 +132,7 @@ func (m *mk2Ser) frameLocker() { } else { if checkChecksum(frameLength, tmp, frame[:frameLengthOffset]) { m.frameLock = true - log.Printf("Locked") + logrus.Info("Locked") } } } @@ -208,7 +209,7 @@ func (m *mk2Ser) handleFrame(l byte, frame []byte) { } } } else { - log.Printf("Invalid incoming frame checksum: %x", frame) + logrus.Errorf("Invalid incoming frame checksum: %x", frame) m.frameLock = false } } @@ -236,7 +237,7 @@ func (m *mk2Ser) scaleDecode(frame []byte) { tmp := scaling{} if len(frame) <= 2 { tmp.supported = false - log.Printf("Skiping scaling factors for: %d", m.scaleCount) + logrus.Warnf("Skiping scaling factors for: %d", m.scaleCount) } else { tmp.supported = true scl := uint16(frame[2])<<8 + uint16(frame[1]) @@ -254,9 +255,8 @@ func (m *mk2Ser) scaleDecode(frame []byte) { if m.scaleCount < ramVarMaxOffset { m.reqScaleFactor(byte(m.scaleCount)) } else { - log.Print("Monitoring starting.") + logrus.Info("Monitoring starting.") } - } // Decode the version number From 4c6df96051a66ca9008cf1fe4249a0c5583ea46d Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Sun, 13 Sep 2020 16:49:02 +0200 Subject: [PATCH 4/5] Add unit test to scaleDecode --- mk2driver/mk2.go | 2 +- mk2driver/mk2_test.go | 121 +++++++++++++++++++++++------------------- 2 files changed, 66 insertions(+), 57 deletions(-) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index 324ff34..6f49650 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -235,7 +235,7 @@ func (m *mk2Ser) reqScaleFactor(in byte) { // Decode the scale factor frame. func (m *mk2Ser) scaleDecode(frame []byte) { tmp := scaling{} - if len(frame) <= 2 { + if len(frame) < 6 { tmp.supported = false logrus.Warnf("Skiping scaling factors for: %d", m.scaleCount) } else { diff --git a/mk2driver/mk2_test.go b/mk2driver/mk2_test.go index 871132b..7326e20 100644 --- 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) + } + }) + } } From ab346bcf9033b72c4eb30d319fc242ea0fef51e6 Mon Sep 17 00:00:00 2001 From: Nicholas Thompson Date: Sun, 13 Sep 2020 21:14:24 +0200 Subject: [PATCH 5/5] Disable dead code check for RAM IDs --- mk2driver/mk2.go | 1 + 1 file changed, 1 insertion(+) diff --git a/mk2driver/mk2.go b/mk2driver/mk2.go index 6f49650..ba0af6e 100644 --- a/mk2driver/mk2.go +++ b/mk2driver/mk2.go @@ -17,6 +17,7 @@ type scaling struct { supported bool } +//nolint:deadcode,varcheck const ( ramVarVMains = iota ramVarIMains