elliotwoods / msgpack-arduino

Lightweight implementation of msgpack for arduino with streaming and COBS support
MIT License
16 stars 8 forks source link

Implicit functions question #5

Open Sod-Almighty opened 5 years ago

Sod-Almighty commented 5 years ago

Hi, and thanks for making this library available.

I notice that writeMapSize(Stream&, const uint8_t) checks if the value exceeds 1 << 4, and if not, encodes it as a map4 value.

However, I note that the other writeMapSize overloads (uint16_t and uint32_t) don't perform this check. Why is this? Surely if the value can fit in a map4, then it should be encoded as such? Wouldn't that be more space-efficient?

elliotwoods commented 5 years ago

Interesting point. Thanks for continuing to use this code following your previous frustrations.

In my application - application size became the limiting factor since I was targeting ATMega328 (arduino nano/uno). I had to remove/add lines of code on each compile to get it to successfully upload to the device within the available program size. Therefore, I opted for compile-time checks of size where possible, and the pattern that you describe would need to be a runtime check (i.e. the code would need to be run on the microcontroller whenever the function was called to perform the size comparison).

An alternative might be to add a general compile time flag like MSGPACK_ENABLE_DYNAMIC_SIZE_CHECKS which added the extra lines of code in for people who want them, or alternatively MSGPACK_DISABLE_DYNAMIC_SIZE_CHECKS or MSGPACK_MINIMUM_PROGRAM_SIZE for applications where application size is more important.

stugol commented 5 years ago

Yes, I figured out how to use it with the ESP6266 WifiUDP class, instead of trying to assemble a string beforehand. As you said, a string buffer would probably eat too much memory in any case.

You make a valid point regarding program size. I think compile-time flags make sense. In some cases, the size of the data payload may be important. For example, there is a hard limit on the size of a UDP packet, and it's pretty damn small.