Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AD review 1 #19

Merged
merged 11 commits into from
Oct 7, 2023
Merged

AD review 1 #19

merged 11 commits into from
Oct 7, 2023

Conversation

cabo
Copy link
Contributor

@cabo cabo commented Oct 3, 2023

Thank you, Francesca!

@cabo cabo requested a review from henkbirkholz as a code owner October 3, 2023 17:57
@cabo cabo changed the title Ad review 1 AD review 1 Oct 3, 2023
@@ -91,7 +90,7 @@ IXDTF = '
' ; extracted from [IXDTF]; update as needed!


Duration = #6.1001(etime-detailed)
Duration = #6.1002(etime-detailed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -83,15 +83,31 @@ normative:
date: 2006-03-01
seriesinfo:
ISO: 80000-3
RFC8575: ptp-yang
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +88 to +93
target: https://standards.ieee.org/ieee/1588/4355/
title: >
1588-2008 — IEEE Standard for a Precision Clock
Synchronization Protocol for Networked Measurement and Control
Systems
IEEE Standard for a Precision Clock Synchronization Protocol for
Networked Measurement and Control Systems
seriesinfo:
IEEE: 1588-2008
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +99 to +110
IEEE1588-2019:
target: https://standards.ieee.org/ieee/1588/6825/
title: >
IEEE Standard for a Precision Clock Synchronization Protocol for
Networked Measurement and Control Systems
seriesinfo:
IEEE: 1588-2019
author:
org: IEEE
date: 2020-06
ann: Often called PTP v2.1, as it has been designed so it can be
used in way that is fully backwards compatible to IEEE1588-2008.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +142 to +153
ISO8601-2019:
display: 'ISO8601-1:2019'
target: https://www.iso.org/standard/70907.html
title: >
Date and time — Representations for information interchange —
Part 1: Basic rules
author:
- org: International Organization for Standardization
abbrev: ISO
seriesinfo:
ISO: '8601-1:2019'
date: 2019-02
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 474 to 481
Key -4 (ClockAccuracy) can be used to indicate the clock accuracy as
per {{RFC8575}} (which is based on Table 6 in Section 7.6.2.5 of
{{IEEE1588-2008}}; additional values have been defined in Table 5 in
Section 7.6.2.6 of {{IEEE1588-2019}}).
It is defined as a one-byte unsigned integer as that is the range defined there.
The range between 32 and 47 is a slightly distorted logarithmic scale from
25 ns to 1 s (see {{formula-accuracy-enum}}); the number 254 is the
25 ns to 1 s (extended to 23 to 47 for 1 ps to 1 s in {{IEEE1588-2019}})
— see {{formula-accuracy-enum}}; the number 254 is the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +466 to +468
{{RFC8575}} (which is based on Table 5 in Section 7.6.2.4 of
{{IEEE1588-2008}}; this has updated language as Table 4 in Section 7.6.2.5
of {{IEEE1588-2019}}).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +497 to +498
characteristic are defined in Section 7.6.3 of {{IEEE1588-2019}} and the
same section in {{IEEE1588-2008}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -615,7 +649,7 @@ durations are structurally identical to time values.


~~~ cddl
Duration = #6.1001(etime-detailed)
Duration = #6.1002(etime-detailed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +807 to +809
| 1 | Base Time value as in CBOR Tag 1 | {{-cbor}} \[RFCthis] |
| 4 | Base Time value as in CBOR Tag 4 | {{-cbor}} \[RFCthis] |
| 5 | Base Time value as in CBOR Tag 5 | {{-cbor}} \[RFCthis] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Key -4 (ClockAccuracy) can be used to indicate the clock accuracy as
per {{RFC8575}} (which is based on Table 6 in Section 7.6.2.5 of
{{IEEE1588-2008}}; additional values have been defined in Table 5 in
Section 7.6.2.6 of {{IEEE1588-2019}}).
It is defined as a one-byte unsigned integer as that is the range defined there.
The range between 32 and 47 is a slightly distorted logarithmic scale from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With there being a specification for the range from 23 to 47, is there even a point in referencing the old scale that starts at 32?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is worth saying (in particular since RFC 8575 is based on -2008), but maybe just as a parenthesis to to the 2019 version presented as the full range.
(Not saying, together with the fact that 23 could look like a typo for 32, will create confusion.)
See commits below...

cabo added 4 commits October 5, 2023 11:36
... as per discussion at CBOR interim meeting 2023-10-04, we do not
want to create new registry groups for tag parameters unless they are
for what could be called protocols of their own such as CWT.
@cabo cabo merged commit 51c8d7d into master Oct 7, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants