new!: full Schedule/Calendar (scheduling) support#213
Merged
Conversation
…_Schedule
Weekly_Schedule could not be written at all ("I cannot encode this") and
decoded only into an opaque nested BacnetValue[] tree. Add the two spec
types (ASHRAE 135-2016 Clause 21) with symmetric encode/decode, dispatch
PROP_WEEKLY_SCHEDULE decoding onto BacnetDailySchedule (one value per
array element), and map the type in TagFromType so a decoded value can
be written straight back. Ported from the IPECON fork with hardening:
bounds-checked decode returning -1, and the generic list-encode fallback
now accepts any List<IEncode>, not just List<object>.
BACnetCalendarEntry modelled a whole Date_List as one List<object> bag and BacnetweekNDay ignored its week-of-month octet, so calendar data could neither be addressed per entry nor evaluated correctly. Replace them with the standard's shapes (ASHRAE 135-2016 Clause 21): a CHOICE class holding exactly one of Date/DateRange/WeekNDay, and a struct with month / week-of-month / day-of-week enums whose IsAFittingDate covers all week-of-month slices (1-9), odd/even months and Sunday=7 mapping. Date_List now decodes one BacnetCalendarEntry per BacnetValue under the dedicated CALENDAR_ENTRY tag, so a read list writes back unchanged. BacnetDate learns day 32 (last day of month), a DateTime constructor, and ToDateTime (renamed from toDateTime) returns the wildcard sentinel instead of DateTime.Now; a wildcarded BacnetDateRange boundary now leaves that side open, which makes the all-wildcard Effective_Period default evaluate as always in effect.
Exception_Schedule decoded into an opaque nested BacnetValue[] tree that could not be written back: re-encoding threw, or produced a PDU peers rejected with MISSING_REQUIRED_PARAMETER. Add the BACnetSpecialEvent type (ASHRAE 135-2016 Clause 21) covering both period choices - calendar-entry in all three forms and calendar-reference - plus the time-value list and event priority, dispatch PROP_EXCEPTION_SCHEDULE decoding onto it, and drop the now-unreachable partial context-tag hints for that property. Values read from a device write back byte-identical.
…dentifier typo The type could encode but not decode (the decoder was a loose ASN1 function), so it lacked the symmetry the other schedule types now have, and its public field was misspelled deviceIndentifier. Add the IDecode implementation and rename the field. The underlying decoder also read past the buffer when a reference ended exactly at the APDU boundary - the previously unused length parameter now guards both optional tags.
Weekly_Schedule, Exception_Schedule and Date_List values stored through Storage.Property previously fell into the default ToString() arm, which can never be read back. Store them as base64 of their ASN.1 encoding - the same convention octet strings already use - and decode them into the typed objects on load.
Exercises the full client/server path for the scheduling properties: a written Weekly_Schedule (including a midnight entry) reads back byte-identical, and Exception_Schedule / Date_List values read from a device can be written back unchanged.
Property writes could only fail as WriteAccessDenied/OutOfRange mapped to the DEVICE error class, and the write's array index was dropped before object dispatch. Add DuplicateEntry and InvalidDataType error codes with their wire mappings, report property-level failures under ERROR_CLASS_PROPERTY (135 Clause 18), and expose the array index to set2_ methods so array properties like Weekly_Schedule can support single-element writes.
Completes the migration started with the spec-shaped BacnetCalendarEntry: the class is public so it can be reused by interop tooling, and a Date_List write with a non-calendar-entry element now reports INVALID_DATA_TYPE instead of a generic out-of-range error.
…2.24 The 2015 Schedule object supported only a weekly schedule, evaluated it with a next-transition search instead of the standard algorithm, and commanded at the wrong property (PROP_PRIORITY, the notification-class id, instead of PROP_PRIORITY_FOR_WRITING). It now carries the full data model - Weekly_Schedule as BACnetARRAY[7] with single-day array writes, Exception_Schedule with every period choice and index-zero resize, Effective_Period as a real date range, a writable 1..16 Priority_For_Writing and computed Reliability/FAULT on datatype mismatches (12.24.13) - and evaluates Present_Value per 12.24.4 in ScheduleCalculation, a pure class shared with the unit tests: highest priority in-effect special event first (ties to the lower index), calendar references resolved against local Calendar objects and followed through their change notifications, then the weekly schedule, then Schedule_Default. Duplicate times are rejected as DUPLICATE_ENTRY, Present_Value writes are honoured (and dispatched) only while Out_Of_Service, and a missing remote recipient no longer aborts dispatching to the remaining referenced properties.
Covers Clause 12.24.4 on the pure calculation core, linked from the example project: latest-at-or-before selection on unordered lists, NULL relinquish to Schedule_Default, exception priority ordering with index tie-break and fall-through past candidates without a current value, calendar-reference resolution, the Effective_Period gate, Monday-first day indexing, and next-transition timing.
encode_bacnet_time treated DateTime(1,1,1) as the 'any time' wildcard, but that value is also exactly what every decoded midnight carries - reading a 00:00:00.00 time or timestamp and writing it back corrupted it to FF FF FF FF. Whole-day schedules are required to start at 00:00 (135-2016 Clause 12.24), which is how the collision surfaced. Midnight now encodes as 00:00:00.00 and a deliberate unspecified time - GetEventInformation timestamps of never-seen transitions, per the Annex F.1.6 vector - is expressed with the new ASN1.BACNET_TIME_WILDCARD marker. Decoders keep mapping a fully-wildcarded time to DateTime(1,1,1) for compatibility, so that direction remains lossy until the lossless 5.0 time types. BacnetTimeValue drops its octet-level workaround now that the shared encoder is safe.
…ps lossless The decoders mapped an unspecified time (FF FF FF FF) to DateTime(1,1,1), which the midnight fix left as the one remaining lossy direction: a wildcard read from a peer was indistinguishable from midnight and wrote back as 00:00:00.00. decode_bacnet_time now returns ASN1.BACNET_TIME_WILDCARD, the exact mirror of the encoder, so both midnight and the wildcard survive a read-modify-write unchanged. Combined date+time decodes (BACnetDateTime, datetime timestamps, ReadRange by-time, log records) go through a shared combine_date_and_time helper that keeps degrading an unspecified time component to 00:00 as before - a merged DateTime cannot carry it - and a spec-illegal wildcard inside a schedule BacnetTimeValue is still tolerated as midnight (12.24.7 requires specific times). Breaking for code that compared decoded times against DateTime.MinValue to detect the wildcard; MIGRATION.md shows the one-line change.
…ve_Period Partially-wildcarded times clamped to zero on decode and date patterns degraded to the minimum sentinel, so values like 11:22:**.** or 'last day of every odd month' could not be read and written back faithfully. Decoded TIME and DATE values now stay per-octet - the new BacnetTime struct and the existing BacnetDate - whenever a DateTime cannot represent the octets, and re-encode byte-identically under their usual application tags. Fully-specified values keep decoding as DateTime, the all-FF wildcards keep their established markers, and the date/time merging decodes still clamp, since a merged DateTime cannot carry an unspecified component. The XML device storage persists the struct forms as base64 next to the textual formats. Effective_Period rides the same idea: it decodes as one typed BacnetDateRange instead of two bare DateTimes, so the open boundaries of a Schedule's period survive a round-trip; writes accept both the typed range and the legacy two-date shape. This completes the IPECON DataUnit harvest for scheduling - BacnetTime was the last piece - and retires the wildcard items previously deferred to 5.0.
The date/time merging paths were the last places that dropped wildcard octets, and two of them broke outright once values could carry the per-octet structs: the ReadPropertyMultiple Date+Time merge cast them to DateTime, and Event_Time_Stamps elements decoded as bare DateTimes that threw on re-encode while losing which BACnetTimeStamp choice the device had used. Add the BACnetDateTime struct (ASHRAE 135-2016 Clause 21) and apply the established faithful-representation rule to the merges: a wildcarded Date+Time pair merges into it per-octet, Event_Time_Stamps elements decode as BacnetGenericTime with the choice preserved, and timestamps keep their original time octets in the new BacnetGenericTime.PartialTime alongside the best-effort clamped Time - echoing a partially-wildcarded event timestamp back, as an AcknowledgeAlarm does, now reproduces the wire bytes. Log-record stamps and service-internal times (TimeSynchronization, ReadRange by-time), where the standard requires specific values, keep clamping. The Wheather2 sample aliases its BaCSharp DateTime-value object over the new core struct name.
The scheduling work grew several parallel copies of the same ideas: application DATE/TIME tag headers hand-rolled at five encode sites, the time octet validity ranges in two places, the all-255 wildcard literals and the sentinel-to-struct conversions in three, one identical typed property dispatch block per schedule property, one identical typed list extraction loop per set2_ writer, and a copy-pasted decode/ re-encode assert pair in two test files. Each now has a single home: encode_application_date/time overloads for the struct forms (also used by BacnetDateRange/BacnetDateTime), BacnetTime owns its octet ranges, BacnetDate.Any/BacnetTime.Any and FromDateTime carry the wildcard/sentinel knowledge, DecodeTypedValue<T> serves the three property dispatches, BaCSharpObject.TryGetTypedValues<T> serves the four typed writers, and the tests share Support/ApplicationValue. No wire-level behavior changes; the golden vector and round-trip tests pin that.
…comments A comment audit found the schedule evaluation relied on List<T>.Sort for the equal-EventPriority tie-break while claiming it is stable - .NET's introsort is not, so the lowest-array-index rule (135-2016 Clause 12.24.8) only held by luck for small arrays. Order with the guaranteed-stable OrderBy instead, and pin it with four equal-priority events in the test. Also corrects two comments the later commits had made wrong: the exception-resize placeholder period is always-matching (harmless via its empty time-value list), not never-active, and Effective_Period network writes arrive as one decoded BacnetDateRange since the typed dispatch - the two-date branch serves in-process callers. The typed property dispatch now also documents that it keys on the property id alone, like its neighbours.
A test audit found ten round-trip tests asserting only len > 0 on decode, which would let a partial-consume regression pass unnoticed. All service and structure decodes of a complete well-formed buffer must consume it exactly, and now assert that.
A broad git add in the deduplication commit accidentally picked up a local debugging change that pinned the AnotherStorageImplementation transport to a specific host address. Restore the stock constructor; selecting an interface on multi-NIC machines is the runner's decision (see MIGRATION.md).
…e item data The ReadRange-ACK extraction took everything after the opening tag [5] minus one byte as the item data - correct only for the by-position layout. By-sequence and by-time acks carry a first-sequence-number [6] after the closing tag, so the returned range ended with those ack bytes and decoding the final trend-log record of a time-based read failed. Walk the item data to the matching closing tag instead, tracking the tag nesting depth (with the application-BOOLEAN value-in-tag quirk handled). Found by the container trend-log regression suite; the by-position path is byte-identical.
The two round-trip tests went through our own encoder, so a complementary encode/decode misunderstanding of the ack layout would have passed. Add a hand-derived by-time ack - header, two log records, closing tag and the trailing first-sequence-number - and assert the extracted range equals exactly the record bytes.
The repo's Annex F suites keep the 'golden vector' phrasing in the class documentation and name the tests after what they verify; the new schedule tests had leaked the technique into the method names and fields. Renames only.
ScheduleCalculation.ComputePresentValue documented one parameter, which makes the compiler flag the four undocumented ones (CS1573) - document them all. The dead-code comments in the non-stream AtomicReadFile branch started with '///*', accidentally forming XML comments attached to no element (CS1587, pre-existing but annotating the PR since the file is in the diff) - space them as plain comments.
…stop shadowing the core BacnetDateTime struct
…aluate the 33/34 day specials
gralin
force-pushed
the
new/schedule-support
branch
from
9473593 to
d850e48
Compare
# Conflicts: # CHANGELOG.md
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
1 participant
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.
Full BACnet scheduling support per ASHRAE 135-2016: Schedule and Calendar properties can now be read and written, with everything the standard's productions allow surviving a round-trip byte-identically.
Core serialization (Clauses 12.9, 12.24, 21)
BacnetTimeValue,BacnetDailySchedule,BacnetSpecialEvent(both period choices),BacnetTime,BacnetDateTime.BACnetCalendarEntry(a real CHOICE — one entry per value) andBacnetweekNDay→BacnetWeekNDay(month / week-of-month / day-of-week enums, with the week-of-month octet actually evaluated, all nine slices).Weekly_Schedule,Exception_ScheduleandDate_Listvalues carry dedicated application tags and typed objects (oneBacnetValueper array/list element), so a read list can be passed straight back toWritePropertyRequest— the exact flows from Schedule Object write #26 ("I cannot encode this") and ExceptionSchedule Property - Write #131 (opaque nestedBacnetValue[]that could not be written back).Effective_Perioddecodes as one typedBacnetDateRange;BacnetDeviceObjectPropertyReferencedecodes itself (and thedeviceIdentifiertypo is fixed); schedule values persist in the XML device storage.Lossless dates, times and timestamps
DateTime(1,1,1)encodes as 00:00:00.00 and the unspecified time has a dedicatedASN1.BACNET_TIME_WILDCARDmarker — both round-trip.11:22:**.**, "last day of every odd month") stay per-octet (BacnetTime/BacnetDate) instead of clamping; wildcarded Date+Time pairs merge intoBacnetDateTime; timestamps keep their original octets inBacnetGenericTime.PartialTime, so echoing an event timestamp back (AcknowledgeAlarm) reproduces the wire bytes.Event_Time_Stampselements (which also lost the BACnetTimeStamp choice).Example objects (AnotherStorageImplementation)
Schedulerewritten to Clause 12.24: full data model, single-day array writes, exception schedules with priorities and calendar references,Effective_Periodgating, DUPLICATE_ENTRY rejection, computed Reliability, and the 12.24.4 Present_Value evaluation in a pure, unit-testedScheduleCalculation(guaranteed-stable priority/index ordering). Also fixes the object commanding viaPROP_PRIORITY(86, the notification-class id) instead ofPROP_PRIORITY_FOR_WRITING(88).Calendarreworked to Clause 12.9 on the new CHOICE type, re-evaluating Present_Value at midnight and notifying schedules immediately on Date_List changes.ERROR_CLASS_PROPERTY, and the WriteProperty array index exposed to property writers.Validation
bacrp/bacwpdriving the C# schedule device through a BACnet router (including the ASCII schedule forms and a cleanduplicate-entryerror), and live runtime behavior (transitions landing on the target's priority array, NULL relinquish to Schedule_Default, calendar-driven exceptions, Out_Of_Service semantics). All 22 pass; the same scenario does not even compile against the published beta.2, since these types did not exist.Breaking changes
new!:/fix!:commits; see the Scheduling types, wildcard TIME and unspecified-value sections of MIGRATION.md (oldBACnetCalendarEntry/BacnetweekNDayremoved,Date_List/Effective_Period/Event_Time_Stampsdecode shapes,toDateTime→ToDateTime,deviceIndentifier→deviceIdentifier).Credits
The serialization design is ported (and hardened) from the IPECON fork — thanks to Andrej Kmetík for the original DataUnit implementations.
Closes #26
Closes #131