How do we keep Decoder lightweight?

[koenvervloesem]

I saw the discussion about Decoder optimizations in 1technophile/OpenMQTTGateway#1248 (comment).

Especially:

Unfortunately, the alternative options would require a major refactor and not worth it to support outdated devices IMO.

I do think eventually we'll come to a point where this is inevitable if we want to keep Theengs Decoder lightweight, as we'll keep piling decoders on it.

Do you know Kaitai Struct? It's a code generator to create decoders for binary data in various languages. I have used it to generate Python code to decode RuuviTag BLE and iBeacon advertisements, and it's quite neat, with YAML code to describe structure of the data.

Unfortunately it hasn't been ported yet to generate C code, but someone has been working on it.

Maybe this (or the code generation approach in general) is something to consider in the long run?

[1technophile]

I do think eventually we'll come to a point where this is inevitable if we want to keep Theengs Decoder lightweight, as we'll keep piling decoders on it.

Good to open this discussion. We can see it as a mid-term improvement, as we still have room to optimize the current architecture.

I was looking at some YAML parser but didn't dig much on how it could help:
https://github.com/jbeder/yaml-cpp

I would say that the main goal is to combine lightness and readiness regarding decoders definitions.

[DigiH]

Also, as previously discussed in #106, introducing a lookup table function would greatly reduce the current MS-CDP decoder and other future decoders benefiting from this.

[h2zero]

I guess this would depend on the definition of lightweight.

• Reduce code space use? = reduce decoder JSON strings/objects.
• Reduce RAM use? = break everything and use cjson or other alternatives.
• Best of all? = custom compilation of decoder JSON files into a binary object that can be linked so only the app input JSON is required.

Starting with the first is the easiest obviously, but limited. The second could be done in a way that the applications do not need to be changed but would bloat the app, so breaking the API to use alternative JSON libraries seems necessary. The third option, as interesting as it would be, is much too extreme.

YAML may have potential, I don't know how efficient the parsers are, but would still have the API breakage problem.

[DigiH]

with part of point 1. still in your reduce-and-optimize branch, only giving a modest saving (~1100 bytes - at the then Decoder stage) , as stated in

#99

I was also wondering how exactly the _props definitions work, assumedly mainly for auto-discovery, and why not pool them all together in one _common_props

With the current Decoder 0.6.2 and OMG 0.9.15 the difference is

0.6.2
RAM: [== ] 17.8% (used 58456 bytes from 327680 bytes)
Flash: [======= ] 73.9% (used 1452756 bytes from 1966080 bytes)

one single _common_props
https://github.com/DigiH/decoder/commits/properties-test
RAM: [== ] 17.8% (used 58448 bytes from 327680 bytes)
Flash: [======= ] 73.5% (used 1445764 bytes from 1966080 bytes)

so another smallish saving of ~6992 bytes, but still about half of what Decoder gained since the above mentioned test.

Depending on the feasibility for auto-discovery, this could be pooled together with the above mentioned reduce-and-optimize implementations.

[h2zero]

That's a decent savings for sure but it will likely break getTheengProperties and force the caller to provide a huge json document buffer for it.
Would be better find make a generator for this and remove the props all together.

[DigiH]

Since each decoder already has the props defined for decoding I just tough it redundant to define the unit and name 10 times (or so) for

"batt":{
         "unit":"%",
         "name":"battery"
      }

over and over again, similar with the other repeating property definitions.

Would be better find make a generator for this and remove the props all together.

Likely the better solution :) but I think we should take this saving opportunity - combined with your reduce-and-optimize; again, depending how this still allows for the correct auto-discovery of only the selected properties of each decoder device.

[1technophile]

Since each decoder already has the props defined for decoding I just tough it redundant to define the unit and name 10 times (or so) for

Agreed, I think we could define one key one time, and call it from the different property definitions, example in the decoder:
Lywsd03mmc_prop = temp + batt

And on a global properties dictionnary;

batt = "batt":{
         "unit":"%",
         "name":"battery"
      }

Not sure the best way to write this but you get the principle :-)

[1technophile]

Or if we follow the thought of Digih we replace;
Lywsd03mmc_prop = temp + batt

By looking at the decoder definition. The keys listed are the ones that the getTheengsProperties return by parsing the decoder definition and taking their detailled properties into the global properties dictionary.
So that we don't break getTheengsProperties.

[h2zero]

I got thinking today and perhaps there is a compromise here. If we changed the API to accept a char* for input and return value, which would point to a JSON string, we could use any JSON lib (or roll our own slim version). That combined with the ideas above could possibly reduce the lib size and RAM by a significant amount.

[DigiH]

Will have a look to see what might be the issue with the SwitchBot Curtain.

[DigiH]

Found a missed masking optimisation, but other than that I can't see anything obviously wrong with the JSON there.

https://github.com/theengs/decoder/pull/176/files

This roughly 70% win with cJSON really is something we should take, giving us the chance to get at least 3 times as many devices as currently into Decoder until we might end up in the current situation again 😉

[h2zero]

Will have a look to see what might be the issue with the SwitchBot Curtain.

I didn't see any issues with it either, just that cJSON wouldn't parse it, not sure why, wasn't really important for this quick test.

It's not so much about the number of devices, but the size of the biggest JSON, which is Model: RDL52832, cJSON size: 2784, ArduinoJSON size: 8124. This determines how much RAM we need to allocate for all the decoder JSON's. Adding more devices just consumes more flash/CPU cycles, which is the least of our issues at the moment.

Whether this is worth pursuing or not from here I can't say (big job) but it's nice to have the data.

I found also this https://arduinojson.org/v6/how-to/reduce-memory-usage/ as our issue is mainly with esp8266 should we store long string in flash?

I'll test that out and see what difference it makes.

[DigiH]

It's not so much about the number of devices, but the size of the biggest JSON

True ;) but then allowing for some decoders to be extended, whereas in the past we deliberately left out some - not so important - properties, which could now be included for completeness sake.

[h2zero]

Some more interesting discovery; after @1technophile mentioned this https://arduinojson.org/v6/how-to/reduce-memory-usage I decided to see what happens when using PROGMEM as a decorator for the json's.

With [env:nodemcuv2-ble] The results are very, very encouraging:

RAM:   [======    ]  56.0% (used 45852 bytes from 81920 bytes)
Flash: [======    ]  61.3% (used 640244 bytes from 1044464 bytes)