core-wg / yang-cbor

Internet-Draft for CBOR representation of YANG data
5 stars 10 forks source link

yang-cbor: issue in bits example (6.7) #143

Closed abierman closed 2 years ago

abierman commented 2 years ago

The example for bits says bit 'critical' is bit 3 but it is bit 2

OLD:

The following example shows the encoding of an 'alarm-state' leaf representation node instance with the 'critical' (position 3),

NEW:

The following example shows the encoding of an 'alarm-state' leaf representation node instance with the 'critical' (position 2),

It is encoded corrected as position 2 in the string 0401

Extra text, not part of ticket:

IMO bits processing is very complicated, just to handle a corner-case that is very rarely used, if ever. The positions will almost always be sequential, starting from 0. Fortunately, pruning middle zero bytes is optional so only 1 byte string is really needed.

(Also finding processing for decimal64 is extreme and the encoding is actually larger than the string most of the time but not relevant to this ticket. The example in 6.3 shows a string that would take 5 bytes to encode vs. an array that takes 6 bytes.)

cabo commented 2 years ago

Indeed. This bug was introduced in 21b92b9 when trying to make the example in the section more useful. Fortunately, as you say, it has a very simple fix.

cabo commented 2 years ago

Unfortunately the bits processing needs to handle what's possible in 7950, and this does require some complexity for the general case which will be almost never be used for the sane cases.

Decimal64 simply was modeled after CBOR's decimal floating point tag (4); we didn't see a reason to change this. Maybe this could have used the schema-informed nature of YANG, leaving out the decimal exponent to take this from the schema. I don't think we want to change this now.