feat(server): BACnet command prioritization (Priority_Array / Relinquish_Default)#43
Open
ravz wants to merge 2 commits into
Open
feat(server): BACnet command prioritization (Priority_Array / Relinquish_Default)#43ravz wants to merge 2 commits into
ravz wants to merge 2 commits into
Conversation
…jects Implements Priority_Array + Relinquish_Default arbitration of Present_Value for server-side Analog_Value and Binary_Value objects. Closes the gap noted in issue #40 where writes simply overwrote PV regardless of priority. Behavior: - Each commandable object now stores a 16-slot Priority_Array. Each slot is either NULL or a commanded {value, type} pair. - Each commandable object now exposes Relinquish_Default; reads return it when all 16 slots are NULL. - Writes to Present_Value honor the WriteProperty `priority` field, defaulting to 16 when absent. Writing NULL relinquishes that slot. - Direct writes to a single Priority_Array slot (via property array index 1..16) set or relinquish that slot. Full-array overwrites are rejected. - Writes to Relinquish_Default update the fallback (NULL is rejected). - Effective Present_Value is recomputed after every prioritized write so read paths need no changes. - Object-level properties (Out_Of_Service, Status_Flags, Reliability, etc.) remain object-level; non-commandable properties fall through to the existing modifyObject path. Backward compatibility: - addObject(name, primitive) still accepts a primitive payload; it seeds Relinquish_Default so reads return the same value as before until a write arrives. Server-sourced refreshes (re-calling addObject with the same name) update Relinquish_Default rather than Present_Value, so externally-commanded priorities continue to win until relinquished. - Objects loaded from on-disk cache that pre-date this change are migrated on startup: their dummy Priority_Array is replaced with a real 16-slot array, Relinquish_Default is seeded from the stored Present_Value, and Property_List is extended to advertise the priority properties. - Binary_Value Present_Value continues to use BACnet application tag BOOLEAN (the prior representation in this file), not ENUMERATED.
There was a problem hiding this comment.
Pull request overview
Implements BACnet command prioritization for server-side commandable objects (Analog_Value and Binary_Value) by introducing Priority_Array / Relinquish_Default arbitration and routing eligible writes through the new priority-aware path.
Changes:
- Add prioritized write handling for
Present_Value,Priority_Array[i], andRelinquish_Defaulton AV/BV objects. - Add migration logic for cached legacy AV/BV objects to ensure they have 16-slot
Priority_ArrayandRelinquish_Default. - Update
addObjectbehavior so server-sourced updates adjustRelinquish_Default(baseline) rather than directly overwritingPresent_Valuefor commandable objects.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- writeProperty handler: return after errorResponse so an UNKNOWN_OBJECT error is no longer followed by a duplicate Ack/emit when modifyObject fails (Copilot review #1). - handlePrioritizedWrite: throws now carry errorClass/errorCode, mapped to specific BACnet errors. PA writes without a 1..16 array index now surface as INVALID_ARRAY_INDEX; NULL writes to RELINQUISH_DEFAULT now surface as VALUE_OUT_OF_RANGE. Catch in writeProperty handler reads these and falls back to PROPERTY/WRITE_ACCESS_DENIED for unknown errors (Copilot review #2). - _ensureCommandableShape: BV's RD seed is now `false` (matching the BOOLEAN encoding used elsewhere in this file) instead of numeric 0 when PV is missing/corrupt (Copilot review #3). - _ensureCommandableShape: PROPERTY_LIST is now constructed from the object's own keys when absent on a legacy cached object, so migrated objects advertise their full property set instead of a [PA, RD] stub (Copilot review #4). - addObject: commandable objects now have PV refreshed via arbitration immediately after creation, so a payload.value vs payload.relinquishDefault mismatch can't leave PV pointing at the raw payload while PA is empty (Copilot review #5). Validated with 19/19 smoke-test assertions covering the new behaviors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #40.
What
Adds BACnet command prioritization to server-side commandable objects (
Analog_ValueandBinary_Value). Writes toPresent_Valuenow honor thepriorityfield per BACnet spec, withPriority_ArrayandRelinquish_Defaultarbitrating the effectivePresent_Value.CharacterString_Valueis not commandable per BACnet 12.x and is unchanged.Behavior
Present_Valuewith priority NPriority_Array[N]Present_Valuewith no priorityPresent_Valueat priority NPriority_Arraywith array index 1..16Relinquish_DefaultPresent_ValueRelinquish_DefaultPriority_ArrayRelinquish_DefaultOut_Of_Service,Status_Flags,ReliabilityBackward compatibility
addObject(name, primitive)(the existing Node-RED gateway publish path) seedsRelinquish_Defaultfrom the primitive, so a read with no active commands returns the same value as before.addObjectwith the same name updatesRelinquish_Default(the server-sourced baseline) rather than overwritingPresent_Valuedirectly. Externally-commanded priority slots continue to win until relinquished.Priority_Arraybecomes a real 16-slot array,Relinquish_Defaultis seeded from the storedPresent_Value, andProperty_Listis extended.Binary_ValuePV continues to use BACnet application tagBOOLEAN(the existing representation in this file), notENUMERATED.Implementation
All changes in
bacnet_server.js:_isCommandable,_appTagFor,_emptyPriorityArray,_arbitratePresentValue,_findObject,_refreshPresentValue,_ensureCommandableShape.handlePrioritizedWriteinvoked from thewritePropertyevent handler before the existingmodifyObjectfallback.addObjectfor AV/BV now creates the 16-slot PA, seeds RD, and registers both inPROPERTY_LIST._ensureCommandableShape.Testing
The project has no test infrastructure, so I exercised the arbitration logic in a throwaway smoke test (20/20 assertions): empty PA → RD; priority slot ordering (5 > 8 > 16); NULL relinquish cascade; direct PA[i] write/relinquish; full-array PA write rejected; NULL RD write rejected; non-commandable types fall through; legacy AV migration (PA promoted, RD seeded, PROPERTY_LIST extended).
Test plan