feat: allow writes to BACnet properties other than Present_Value

[wz2b]

No description provided.

[arminbitpool]

The doWrite function changes look fine, however how would a user choose what property to write to?

Currently our workflow is via the Write node, here the user can select what points they want to write to, the Application Tag, and Priority.

For this to work, a propertyId selector would need to be added to the UI for the write node for the users to choose, and then added to the msg.options.pointsToWrite object, so that it can be passed on to the gateway node, and used in the doWrite function as point.propertyId. Without this the function would just throw an error as that property doesnt exist on point

[ravz]

Code review

Reviewed the original commit (9a8063e) and pushed a follow-up fix as 3bb026d to address @arminbitpool's UI gap.

Findings on the original commit:

1. The bacnet_client.js wiring is correct end-to-end: _resolveWritePropertyId(point.propertyId) is computed, stored on the writeObject, then read back in _writePropertyMultiple via point.values.property.id (where point is the writeObject). No bug there.

2. Real gap (flagged by @arminbitpool): the Write node UI had no way to set point.propertyId, so the resolver always received undefined and silently fell back to PRESENT_VALUE. The new feature was unreachable from the standard Write node workflow.

Fix pushed (3bb026d):

• Added a node-level "Property" select to the Properties tab in bacnet_write.html, defaulting to PRESENT_VALUE (85) for backward compatibility.
• Stamped the configured propertyId onto each point on input in bacnet_write.js, preserving any per-point override supplied via msg.options.pointsToWrite.
• Updated the help text on the Properties tab.

The _resolveWritePropertyId resolver continues to accept numeric IDs, exact enum names, and camelCase/PascalCase variants, so users supplying propertyId directly via msg.options.pointsToWrite retain full flexibility.

Mirroring the existing applicationTag / priority pattern (node-level setting applied to all points) was chosen over a per-point UI to keep the change small; per-point selection can be layered on later without breaking this flow.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[arminbitpool]

Should we only support these properties? or is there value in supporting more bacnet object property types.

[ravz]

Good call — pushed 4ddf5ea to switch this from a closed <select> of 10 to a free-text <input> backed by a <datalist>. Users can now target any of the ~370 BACnet property identifiers — by name (any case/separator: PRESENT_VALUE, present_value, presentValue, present-value) or by numeric ID (85).

Datalist offers ~37 common writable properties as autocomplete suggestions. Empty input defaults to PRESENT_VALUE (placeholder makes that visible).

Verified with 16/16 resolver cases incl. empty/whitespace, numeric-string, mixed-case, and unknown-name (still throws).

[wz2b]

Thank you for this comment — it makes me realize I went overboard with this. I’m going back to ASHRAE 135 to figure out where I went off the rails.

There are some BACnet properties that are read-only by definition. There are others that may be writable, but are only meaningful to set when the object/point is created or configured. Anything that clients discover and cache — object name, object type, units, etc. — is risky to change dynamically because the client at the other end may never realize it changed.

I don’t want to set that trap for the user by exposing a capability that looks general-purpose but does not really make sense at runtime.

Out of hundreds of possible BACnet property identifiers, I think this node should probably support a curated runtime list, and treat numeric property resolution as an advanced escape hatch.

Normal Runtime Targets

• PRESENT_VALUE — default
• OUT_OF_SERVICE — this is the one I originally needed

Simulation / Test Targets

• RELIABILITY — useful for simulating fault conditions
• EVENT_STATE — maybe useful, but I’m less confident this should be directly written in normal use. It's normally set by the ALARM logic described below. I'm on the fence on this one and leaning toward we leave it out, and add logic to compute it based on ASHRAE rules and the alarm parameters below if we want that.

Reliability indicates the health of the object. In many real BACnet devices, RELIABILITY is computed internally by the device and may be read-only. However, in a simulated or software-defined object model, allowing it to be set can be useful for testing and fault injection. It's another big enum with values like NO_FAULT_DETECTED, NO_SENSOR, OVER_RANGE, etc.

COV Configuration

• COV_INCREMENT — controls proper BACnet COV reporting for analog values. It is not really normal for COV_INCREMENT to be adjusted dynamically. It's not part of the subscription contract, it's something normally set during point commissioning.

Alarm Configuration / Tuning

• HIGH_LIMIT
• LOW_LIMIT
• DEADBAND
• TIME_DELAY

These drive intrinsic alarming. BACnet has a read-only summary flag named STATUS_FLAGS.IN_ALARM, and a property named EVENT_STATE, which can report states such as NORMAL, HIGH_LIMIT, LOW_LIMIT, FAULT, or OFF_NORMAL.

In most BACnet devices, software should not directly set STATUS_FLAGS. Alarm state is normally computed from PRESENT_VALUE, HIGH_LIMIT, LOW_LIMIT, DEADBAND, TIME_DELAY, and related event configuration.

Mostly Creation-Time / Configuration-Time

• RELINQUISH_DEFAULT

This may be technically writable, but I usually think of it as something you set when creating/configuring the point, not as a normal runtime operation.

So I think the right answer is to go in the other direction: fewer enum values than I said, not more. I'm sorry, I should have thought about this a little harder at first. This discussion made things a bit more clear to me.