MangoAutomation / BACnet4J

BACnet/IP stack written in Java. Forked from http://sourceforge.net/projects/bacnet4j/
GNU General Public License v3.0
183 stars 110 forks source link

BinaryPV seems to be incompatible with YABE Room Control Simulator #21

Open mattnathan opened 6 years ago

mattnathan commented 6 years ago

I'm having an issue where the BACnet4J library is unable to parse the binary present-values that are returned by the YABE ( https://sourceforge.net/projects/yetanotherbacnetexplorer/ ) Room Control Simulator.

I get exceptions like:

java.lang.ArrayIndexOutOfBoundsException: -1
    at com.serotonin.bacnet4j.util.sero.ByteQueue.pop(ByteQueue.java:291)
    at com.serotonin.bacnet4j.type.primitive.Enumerated.<init>(Enumerated.java:113)
    at com.serotonin.bacnet4j.type.enumerated.BinaryPV.<init>(BinaryPV.java:74)
    ... 33 more

After a lot of digging it seems to be caused by an inconsistency between how YABE and BACnet4J handle the serial form of the BinaryPV. I don't have access to the BACnet specification to understand which library is correct, however I can tell you what the difference is.

BACnet4J treats BinaryPV as an Enumeration and writes 9100 and 9101 (hex) for inactive and active respectively. YABE treats BinaryPV differently and writes 10 and 11 (hex) for inactive and active respectively.

In the case where BACnet4J receives an inactive response (10) from YABE it reads it as a length of 0 and defaults the BinaryPV to inactive, but in the case where 11 is received it reads it as a length of 1 and attempts to read the next byte in the buffer causing the ArrayIndexOutOfBoundsException: -1 you see above.

If I were to make a guess (totally without any prior knowledge of what the spec actually says) I'd say that there exists a condensed version of numerical values which uses the least significant 4 bits of the byte as the value when the most significant 4 bits are 0001 and Enumeration isn't taking this into account. For example 00010001 is a value of 1 not a length of 1

All using BACnet4J 4.1.4, YABE 1.1.6.0

Simple test case that showcases the issue.

// test that condensed numerical values are supported
assertEquals(BinaryPV.inactive, new BinaryPV(new ByteQueue("10")));
assertEquals(BinaryPV.active, new BinaryPV(new ByteQueue("11")));
// test that standard numerical values are supported
assertEquals(BinaryPV.inactive, new BinaryPV(new ByteQueue("9100")));
assertEquals(BinaryPV.active, new BinaryPV(new ByteQueue("9101")));
mlohbihler commented 6 years ago

I suspect that YABE is returning a Boolean value instead of a BinaryPV.

mattnathan commented 6 years ago

Again, no idea what the spec says. Is this something that is allowed or is YABE acting outside the spec?

mlohbihler commented 6 years ago

The spec is very, ... well, specific about datatypes. The binary types (input, output, and value) all use a BinaryPV as their present values. Booleans are only used as service parameters. I expect that YABE is doing the wrong thing, although i would welcome evidence to the contrary. I would also say that BACnet4J might handle cases like this more elegantly (although i don't recall off hand how exactly it is handling this case).