From 99f21fd0e861332adcc6f10edbc054534816187c Mon Sep 17 00:00:00 2001 From: MatthewHink Date: Wed, 12 Aug 2020 07:06:44 -0700 Subject: [PATCH] Check for nil Reading.Value. Scheduler will fail on nil reading. --- Makefile | 2 +- pkg/devices/coils.go | 2 ++ pkg/devices/common.go | 16 +++++++++++----- pkg/devices/device_test.go | 20 +++++++++++++------- pkg/devices/holding_register.go | 2 ++ pkg/devices/input_register.go | 2 ++ 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index bbcd12a..55d085f 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ # PLUGIN_NAME := modbus-ip -PLUGIN_VERSION := 2.0.6 +PLUGIN_VERSION := 2.0.7 IMAGE_NAME := vaporio/modbus-ip-plugin BIN_NAME := synse-modbus-ip-plugin diff --git a/pkg/devices/coils.go b/pkg/devices/coils.go index fedb9d6..9a55321 100644 --- a/pkg/devices/coils.go +++ b/pkg/devices/coils.go @@ -59,6 +59,8 @@ func bulkReadCoils(devices []*sdk.Device) (readContexts []*sdk.ReadContext, err var readResults []byte readResults, err = client.ReadCoils(read.StartRegister, read.RegisterCount) incrementModbusCallCounter() + log.Debugf("[modbus call]: ReadCoils(0x%x, 0x%x), result: %v, len(d%d), err: %v\n", + read.StartRegister, read.RegisterCount, readResults, len(readResults), err) if err != nil { log.Errorf("modbus bulk read coils failure: %v", err.Error()) if modbusDeviceData.FailOnError { diff --git a/pkg/devices/common.go b/pkg/devices/common.go index 6c19978..55c645e 100644 --- a/pkg/devices/common.go +++ b/pkg/devices/common.go @@ -71,10 +71,12 @@ func UnpackCoilReading(output *output.Output, rawReading []byte, startAddress ui bitIndex = bitIndex % 8 if int(byteIndex) >= len(rawReading) { + // Make a reading with a nil Reading.Value. + reading = output.MakeReading(nil) if failOnErr { - return nil, fmt.Errorf("failed to get coil data") + return reading, fmt.Errorf("failed to get coil data") } - return nil, nil // No Reading + return reading, nil // Reading with nil reading.Value } coilByte := rawReading[byteIndex] @@ -93,11 +95,13 @@ func UnpackReading(output *output.Output, typeName string, rawReading []byte, fa // Cast the raw reading value to the specified output type data, err := utils.CastToType(typeName, rawReading) if err != nil { + // Make a reading with a nil Reading.Value. + reading = output.MakeReading(nil) log.Errorf("Failed to cast typeName: %v, rawReading: %x", typeName, rawReading) if failOnErr { - return nil, err + return reading, err } - return nil, nil // No reading. + return reading, nil // No reading. } return output.MakeReading(data), nil @@ -400,7 +404,9 @@ func MapBulkReadData(bulkReadMap map[ModbusBulkReadKey][]*ModbusBulkRead, keyOrd // nil reading. log.Errorf("No data. Attempt to read beyond bounds. startDataOffset: %v, endDataOffset: %v, readResultsLength: %v", startDataOffset, endDataOffset, readResultsLength) - readings = append(readings, nil) + // Make a reading with a nil Reading.Value. + reading = theOutput.MakeReading(nil) + readings = append(readings, reading) // Append a read context here for the nil reading. readContext := sdk.NewReadContext(device, readings) readContexts = append(readContexts, readContext) diff --git a/pkg/devices/device_test.go b/pkg/devices/device_test.go index d9a9025..13c5571 100644 --- a/pkg/devices/device_test.go +++ b/pkg/devices/device_test.go @@ -224,9 +224,9 @@ func verifyReadContexts(t *testing.T, expected []*sdk.ReadContext, actual []*sdk t.Logf("*** verifyReadContexts end ------------------------\n") } -// verifySingleNilReading verifies that there is one read context with one +// verifySingleNilReadingValue verifies that there is one read context with one // reading that is nil. -func verifySingleNilReading(t *testing.T, readContexts []*sdk.ReadContext) { +func verifySingleNilReadingValue(t *testing.T, readContexts []*sdk.ReadContext) { if len(readContexts) != 1 { t.Fatalf("Expected 1 read context, got %v", len(readContexts)) } @@ -236,8 +236,14 @@ func verifySingleNilReading(t *testing.T, readContexts []*sdk.ReadContext) { t.Fatalf("Expected 1 reading, got %v", len(readContexts[0].Reading)) } - if readContexts[0].Reading[0] != nil { - t.Fatalf("Expected nil reading, got %#v", readContexts[0].Reading[0]) + // The scheduler was dereferencing ReadContext.Reading.Value here, + // so don't give it a nil reading but a nil reading value. + if readContexts[0].Reading[0] == nil { + t.Fatalf("Expected non-nil reading, got %v\n", readContexts[0].Reading[0]) + } + + if readContexts[0].Reading[0].Value != nil { + t.Fatalf("Expected nil reading value, got %#v", readContexts[0].Reading[0].Value) } } @@ -3511,7 +3517,7 @@ func TestReadHoldingRegisters_NoConnection(t *testing.T) { if err != nil { t.Fatalf(err.Error()) } - verifySingleNilReading(t, readContexts) + verifySingleNilReadingValue(t, readContexts) } // Unable to connect to the device. Fail on error is true, which fails all @@ -3583,7 +3589,7 @@ func TestReadInputRegisters_NoConnection(t *testing.T) { if err != nil { t.Fatalf(err.Error()) } - verifySingleNilReading(t, readContexts) + verifySingleNilReadingValue(t, readContexts) } // Unable to connect to the device. Fail on error is false, which allows @@ -3615,7 +3621,7 @@ func TestReadCoils_NoConnection(t *testing.T) { if err != nil { t.Fatalf(err.Error()) } - verifySingleNilReading(t, readContexts) + verifySingleNilReadingValue(t, readContexts) } // We will need a read (modbus over IP call) for each device below due to different IPs. diff --git a/pkg/devices/holding_register.go b/pkg/devices/holding_register.go index ad5848b..cbeb00f 100644 --- a/pkg/devices/holding_register.go +++ b/pkg/devices/holding_register.go @@ -60,6 +60,8 @@ func bulkReadHoldingRegisters(devices []*sdk.Device) (readContexts []*sdk.ReadCo var readResults []byte readResults, err = client.ReadHoldingRegisters(read.StartRegister, read.RegisterCount) incrementModbusCallCounter() + log.Debugf("[modbus call]: ReadHoldingRegisters(0x%x, 0x%x), result: %v, len(d%d), err: %v\n", + read.StartRegister, read.RegisterCount, readResults, len(readResults), err) if err != nil { log.Errorf("modbus bulk read holding registers failure: %v", err.Error()) if modbusDeviceData.FailOnError { diff --git a/pkg/devices/input_register.go b/pkg/devices/input_register.go index c1c85b9..1c46ac1 100644 --- a/pkg/devices/input_register.go +++ b/pkg/devices/input_register.go @@ -49,6 +49,8 @@ func bulkReadInputRegisters(devices []*sdk.Device) (readContexts []*sdk.ReadCont var readResults []byte readResults, err = client.ReadInputRegisters(read.StartRegister, read.RegisterCount) incrementModbusCallCounter() + log.Debugf("[modbus call]: ReadInputRegisters(0x%x, 0x%x), result: %v, len(d%d), err: %v\n", + read.StartRegister, read.RegisterCount, readResults, len(readResults), err) if err != nil { log.Errorf("modbus bulk read input registers failure: %v", err.Error()) if deviceData.FailOnError {