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.

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());

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.

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)

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)

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);
    }
}

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);
    }
}

Exception in thread "main" java.lang.NoClassDefFoundError: org/apache/commons/lang3/ObjectUtils  

You are missing a jar file (commons-lang3-3.0.jar) in your classpath

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())));
  

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())));
  

I am a bit confused by your response.
Looking at the top answer at the link you provided, they list 4 reasons.
1 - "Used to compare large amounts of data". Not really applicable here
2 - "Hashes can be used to index data". Yes! That's what I attempted, but when two different objects property references produce the same hash, it cannot be used as an index
3 - "They can be used in cryptographic applications like digital signatures". Not applicable.
4 - "Hashing can be used to generate seemingly random strings". Not really applicable here.

Looking at the second answer, the reason they describe as to why I would use a hash table is exactly what I was doing.

So, expanding on my original code:

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));
		
HashMap<Integer, Encodable> propertyValues = new HashMap<Integer, Encodable>();
Real value1 = new Real(20.0f);
Real value2 = new Real(30.0f);
propertyValues.put(dopr1.hashCode(), value1);
propertyValues.put(dopr2.hashCode(), value2);
System.out.println("Value1=" + propertyValues.get(dopr1.hashCode()));
System.out.println("Value2=" + propertyValues.get(dopr2.hashCode()));

```In theory, it should print out
Value1=20.0
Value2=30.0
but it actually prints
Value1=30.0
Value2=30.0

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).

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).

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?

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?

I am a little taken aback hearing that there are people using the same device instance for multiple devices on a network. They should be shot!

From what I can see however, with the code as is, even if they were happy to violate (as personally I don't think there is any level of compliance in this department) the standard and have same device instance for multiple devices, it will only ever work with the first device in the list if BACnet4j has to make any sort of request to that device (I could be wrong, but that is my observation). So I'm not really sure how useful the current implementation is to anyone, as it doesn't really handle multiple devices, and it doesn't handle devices that change network details.
I agree a flag in the localDevice object would be the simplest way to accomodate all parties, so that particular behaviour could be specified.

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.

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.

In
com.serotonin.bacnet4j.type.constructed.PropertyStates.java:118

public PropertyStates(int type, BaseType state) {
        this.state = new Choice(type, state);
    }

I think it should be

public PropertyStates(int type, Encodable state) {
        this.state = new Choice(type, state);
    }

Most of the possible choices for a PropertyStates object are of Enumerated type, which doesn't extend from BaseType. The Choice constructor takes an Encodable as its parameter, so it should work. In my own testing, it appears to work just fine.