Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for nil Reading.Value. Scheduler will fail on nil reading. #65

Merged
merged 1 commit into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions pkg/devices/coils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 11 additions & 5 deletions pkg/devices/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 13 additions & 7 deletions pkg/devices/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions pkg/devices/holding_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/devices/input_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down