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

RAD-4123 Allow null values for daily and exception schedule #97

Closed MertCingoz closed 2 months ago

MertCingoz commented 2 months ago

https://radixiot.atlassian.net/browse/RAD-4123

NULL shall be allowed to configure in the value (time-value) of Daily Schedule and Exception Schedule

ANSI/ASHRAE Standard 135-2016 Page: 293

The normal calculation of the value of the Present_Value property is illustrated as follows (the actual algorithm used is a local matter but shall yield the same results as this one):

  1. Find the highest relative priority (as defined by Clause 12.24.8) Exception_Schedule array element that is in effect for the current day and whose current value (see method below) is not NULL, and assign that value to the Present_Value property.
  2. If the Present_Value was not assigned in the previous step, then evaluate the current value of the Weekly_Schedule array element for the current day and if that value is not NULL, assign it to the Present_Value property.
  3. If the Present_Value was not assigned in the previous steps, then assign the value of the Schedule_Default property to the Present_Value property.
kishorevenki commented 2 months ago

As I mentioned above, in site the same schedule object would be configured for Real / BinaryPV/UnsignedInteger/etc, but one datatype at a time. That is Schedule,1 may be configured for AO,1 initially. Later same Schedule,1 may be configured for BO1, or MSO,1 etc. Depending upon the memory size it may not practically possible to add schedule object for each of such objects. Hence using WP request the configurations of Schedule object can be changed. This type of implementation is considered as good interoperability. So based on the above I noticed two issues here in the line 191:

  1. The changes in the code at line 191 will always throw exception in the above mentioned scenario. It will become highly non-interoperable in the BACnet network.
  2. Null is checked for TV pair, but what happened to setting NULL at Schedule-Default and Value field of TV pair is other datatype say Real(110). In this condition also exception will be thrown.

Regards, Kishore

kishorevenki commented 2 months ago

In the ScheduleObjectTest,java in the line 80: Can you please change the existing code to the below and test? new DailySchedule(new SequenceOf<>(new TimeValue(new Time(17, 0, 0, 0), new Real(11)), new TimeValue(new Time(8, 0, 0, 0), new Real(10))))

MertCingoz commented 2 months ago

12.24.7 Weekly_Schedule ..... If the Weekly_Schedule property is written with a schedule item containing a datatype not supported by this instance of the Schedule object (e.g., the List_Of_Object_Property_References property cannot be configured to reference a property of the unsupported datatype), the device may return a Result(-) response, specifying an 'Error Class' of PROPERTY and an 'Error Code' of DATATYPE_NOT_SUPPORTED.

12.24.8 Exception_Schedule .... If the Exception_Schedule property is written with a schedule item containing a datatype not supported by this instance of the Schedule object (e.g., the List_Of_Object_Property_References property cannot be configured to reference a property of the unsupported datatype), the device may return a Result(-) response, specifying an 'Error Class' of PROPERTY and an 'Error Code' of DATATYPE_NOT_SUPPORTED.

12.24.9 Schedule_Default This property holds a default value to be used for the Present_Value property when no other scheduled value is in effect (see Clause 12.24.4). This may be any primitive datatype. If the Schedule_Default property is written with a value containing a datatype not supported by this instance of the Schedule object (e.g., the List_Of_Object_Property_References property cannot be configured to reference a property of the unsupported datatype), the device may return a Result(-) response, specifying an 'Error Class' of PROPERTY and an 'Error Code' of DATATYPE_NOT_SUPPORTED.

@kishorevenki I am not BACNet expert here but by looking the spec, 3 properties stating that "cannot be configured" and "unsupported datatype".

I think you are looking for schedule templates, which don't seem to be supported by design. You might need to use WritePropertyMultiple for 4 properties in order to change the data type. These changes should then be validated.

I don't see any problem if you want to reuse time portion of the TimeValue pairs and try to reuse schedules as a template to create/update schedules with some workaround.

In the ScheduleObjectTest,java in the line 80: Can you please change the existing code to the below and test? new DailySchedule(new SequenceOf<>(new TimeValue(new Time(17, 0, 0, 0), new Real(11)), new TimeValue(new Time(8, 0, 0, 0), new Real(10))))

When TimeValue pairs for the day not in chronological order, looks like next calculation is not correct. It is expecting TV pairs in chronological order. I think this is misconfiguration if it is valid one. If not then there should be another validation to check order of TV pairs for each day.

  1. Null is checked for TV pair, but what happened to setting NULL at Schedule-Default and Value field of TV pair is other datatype say Real(110). In this condition also exception will be thrown.

https://github.com/MangoAutomation/BACnet4J/pull/97#discussion_r1655336889

kishorevenki commented 2 months ago
  1. I am not looking for Schedule Template at the outset.
  2. I am also referring to change those parameters using either WP or WPM request. But here only I am highlighting that upon receiving such requests, the validation function would throw exception and will never allow to write. You can test by creating Sch1 for AV and later to change to BV1. I doubt with this current it can be possible.
  3. If possible, then no issue. Otherwise it will look like creating template ..
kishorevenki commented 2 months ago

The standard does not mandate for TV pair should be chronological order. But checking of chrnological order has to be done by the controller. As mostly BACnet4J is used as client, checking such condition never arise. But the code that you are modifying is for BACnet4J as controller. Hence all such conditions are to be checked and validated.