fix: add day of week to BACnet protocol binding's Date and DateTime types #381
Conversation
…ypes Without wildcards, day of week would not be necessary, that's why ISO8601 doesn't support day of week in calendar dates. Because BACnet supports wildcards for the dates, we need to represent day of week. This change takes the day of week representation of ISO8601's week dates and extends the calendar date format with it.
✅ Deploy Preview for wot-binding-templates ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@egekorkan @ektrah @mjkoster here a small change, please review. |
There was a problem hiding this comment.
I've tried to come up with a few examples and how I'd interpret them:
"*-*-01"-- the set of dates containing the first day of every month"2024-*-01"-- the set of dates containing the first day of every month in 2024"*-*-*(4)"-- the set of dates containing every Thursday of every month"*-10-*(4)"-- the set of dates containing every Thursday in October every year"*-10-29(4)"-- the set of dates containing the 29th day of October every year, if it is a Thursday"2024-10-29(4)"-- the single-element set consisting of the 29th day of October in the year 2024"2024-10-29(7)"-- the empty set
Is that the correct interpretation?
Since the wildcards are not part of ISO8601, should they maybe be explained in a bit more detail? (perhaps in a separate pull request)
Generally speaking, would it make sense to re-use something like iCalendar recurrence rules or crontab syntax to express recurrence like these, instead of inventing a new syntax?
Otherwise this looks good to me.
|
Thanks for the comments @ektrah Your examples are correct. I can explain the wildcards a bit more, ultimately BACnet standard is the main source here, the mappings only reflect what BACnet can represent in date/time formats. Basically an asterisk means it can be any year/month/day/day-of-week, and that's it. Does that need further explanation? I checked crontab and iCalendar in detail:
|
egekorkan
added
Editorial
bacnet
|
If the discussion above is finalized and the changes are ok with @mjkoster , it can be merged but we will check in today's call as all the approvals are from the same organization |
|
I think the important insight here is that we are not describing a date (or date and time), but a recurrence rule. That is, the string compactly describes a set of dates (or dates and times); and this is the case even if no wildcards are present. Counterintuitively, strings with an invalid date (or date and time) are not actually invalid, since they simply describe the empty set. I think this would be very helpful to point out in the document. Too bad the crontab syntax does not support years, otherwise it would have been nice to reuse parts of the syntax instead of defining a new one. I agree with you that iCalendar is too complex for our purposes. |
|
@ektrah ok your point makes sense. I will explain this generally for the date / time types and add some examples. Then I hope we can merge. |
|
Reviewed in the call: We are fine with the changes. We can merge it once the textual improvements are done. |
|
@egekorkan @ektrah I did the textual changes now. I realized I hadn't added the odd and even wildcards for month and day of month, so now they are also in. Please have a last look before merging. |
Co-authored-by: ektrah <[email protected]>
Co-authored-by: ektrah <[email protected]>
|
@egekorkan I think I was able to incorporate all findings. Please merge when you get a chance. |
Without wildcards, day of week would not be necessary, that's why ISO8601 doesn't support day of week in calendar dates.
Because BACnet supports wildcards for the dates, we need to represent day of week. This change takes the day of week representation of ISO8601's week dates and extends the ISO8601's calendar date format with it.