On Wed, 2015-07-15 at 14:41 -0700, Thiago Macieira wrote:
> On Wednesday 15 July 2015 13:14:35 Keane, Erich wrote:
> > Ive put together a BNF inspired document, attached to here!
> >
> > You'll note that it is actually a bit wasteful, which is a result of
> > trying to make it look as close to the JSON as possible without being
> > overly-wasteful. Even so, we are at 1/3 to 1/2 package size for the
> > same data.
>
> Thanks Erich, this is very good!
>
> Some comments:
>
> > PAYLOAD_REP_ITEM_VALUE = nullValue Cbor
> > NULL type: The value is a 'NULL' type as defined in the IoTivity
> > specification>
> > | intValue Cbor
> > | Int type: The value is an integer doubleValue
> > | Cbor Double
> > | Type
> > | booleanValue Cbor
> > | Boolean Type
> > | stringValue Cbor
> > | String
> > | PAYLOAD_REPRESENTATION A
> > | child-representation
> > | PAYLOAD_ARRAY An
> > | array of values, contains a subset of these
> > | values>
>
> > ARRAY_CONTENT_TYPE = OC_REP_PROP_INT (1)
> >
> > | OC_REP_PROP_DOUBLE (2)
> > | OC_REP_PROP_BOOL (3)
> > | OC_REP_PROP_STRING (4)
> >
> > OC_REP_PROP_OBJECT (5)
>
> I'd check if the (int64_t)value == value for the double case and encode as
> integer instead of double. That saves a lot on the receiver side.
>
> Or, better yet, I'd make sure that we don't use floating point, at all. Drop
> support for doubles. Small devices with no FPU would have to carry a huge FP
> emulation library in order to interpret them properly.
>
I considered this, however it causes a regression in functionality on
the C++ stack. I'd definitely consider seeing if we could find a way to
better encode the doubles, however I believe at the moment, the
performance/size increase over the JSON is enough to back-pat for the
moment.
> > -- Dimensions list, a zero in a place means that the dimension is not used.
> > So a 1D 5 element array is 5,0,0, and a 2D 3x5 array is: 3,5,0
>
> Instead of 0, you should use 1, which makes the size multiplication work
> without special casing. An 1D 5-element array is 5x1x1 = 5, whereas 5*0*0 = 0.
That makes a lot of sense. I definitely should have done that.
>
> I also don't understand why we need 3 dimensions, no more. Or why this array
> has a fixed type (are you checking or coercing the type?). This array feature
> seems like a overdesigned for the needs of the protocol. Just allow
> PAYLOAD_ARRAY to contain PAYLOAD_REP_ITEM_VALUE [PAYLOAD_REP_ITEM_VALUE, ...].
The 3 dimensions limit was inherited from C++. We could likely make it
higher. The 'nested array' requirement was put forth a while ago, which
is why we have dimensions in the first place.
>
> > Many of the string-keys could easily be replaced by integers (making the
> > maps INT->val rather than String->val), which would save a ton of space.
>
> Agreed. String-variant maps should only be used when expansion is necessary
> and the map is type-constrained.
>
> CBOR allows for heterogeneous keys, so I'd advise using integers for OIC-
> standardised types, but still allowing strings for vendor-specific extensions
>
> And here's a nifty trick. For example, for the following map entries:
>
> "pi" Cbor String (map tag)
> platformID Cbor String: Configured platform
> identifier
> "mnmn" Cbor String (map tag)
> ManufacturerName Cbor String: Configured name of the
> manufacturer of the platform
>
> Use instead
> #define OIC_PLATFORM_PID 0x7069
> #define OIC_PLATFORM_MANUFACTURER_NAME 0x6d6e6d6e
>
> Before, the keys would be encoded
> 62 70 69 string length 2 "pi"
> 64 6d 6e 6d 6e string length 4 "mnmn"
>
> Now they'd be encoded:
> 19 70 69
> 1a 6d 6e 6d 6e
>
> Or with a macro:
> #define ITVT_TO_NUM2(a, b) ((a) << 8) | (b)
> #define ITVT_TO_NUM4(a, b, c, d) ((a) << 24) | ((b) << 16) | ((c) << 8) |
> (d)
> #define OIC_PLATFORM_PID
> ITVT_TO_NUM2('p', 'i')
> #define OIC_PLATFORM_MANUFACTURER_NAME ITVT_TO_NUM4('m', 'n',
> 'm', 'n')
>
>
I was thinking something like:
#define OIC_PLATFORM_PID 0x1
#define OIC_PLATFORM_MANUFACTURER_NAME 0x2
Now the keys get encoded even smaller :)
