• J
    joolz

    There is a bug in the way the Destination class determines whether or not the specified timestamp falls on a valid day.

    In com.serotonin.bacnet4j.type.constructed.Destination.java:

    public boolean isSuitableForEvent(TimeStamp timeStamp, EventState toState) {
        // Only check date fields if the timestamp is not a sequence number.
        if (!timeStamp.isSequenceNumber()) {
            // Check if the timestamp day of week is in the valid days of week list.
            if (!validDays.contains(timeStamp.getDateTime().getDate().getDayOfWeek().getId()))
                return false;
    
            // Check if the timestamp is between the from and to times.
            if (timeStamp.getDateTime().getTime().before(fromTime) || timeStamp.getDateTime().getTime().after(toTime))
                return false;
        }
    
        // Check if the destination is interested in this new event state.
        return transitions.contains(toState);
    }
    

    and com.serotonin.bacnet4j.type.constructed.DaysOfWeek.java

    public boolean contains(int day) {
        return getValue()[day];
    }
    

    timeStamp.getDateTime().getDate().getDayOfWeek().getId() returns a number from 1-7
    However, the DaysOfWeek class stores the days in a 0 indexed array 0-6, which means the wrong index is checked every time this function is called.

    posted in BACnet4J general discussion read more
  • J
    joolz

    I have recently discovered that if you were to create two objects of the same type which have a priority array (e.g. BV), if you set the Priority at index 3 in BV1, it will also set it in BV2. This happens because in ObjectProperties.java, the static add() function creates a new PriorityArray instance which is then passed into all objects of the same type when they are created. This means the same PriorityArray object is used and passed into all BV instances when they are created.

    The solution to this is to after creating your BACnet object (AO, AV, BO, BV etc), call the setProperty() function, and create a new PriorityArray instance;
    I.E
    BACnetObject bv1 = new BACnetObject(.....);
    bv1.setProperty(PropertyIdentifier.priorityArray, new PriorityArray());

    posted in BACnet4J general discussion read more
  • J
    joolz

    I have, but BACnet4j doesn't make it straight forward.

    First you need to subclass BACnetObject and make a Schedule object class.
    In that schedule object class, have a local variable for schedule default property.
    In the constructor, set the present value and schedule default properties with this value:
    new AmbiguousValue(new ByteQueue(new byte[]{0}))

    Then override the get() and set() methods, so that if the schedule default property is being set/retrieved, you get it from your local variable instead of the property map.

    posted in BACnet4J general discussion read more
  • J
    joolz

    I am trying to create a schedule object in BACnet4j using the following (simplified for clarity):

    
    import com.serotonin.bacnet4j.LocalDevice;
    import com.serotonin.bacnet4j.exception.BACnetServiceException;
    import com.serotonin.bacnet4j.npdu.ip.IpNetwork;
    import com.serotonin.bacnet4j.obj.BACnetObject;
    import com.serotonin.bacnet4j.transport.Transport;
    import com.serotonin.bacnet4j.type.enumerated.ObjectType;
    import com.serotonin.bacnet4j.type.enumerated.PropertyIdentifier;
    import com.serotonin.bacnet4j.type.primitive.Enumerated;
    import com.serotonin.bacnet4j.type.primitive.ObjectIdentifier;
    
    
    public class ScheduleTest
    {
    	public static void main(String[] args)
    	{
    		try
    		{
    			LocalDevice localDevice = new LocalDevice(21, new Transport(new IpNetwork("255.255.255.0")));
    			BACnetObject schedule = new BACnetObject(localDevice, new ObjectIdentifier(ObjectType.schedule, 0));
    
    			schedule.setProperty(PropertyIdentifier.scheduleDefault, new Enumerated(0));
    		}
    		catch (BACnetServiceException e)
    		{
    			e.printStackTrace();
    		}
    	}
    }
    ```However, when I run it, I get this:
    

    com.serotonin.bacnet4j.exception.BACnetServiceException: class=Property, code=Invalid data type, message=expected class com.serotonin.bacnet4j.type.AmbiguousValue, received=class com.serotonin.bacnet4j.type.primitive.Enumerated
    at com.serotonin.bacnet4j.obj.ObjectProperties.validateValue(ObjectProperties.java:154)
    at com.serotonin.bacnet4j.obj.BACnetObject.setProperty(BACnetObject.java:192)
    at ScheduleTest.main(ScheduleTest.java:21)

    posted in BACnet4J general discussion read more
  • J
    joolz

    According to the standard, BACnetLogData is a choice of log-status, log-data or time-change.

    However, looking at com.serotonin.bacnet4j.type.constructed.LogData.java:```
    public LogData(ByteQueue queue) throws BACnetException {
    logStatus = read(queue, LogStatus.class, 0);
    logData = readSequenceOfChoice(queue, classes, 1);
    timeChange = read(queue, Real.class, 2);
    }

    @Override
    public void write(ByteQueue queue) {
        write(queue, logStatus, 0);
        write(queue, logData, 1);
        write(queue, timeChange, 2);
    }
    
    
    By changing
    
    public LogData(ByteQueue queue) throws BACnetException {
        logStatus = read(queue, LogStatus.class, 0);
        logData = readSequenceOfChoice(queue, classes, 1);
        timeChange = read(queue, Real.class, 2);
    }
    
    public LogData(ByteQueue queue) throws BACnetException {
        int tag = peekTagNumber(queue);
        if (tag == 0)
        {
            logStatus = read(queue, LogStatus.class, 0);
            logData = null;
            timeChange = null;
        }
        else if (tag == 1)
        {
            logStatus = null;
            logData = readSequenceOfChoice(queue, classes, 1);
            timeChange = null;
        }
        else if (tag == 2)
        {
            logStatus = null;
            logData = null;
            timeChange = read(queue, Real.class, 2);
        }
        else
        {
            throw new BACnetErrorException(ErrorClass.property, ErrorCode.missingRequiredParameter);
        }
    }
    

    posted in BACnet4J general discussion read more
  • J
    joolz

    The BACnet standard states that any attempt to read the LogBuffer property of a Trendlog or Trendlog Multiple object with a ReadProperty-Request or a ReadPropertyMultiple-Request shall cause a a Result(-) response, with an Error class of PROPERTY, and an Error Code of READ_ACCESS_DENIED.
    Currently BACnet4j does not do this.
    I've created a couple of patches which I believe fix this:

    
    diff --git a/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java b/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    --- a/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    +++ b/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    @@ -44,6 +44,8 @@
     import com.serotonin.bacnet4j.type.constructed.ReadAccessResult.Result;
     import com.serotonin.bacnet4j.type.constructed.ReadAccessSpecification;
     import com.serotonin.bacnet4j.type.constructed.SequenceOf;
    +import com.serotonin.bacnet4j.type.enumerated.ErrorClass;
    +import com.serotonin.bacnet4j.type.enumerated.ErrorCode;
     import com.serotonin.bacnet4j.type.enumerated.PropertyIdentifier;
     import com.serotonin.bacnet4j.type.primitive.ObjectIdentifier;
     import com.serotonin.bacnet4j.type.primitive.OctetString;
    @@ -151,7 +153,10 @@
             else {
                 // Get the specified property.
                 try {
    -                results.add(new Result(pid, pin, obj.getPropertyRequired(pid, pin)));
    +            	if (pid.equals(PropertyIdentifier.logBuffer))
    +            		results.add(new Result(pid, pin, new BACnetError(ErrorClass.property, ErrorCode.readAccessDenied)));
    +            	else
    +            		results.add(new Result(pid, pin, obj.getPropertyRequired(pid, pin)));
                 }
                 catch (BACnetServiceException e) {
                     results.add(new Result(pid, pin, new BACnetError(e.getErrorClass(), e.getErrorCode())));
    
    

    diff --git a/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java b/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    --- a/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    +++ b/src/com/serotonin/bacnet4j/service/confirmed/ReadPropertyMultipleRequest.java
    @@ -44,6 +44,8 @@
    import com.serotonin.bacnet4j.type.constructed.ReadAccessResult.Result;
    import com.serotonin.bacnet4j.type.constructed.ReadAccessSpecification;
    import com.serotonin.bacnet4j.type.constructed.SequenceOf;
    +import com.serotonin.bacnet4j.type.enumerated.ErrorClass;
    +import com.serotonin.bacnet4j.type.enumerated.ErrorCode;
    import com.serotonin.bacnet4j.type.enumerated.PropertyIdentifier;
    import com.serotonin.bacnet4j.type.primitive.ObjectIdentifier;
    import com.serotonin.bacnet4j.type.primitive.OctetString;
    @@ -151,7 +153,10 @@
    else {
    // Get the specified property.
    try {

    •            results.add(new Result(pid, pin, obj.getPropertyRequired(pid, pin)));
      
    •        	if (pid.equals(PropertyIdentifier.logBuffer))
      
    •        		results.add(new Result(pid, pin, new BACnetError(ErrorClass.property, ErrorCode.readAccessDenied)));
      
    •        	else
      
    •        		results.add(new Result(pid, pin, obj.getPropertyRequired(pid, pin)));
             }
             catch (BACnetServiceException e) {
                 results.add(new Result(pid, pin, new BACnetError(e.getErrorClass(), e.getErrorCode())));
      

    posted in BACnet4J general discussion read more
  • J
    joolz

    The current DeviceObjectPropertyReference hashCode implementation appears to not generate unique hashes.

    Take for example the following:

    DeviceObjectPropertyReference dopr1 = new DeviceObjectPropertyReference(new ObjectIdentifier(ObjectType.analogValue, 65), PropertyIdentifier.presentValue, null, new ObjectIdentifier(ObjectType.device, 1002));
    DeviceObjectPropertyReference dopr2 = new DeviceObjectPropertyReference(new ObjectIdentifier(ObjectType.analogValue, 3), PropertyIdentifier.presentValue, null, new ObjectIdentifier(ObjectType.device, 1004));
    System.out.println(dopr1.hashCode());
    System.out.println(dopr2.hashCode());
    
    

    Both return a value of 987574618.

    The javadocs for the hashCode() function state:

    It is not required that if two objects are unequal according to the java.lang.Object.equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hash tables.
    So technically this isn't a bug, but I don't so how anyone can reliably depend on the result of this function for anything useful (like using it as a key in a hashmap as I was).

    posted in BACnet4J general discussion read more
  • J
    joolz

    When I try to serialize a read property result (which occasionally creates an AmbiguousValue object) class I get a java.io.NotSerializableException exception.

    I believe this occurs because AmbiguousValue extends Encodable, which is Serializable, but the ByteQueue class does not. Would it also be possible to make ByteQueue serializable?

    posted in BACnet4J general discussion read more
  • J
    joolz

    Suppose a device of a given instance is discovered (say device 100, which is on 192.168.0.30), it gets added to the remoteDevices list in the LocalDevice object.

    Now if that IP address for device 100 gets updated (to say 192.168.0.31), when it is discovered by BACnet4j again, it checks the instance, address and network to see if it exists in the removeDevices list. Because it is different, the check fails, and it gets added to the remoteDevices list again, with new network information.

    The problem is that there are now two devices in the remoteDevices list which have the same device instance. If BACnet4j were to send out a request to device 100, it will used the old device network information, not the new one. It is worse if a device is accessed via an MSTP network, and is moved from one device to another.

    I've come up a fix to resolve the issue. I am having issues with accessing CVS for some reason, and because of that it isn't letting me create a patch, so here is a before and after:
    com.serotonin.bacnet4j.LocalDevice.java:570

    
        private RemoteDevice getRemoteDeviceImpl(int instanceId, Address address, Network network) {
            for (RemoteDevice d : remoteDevices) {
                if (d.getInstanceNumber() == instanceId && d.getAddress().equals(address)
                        && ObjectUtils.equals(d.getNetwork(), network))
                    return d;
            }
            return null;
        }
    
    

    Changed to:

        private RemoteDevice getRemoteDeviceImpl(int instanceId, Address address, Network network) {
            for (RemoteDevice d : remoteDevices) {
                if (d.getInstanceNumber() == instanceId) {
                	if (d.getAddress().equals(address) && ObjectUtils.equals(d.getNetwork(), network))
                		return d;
                	else
                		remoteDevices.remove(d);
                }
            }
            return null;
        }
    
    

    This will ensure that there cannot be two devices of the same instance in the remoteDevices list (which must be unique on the network as per the standard). Alternatively, the check could be done in getRemoteDeviceCreate() instead if the getRemoteDevice() function is used and removing it from the list is undesirable, however I found no reference to that function in BACnet4j and figured doing it this way would be more efficient.

    posted in BACnet4J general discussion read more