- Dec 16, 2013View Source
Some ideas and suggestions:
Regarding the use of "CodeOctet":
I think this can be resolved in the "9.5.2 Variables" section. "Code block" is repeatedly used with no reference as to what that means at this point in the text. Also, it seems like a single "code block" is not so much decoded octet by octet as it is processed, since most of the decoding seems to happen at the block boundaries rather than within the block. Finally, exclusively referring to "decoded" (multiple places in the variables section) leaves one wondering if there is any "encoding" required at all.
Consider something like the following for "CodeOctet" (and possibly similar changes elsewhere in the section):
"Used by devices that support COBS-encoded frames to count down the number of octets remaining to be processed in the current block of data within the frame."
What I am trying to get across here is that we are dealing with part of the frame (a subset) and to tie this into 9.10.3 where it refers to "Process a code octet" in several places.
Regarding "CRC-32K register C31-C0":
There needs to be a reference that exactly matches a register in the example. Would this be "crcValue"? If so, something like the following would be more enlightening:
"In operation, the CRC32K register "crcValue" bits C31-C0 are initialized to all ones."
What I am trying to do here is to tie the text name (CRC32K) to a variable name used in the example and provide some hint as to what C32 (etc.) means. The "C31-C0" part could be deleted since I don't think it adds much clarity since a register can have bits that can be initialized to all ones. This would then read as:
"In operation, the CRC32K register "crcValue" bits are initialized to all ones."
"See clause 19.X for the preferred approach for determining…"
to something like
"See clause 19.X for an approach for determining…"
Consider deleting the last two words of the first sentence so it ends "…as well as opening up future possibilities." I don't think deleting the whole sentence is a good idea since it is the heart of the rationale (i.e. why do we want to make this change).
I would also like to see consistency in terminology for the uninitiated. Use "CCITT-16 CRC" or "CRC-16-CCITT" or whatever is chosen everywhere. Someone reading this for the first time may not realize these are the same thing.
Thanks for your comments. Se my responses inline...
On Fri, Dec 13, 2013 at 5:21 PM, Clifford H Copass <Clifford.H.Copass@...> wrote:
I have been reading through Add-135-2012an-PPR2-draft-rc4.docx and found some places that confuse me. I am checking to see if I am missing something significant or if there are some editorial corrections that can be made before public review.
The term "CodeOctet" is giving me some problems. In some places it sounds like a data octet and in some places it sounds like an index. Is this where the omitted "zeros" are added back in by counting down the block size? If so, that could be clearer.
The fundamental definition is given on page 20:
"A COBS code block consists of a single code octet, followed by zero or more data octets. The number of data octets is represented by the code octet."
So a COBS-encoded field consists one or more code blocks, each or which begins with
a code octet. Without more detail, I'm not sure which sections of the text are causing
On page 25, I don't see what "CRC-32K register C31-C0" in the text has to do with the example code.
is similar to the descriptions of the header and data CRC examples in Annex G.
Would you just like to see "C31-C0" removed?
On page 5, "See clause 19.X for the preferred approach…" seems too strong a statement.
What text would you propose?
On page 3, in the Rationale, the first sentence seems incomplete. Also in the drawing, "CCITT-16 CRC" doesn't match the text which uses "CRC-16-CCITT".
Would you propose to delete the first sentence? It was perhaps more timely three years
ago when 115,200 baud had just been approved. Although the proposal doesn't explicitly
forbid the use of large frames with low baud rates, vendors will hopefully realize this is a
There are many variations on the name "CRC-16-CCITT" but they all refer to the same
polynomial. "CRC-16-CCITT" and "CRC-32K" are the names used in the Wikipedia
article on CRC polynomials.
Please respond with your thoughts on the points above.
[Attachment(s) from Kerry Lynn included below]
I have uploaded the third revision of the Add-135-2012an PPR2 draft up to the files
area and have attached it here.
It contains all the edits we discussed on last weeks call as well as resolutions to
several good comments from John Hartman.
This version contains a sample COBS-encoded frame (see X.4) but I did not have
time to do a flowchart for COBS-decoding. We have pseudo-code (9.10.3) and
example code (X.3) so hopefully that will be sufficient.
I also reversed spur-of-the-moment decision that we made on last weeks call,
namely to define the range of Frame Types 32 - 63 as COBS-encoded. I feel
there are several good reasons for setting the range back to 32 - 127:
a) We started PPR1 with the thoroughly MSTP-WG discussed range 8 - 127.
b) That range was reduced to 32 - 127 just in case we ever want to define
more "legacy" frames
c) The rationale for reducing the COBS-encoded range still further was "in
case a new technology comes forward". I doubt that will ever occur.
d) Adding a third range, e.g. "Undefined", to "COBS-encoded" and "Non-encoded"
would seriously complicate the document at this point.
e) The only current use of this range in the proposal is for the purposes of
validating header fields
f) If we really think some new format may come out later, then we can deal with
that possibility by allocating Frame Types from 34 on. This will permit later
definition of a new range after COBS-encoded.
If this decision or lack of a flowchart is a show-stopper, then we should discuss it
at the next SSPC.