Fixed out-of-service writability to be consistent with present-value in objects using Write_Enabled flag (#921)

src/bacnet/basic/object/bi.c

@@ -282,7 +282,7 @@ BACNET_BINARY_PV Binary_Input_Present_Value(uint32_t object_instance)
 /**
  * @brief For a given object instance-number, checks the present-value for COV
  * @param  pObject - specific object with valid data
- * @param  value - floating point analog value
+ * @param  value - binary value
  */
 static void Binary_Input_Present_Value_COV_Detect(
     struct object_data *pObject, BACNET_BINARY_PV value)
@@ -294,6 +294,21 @@ static void Binary_Input_Present_Value_COV_Detect(
     }
 }
 
+/**
+ * @brief For a given object instance-number, checks the out-of-service for COV
+ * @param  pObject - specific object with valid data
+ * @param  value - out-of-service value
+ */
+static void
+Binary_Input_Out_Of_Service_COV_Detect(struct object_data *pObject, bool value)
+{
+    if (pObject) {
+        if (pObject->Out_Of_Service != value) {
+            pObject->Change_Of_Value = true;
+        }
+    }
+}
+
 /**
  * @brief For a given object instance-number, returns the out-of-service
  * property value
@@ -326,10 +341,8 @@ void Binary_Input_Out_Of_Service_Set(uint32_t object_instance, bool value)
 
     pObject = Binary_Input_Object(object_instance);
     if (pObject) {
-        if (pObject->Out_Of_Service != value) {
+        Binary_Input_Out_Of_Service_COV_Detect(pObject, value);
-            pObject->Out_Of_Service = value;
+        pObject->Out_Of_Service = value;
-            pObject->Change_Of_Value = true;
-        }
     }
 
     return;
@@ -520,7 +533,7 @@ bool Binary_Input_Present_Value_Set(
  * For a given object instance-number, sets the present-value
  *
  * @param  object_instance - object-instance number of the object
- * @param  value - floating point analog value
+ * @param  value - binary present-value
  * @param  error_class - the BACnet error class
  * @param  error_code - BACnet Error code
  *
@@ -570,6 +583,43 @@ static bool Binary_Input_Present_Value_Write(
     return status;
 }
 
+/**
+ * For a given object instance-number, sets the out-of-service flag
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - boolean out-of-service value
+ * @param  error_class - the BACnet error class
+ * @param  error_code - BACnet Error code
+ *
+ * @return  true if value is set, or false if not set or error occurred
+ */
+static bool Binary_Input_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = Binary_Input_Object(object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            Binary_Input_Out_Of_Service_COV_Detect(pObject, value);
+            pObject->Out_Of_Service = value;
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * @brief Get the object name
  * @param  object_instance - object-instance number of the object
@@ -1081,8 +1131,9 @@ bool Binary_Input_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                Binary_Input_Out_Of_Service_Set(
+                status = Binary_Input_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
         case PROP_POLARITY:

src/bacnet/basic/object/bitstring_value.c

@@ -262,6 +262,21 @@ bool BitString_Value_Out_Of_Service(uint32_t object_instance)
     return value;
 }
 
+/**
+ * @brief For a given object instance-number, checks the out-of-service for COV
+ * @param  pObject - specific object with valid data
+ * @param  value - out-of-service value
+ */
+static void BitString_Value_Out_Of_Service_COV_Detect(
+    struct object_data *pObject, bool value)
+{
+    if (pObject) {
+        if (pObject->Out_Of_Service != value) {
+            pObject->Change_Of_Value = true;
+        }
+    }
+}
+
 /**
  * @brief For a given object instance-number, set the out of service value.
  * @param  object_instance - object-instance number of the object
@@ -273,15 +288,51 @@ void BitString_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
 
     pObject = BitString_Value_Object(object_instance);
     if (pObject) {
-        if (pObject->Out_Of_Service != value) {
+        BitString_Value_Out_Of_Service_COV_Detect(pObject, value);
-            pObject->Change_Of_Value = true;
-        }
         pObject->Out_Of_Service = value;
     }
 
     return;
 }
 
+/**
+ * For a given object instance-number, sets the out of service flag
+ * from WriteProperty service
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - new value
+ * @param  error_class - BACnet error class
+ * @param  error_code - BACnet error code
+ *
+ * @return  true if value is set, or false if not set or error occurred
+ */
+static bool BitString_Value_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = BitString_Value_Object(object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            BitString_Value_Out_Of_Service_COV_Detect(pObject, value);
+            pObject->Out_Of_Service = value;
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * @brief For a given object instance-number, read the reliability value.
  * @param  object_instance - object-instance number of the object
@@ -659,8 +710,9 @@ bool BitString_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                BitString_Value_Out_Of_Service_Set(
+                status = BitString_Value_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
         default:

src/bacnet/basic/object/blo.c

@@ -47,7 +47,6 @@ struct object_data {
     bool Out_Of_Service : 1;
     bool Blink_Warn_Enable : 1;
     bool Egress_Active : 1;
-    bool Changed : 1;
     bool Polarity : 1;
 };
 /* Key List for storing the object data sorted by instance number  */
@@ -472,7 +471,6 @@ static void Present_Value_Relinquish_Handler(uint32_t object_instance)
             value = Priority_Array_Value(pObject, current_priority);
         }
         if (pObject->Feedback_Value != value) {
-            pObject->Changed = true;
             if ((!pObject->Out_Of_Service) &&
                 (Binary_Lighting_Output_Write_Value_Callback)) {
                 Binary_Lighting_Output_Write_Value_Callback(

src/bacnet/basic/object/bv.c

@@ -288,9 +288,9 @@ void Binary_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
     pObject = Binary_Value_Object(object_instance);
     if (pObject) {
         if (pObject->Out_Of_Service != value) {
-            pObject->Out_Of_Service = value;
             pObject->Change_Of_Value = true;
         }
+        pObject->Out_Of_Service = value;
     }
 
     return;
@@ -525,6 +525,42 @@ static bool Binary_Value_Present_Value_Write(
     return status;
 }
 
+/**
+ * For a given object instance-number, sets the out-of-service flag if writable
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - binary value
+ * @param  error_class - the BACnet error class
+ * @param  error_code - BACnet Error code
+ *
+ * @return  true if flag is set, false if errors occurred
+ */
+static bool Binary_Value_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = Binary_Value_Object(object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            Binary_Value_Out_Of_Service_Set(object_instance, value);
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * @brief Get the object name
  * @param  object_instance - object-instance number of the object
@@ -1004,8 +1040,9 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                Binary_Value_Out_Of_Service_Set(
+                status = Binary_Value_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
 #if defined(INTRINSIC_REPORTING) && (BINARY_VALUE_INTRINSIC_REPORTING)

src/bacnet/basic/object/ms-input.c

@@ -317,7 +317,7 @@ uint32_t Multistate_Input_Present_Value(uint32_t object_instance)
 /**
  * @brief For a given object instance-number, checks the present-value for COV
  * @param  pObject - specific object with valid data
- * @param  value - floating point analog value
+ * @param  value - multistate value
  */
 static void Multistate_Input_Present_Value_COV_Detect(
     struct object_data *pObject, uint32_t value)
@@ -359,7 +359,7 @@ bool Multistate_Input_Present_Value_Set(
  * For a given object instance-number, sets the present-value
  *
  * @param  object_instance - object-instance number of the object
- * @param  value - floating point analog value
+ * @param  value - state value 1..N
  * @param  error_class - the BACnet error class
  * @param  error_code - BACnet Error code
  *
@@ -372,12 +372,14 @@ static bool Multistate_Input_Present_Value_Write(
     BACNET_ERROR_CODE *error_code)
 {
     bool status = false;
-    struct object_data *pObject;
     uint32_t old_value = 1;
+    unsigned max_states;
+    struct object_data *pObject;
 
     pObject = Multistate_Input_Object(object_instance);
     if (pObject) {
-        if (value <= UINT32_MAX) {
+        max_states = state_name_count(pObject->State_Text);
+        if ((value >= 1) && (value <= max_states)) {
             if (pObject->Write_Enabled) {
                 old_value = pObject->Present_Value;
                 Multistate_Input_Present_Value_COV_Detect(pObject, value);
@@ -439,13 +441,51 @@ void Multistate_Input_Out_Of_Service_Set(uint32_t object_instance, bool value)
 
     pObject = Multistate_Input_Object(object_instance);
     if (pObject) {
+        if (pObject->Out_Of_Service != value) {
+            pObject->Change_Of_Value = true;
+        }
         pObject->Out_Of_Service = value;
-        pObject->Change_Of_Value = true;
     }
 
     return;
 }
 
+/**
+ * For a given object instance-number, sets the out-of-service state
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - out-of-service state
+ * @param  error_class - the BACnet error class
+ * @param  error_code - BACnet Error code
+ *
+ * @return  true if value is set, false if error occurred
+ */
+static bool Multistate_Input_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = Multistate_Input_Object(object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            Multistate_Input_Out_Of_Service_Set(object_instance, value);
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * @brief For a given object instance-number, loads the object-name into
  *  a characterstring. Note that the object name must be unique
@@ -831,8 +871,9 @@ bool Multistate_Input_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                Multistate_Input_Out_Of_Service_Set(
+                status = Multistate_Input_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
         default:

src/bacnet/basic/object/msv.c

@@ -317,7 +317,7 @@ uint32_t Multistate_Value_Present_Value(uint32_t object_instance)
 /**
  * @brief For a given object instance-number, checks the present-value for COV
  * @param  pObject - specific object with valid data
- * @param  value - floating point analog value
+ * @param  value - multistate value
  */
 static void Multistate_Value_Present_Value_COV_Detect(
     struct object_data *pObject, uint32_t value)
@@ -359,7 +359,7 @@ bool Multistate_Value_Present_Value_Set(
  * For a given object instance-number, sets the present-value
  *
  * @param  object_instance - object-instance number of the object
- * @param  value - floating point analog value
+ * @param  value - multistate value
  * @param  error_class - the BACnet error class
  * @param  error_code - BACnet Error code
  *
@@ -374,10 +374,12 @@ static bool Multistate_Value_Present_Value_Write(
     bool status = false;
     struct object_data *pObject;
     uint32_t old_value = 1;
+    unsigned max_states = 0;
 
     pObject = Multistate_Value_Object(object_instance);
     if (pObject) {
-        if (value <= UINT32_MAX) {
+        max_states = state_name_count(pObject->State_Text);
+        if ((value >= 1) && (value <= max_states)) {
             if (pObject->Write_Enabled) {
                 old_value = pObject->Present_Value;
                 Multistate_Value_Present_Value_COV_Detect(pObject, value);
@@ -439,13 +441,51 @@ void Multistate_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
 
     pObject = Multistate_Value_Object(object_instance);
     if (pObject) {
+        if (pObject->Out_Of_Service != value) {
+            pObject->Change_Of_Value = true;
+        }
         pObject->Out_Of_Service = value;
-        pObject->Change_Of_Value = true;
     }
 
     return;
 }
 
+/**
+ * For a given object instance-number, sets the out-of-service state
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - out-of-service state
+ * @param  error_class - the BACnet error class
+ * @param  error_code - BACnet Error code
+ *
+ * @return  true if value is set, false if not or error occurred
+ */
+static bool Multistate_Value_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = Multistate_Value_Object(object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            Multistate_Value_Out_Of_Service_Set(object_instance, value);
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * @brief For a given object instance-number, loads the object-name into
  *  a characterstring. Note that the object name must be unique
@@ -833,8 +873,9 @@ bool Multistate_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                Multistate_Value_Out_Of_Service_Set(
+                status = Multistate_Value_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
         default:

src/bacnet/basic/object/time_value.c

@@ -171,7 +171,7 @@ bool Time_Value_Present_Value(uint32_t object_instance, BACNET_TIME *value)
 /**
  * @brief For a given object instance-number, checks the present-value for COV
  * @param  pObject - specific object with valid data
- * @param  value - floating point analog value
+ * @param  value - time value
  */
 static void Time_Value_Present_Value_COV_Detect(
     struct object_data *pObject, const BACNET_TIME *value)
@@ -240,7 +240,13 @@ static bool Time_Value_Present_Value_Write(
             datetime_copy_time(&old_value, &pObject->Present_Value);
             Time_Value_Present_Value_COV_Detect(pObject, value);
             datetime_copy_time(&pObject->Present_Value, value);
-            if (Time_Value_Write_Present_Value_Callback) {
+            if (pObject->Out_Of_Service) {
+                /* The physical point that the object represents
+                    is not in service. This means that changes to the
+                    Present_Value property are decoupled from the
+                    physical point when the value of Out_Of_Service
+                    is true. */
+            } else if (Time_Value_Write_Present_Value_Callback) {
                 Time_Value_Write_Present_Value_Callback(
                     object_instance, &old_value, value);
             }
@@ -292,6 +298,9 @@ bool Time_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
 
     pObject = Keylist_Data(Object_List, object_instance);
     if (pObject) {
+        if (pObject->Out_Of_Service != value) {
+            pObject->Change_Of_Value = true;
+        }
         pObject->Out_Of_Service = value;
         status = true;
     }
@@ -299,6 +308,42 @@ bool Time_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
     return status;
 }
 
+/**
+ * For a given object instance-number, sets the out-of-service flag
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  value - indicator of 'Out-of-service'
+ * @param  error_class - the BACnet error class
+ * @param  error_code - BACnet Error code
+ *
+ * @return  true if value is set, or false if not or error occurred
+ */
+static bool Time_Value_Out_Of_Service_Write(
+    uint32_t object_instance,
+    bool value,
+    BACNET_ERROR_CLASS *error_class,
+    BACNET_ERROR_CODE *error_code)
+{
+    bool status = false;
+    struct object_data *pObject;
+
+    pObject = Keylist_Data(Object_List, object_instance);
+    if (pObject) {
+        if (pObject->Write_Enabled) {
+            Time_Value_Out_Of_Service_Set(object_instance, value);
+            status = true;
+        } else {
+            *error_class = ERROR_CLASS_PROPERTY;
+            *error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
+        }
+    } else {
+        *error_class = ERROR_CLASS_OBJECT;
+        *error_code = ERROR_CODE_UNKNOWN_OBJECT;
+    }
+
+    return status;
+}
+
 /**
  * For a given object instance-number, loads the object-name into
  * a characterstring. Note that the object name must be unique
@@ -619,8 +664,9 @@ bool Time_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             status = write_property_type_valid(
                 wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN);
             if (status) {
-                Time_Value_Out_Of_Service_Set(
+                status = Time_Value_Out_Of_Service_Write(
-                    wp_data->object_instance, value.type.Boolean);
+                    wp_data->object_instance, value.type.Boolean,
+                    &wp_data->error_class, &wp_data->error_code);
             }
             break;
         default: