cmseaton42 / node-ethernet-ip

A Lightweight Ethernet/IP API written to interface with Rockwell ControlLogix/CompactLogix Controllers.
MIT License
265 stars 106 forks source link

Wrong comparison reading Atomic BOOL #29

Closed pfumagalli closed 6 years ago

pfumagalli commented 6 years ago

Current Behavior

I am trying to read from a Logix5572 (firmware 31.011) and when reading an Atomic BOOL, I see that the value returned by the PLC in the data buffer at index 2 is either 0x00 for false or 0x01 for true.

In line 426 of src/tag/index.js the code does a hard comparison matching only 0xff to true

This also seem to be the value written to the PLC in lines 551/552 of src/tag/index.js

Expected Behavior

I personally think that any non-zero value should be considered as true.

Possible Solution (Optional)

Change line 426 of src/tag/index.js of from:

this.controller_value = data.readUInt8(2) === 0xff ? true : false;

to

this.controller_value = data.readUInt8(2) !== 0;

A pull request is available as #30

Context

We are reading whether some tags in the PLC are either true or false, and the value returned by the library does not change when the PLC value changes

Steps to Reproduce (for bugs only)

  1. Define a BOOL value in the PLC (currently doing this with Logix Designer studio 5000)
  2. Change the value of said value from 0 to 1 and vice versa
  3. The library always returns false

Your Environment

jhenson29 commented 6 years ago

I’ll look into this tonight.

jhenson29 commented 6 years ago

Well. This is at least somewhat comical. Issue #19 was that bool read didn't work because of the following line

this.controller_value = data.readUInt8(2) === 0x01 ? true : false;

which was changed to

this.controller_value = data.readUInt8(2) === 0xff ? true : false;

per both testing and the docs.

Data Access.pdf, page 17:

When reading a BOOL tag, the values returned for 0 and 1 are 0 and 0xFF, respectively.

I checked a v20 and a v31 processor today. v20 returns 0xFF and v31 return 0x01. Maybe the line should have been

this.controller_value = data.readUInt8(2) === 0x01 ? true : data.readUInt8(2) === 0xFF ? true : false;

I think if we keep chaining ternaries, we'll eventually get it right. 😁

cmseaton42 commented 6 years ago

Closed by #30