thingsboard-client-sdk icon indicating copy to clipboard operation
thingsboard-client-sdk copied to clipboard

Sketches don't run with ArduinoJSON ver. 7.0.0

Open PauloMtz10 opened this issue 2 years ago • 12 comments

PauloMtz10 avatar Jan 04 '24 18:01 PauloMtz10

This library doesn't support ArduinoJson v7.X.X, use version v6.X.X instead. Version 6.21.4 is recommended.

This is the case because the version was only released yesterday 7.0.0 and includes major API changes.

For now the API has implemented stubs for old features that allow to still use the old names but the features under the hood have already been changed, so it could be that some methods in the library do not work anymore. Meaning it would require internal library changes to work correctly again.


Sadly I also don't think that support for this version of ArduinoJson will be implemented any time soon. This is mainly because I don't think the advantages of new features, which have not been implemented yet, out way the new cost. Read more of the changes here in the blog post.

Especially for this library the version upgrade would be a downgrade in performance and memory usage, which is not acceptable in my opinion.

First of all I do agree that the internal allocation and a lot of cool features, which ArduinoJson provided were sadly hard to use and sometimes misunderstood. I learned that part myself the hard way, because I completely misunderstood how the library worked in the background.

After reading the very good documentation tough. I have to say that I love the StaticJsonDocument and the zero-copy feature for both deserialize() and serialize().

Below I've listed the places where above mentioned features are required or cause massive improvements and what their removal in v7.X.X would mean for the library:

v6.X.X

deserialize(), non copy: Does not copy when reading char*, which is the type we receive from the underlying MQTT client. Meaning we do not have to copy the received buffer into the JsonDocument.

serialize(), non copy: Does not copy when reading const char*, which is the type we use a lot internally for sent values and all the time for keys. Most of the time the aforementioned strings are even in flash memory (PROGMEM). This allows to keep the JsonDocument much smaller and leaner and only have its size be 16 * the amount of sent key value pairs. Additionally this allows to predict the size of the JsonDocument and therefore allocate it on the stack instead of the heap.

StaticJsonDocument and DynamicJsonDocument: Currently the user can already use DynamicJsonDocument to allocate on the heap if that is wished with the THINGSBOARD_ENABLE_DYNAMIC macro. This also removes the need for the template arguments. Whereas the default of StaticJsonDocument is mainly managed by the library and needs zero to no direct interaction by the user. The only thing required currently that has to do with ArduinoJson with v0.12.2 is to configure the maximum amount of key-value pairs that will be received or sent. Which is used to create the StaticJsonDocument that hold the response or sent user data. If not enough of those key value pairs are configured then the library will print an error signalling this issue as well as possible fixes to the user directly. Furthermore, the internally allocated StaticJsonDocument are in the worst case 144 bytes on the stack (Provision_Request method). In all other cases beside MaxFieldsAmount configured by the user we allocate 16 or 32 bytes at most.

Additionally even in dynamic mode those small allocation which really do not need to be on the heap are still on the stack, because we do always know the amount of key-value pair the ThingsBoard API excepts for certain calls and can therefore easily hardcore it into the library directly.

v7.X.X

deserialize(), copy: The complete internal MQTT buffer would be copied into the JsonDocument every time, which depending on your configured buffer size could be rather big. Additionally that JsonDocument is relatively short lived but does call a method that can allocate an additional JsonDocument or even allocate memory on the heap itself if it is a very big JsonDocument, which increases the danger for heap fragmentation.

serialize(), copy: All strings passed into aJsonDocument even const char*, would be now copied meaning all internal library functions would be much bigger and allocate memory on the heap.

JsonDocument: Only allows for heap allocation anymore and allocates 1KB per default. Meaning every single usage of JsonDocument in this library, which is nearly every method, would allocate 1KB on the heap at the minimum.


Because those features have been removed and replaced with copy and heap allocation. I will wait for newer versions which might bring other features which decrease this problematic, but for now as you can hopefully see the disadvantages far out way the advantages of upgrading to v7.X.X, in my opinion.

Especially if the main advantage currently is user friendliness. This because, it is already done by this library, because most of the internals with JSON are handled by the library itself and do not require interaction by the user.

MathewHDYT avatar Jan 04 '24 19:01 MathewHDYT

my Sketch compiles with ArduinoJson v7.0.0 I get many warnings eg: "Static JsonDocument is deprecated use JsonDocument".. but it compiles.

ChrSchultz avatar Jan 04 '24 21:01 ChrSchultz

This will only be for a while that it even compiles, because at the start of v7.X.X it still keeps stubs for the old components.

But in the background all components have been changed to reflect the changes mentioned above and what @PauloMtz10 might have meant is that the library does not work correctly anymore even if it compiles for certain features with ArduinoJson v7.X.X

MathewHDYT avatar Jan 05 '24 07:01 MathewHDYT

MathewHDYT hi, whats the timing on the release of the version to resolve this #186 , tks.

TPCQitek avatar Feb 05 '24 04:02 TPCQitek

@TPCQitek I hope soon, sadly I don't know either because the examples have to be adjusted accordingly.

MathewHDYT avatar Feb 05 '24 05:02 MathewHDYT

Hi, ok thanks, mostly my system is ok on the 6.21.4 , but the control widgets are not updating the initial state , IE ThingsBoard Switch etc particularly, it defaults to ON, was expecting that may be resolved in the 7.xx I had been very busy on other things, and was looking today, I wanted to get the switch and RPC send buttons sorted,

thanks.

Regards:

Terence Curtis

CONTACTS: SKYPE: terencepcurtis

Mobile: Australia +61 412 262 646


On Mon, 5 Feb 2024 at 16:37, MathewHDYT @.***> wrote:

@TPCQitek https://github.com/TPCQitek I hope soon, sadly I don't know either because the examples have to be adjusted accordingly.

— Reply to this email directly, view it on GitHub https://github.com/thingsboard/thingsboard-client-sdk/issues/186#issuecomment-1926266680, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANLX6TYPTBDHTCJ2JRBUJGTYSBV2PAVCNFSM6AAAAABBNKRBSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRWGI3DMNRYGA . You are receiving this because you were mentioned.Message ID: @.***>

TPCQitek avatar Feb 05 '24 06:02 TPCQitek

@TPCQitek That is because of another issue. As a quick fix until the PR is merged, simply move the StaticJsonDocument into global scope an call clear at the top of the method.

See #167 for more info.

MathewHDYT avatar Feb 05 '24 06:02 MathewHDYT

Mathew, hi, apologies not sure what you mean ??

simply move the StaticJsonDocument into global scope an call clear at the top of the method.

Regards:

Terence Curtis

CONTACTS: SKYPE: terencepcurtis

Mobile: Australia +61 412 262 646


On Mon, 5 Feb 2024 at 17:50, MathewHDYT @.***> wrote:

That is because of another issue. As a quick fix until the PR is merged, simply move the StaticJsonDocument into global scope an call clear at the top of the method.

— Reply to this email directly, view it on GitHub https://github.com/thingsboard/thingsboard-client-sdk/issues/186#issuecomment-1926331196, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANLX6T2T56EOGVQQ4SLTMGDYSB6LVAVCNFSM6AAAAABBNKRBSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRWGMZTCMJZGY . You are receiving this because you were mentioned.Message ID: @.***>

TPCQitek avatar Feb 06 '24 08:02 TPCQitek

@TPCQitek See this https://github.com/thingsboard/thingsboard-client-sdk/issues/167#issuecomment-1855768052.

MathewHDYT avatar Feb 06 '24 18:02 MathewHDYT

MathewHDYT

I did the workaround, as best I see it,

the RPC Widget Button Send is OK and the processPower ON / OFF is ok, but the Widget Control Switch is not working, not reading the current state and see below. the [TB] was unable to serialize, data and not Send ON / OFF

the code follows, can you suggest anything? thanks.

19:48:04.914 -> RPC_Response processPower 19:48:04.914 -> #### Exit NULL processPower #### 19:48:04.914 -> Power_State-0 19:48:04.914 -> Null RPC_Response variant: 19:48:04.914 -> {"RPC_response":false} 19:48:04.914 -> [TB] Unable to serialize json data

Using the RPC Send Button :

19:59:54.128 -> RPC_Response processPower 19:59:54.128 -> RPC_Response Power ON 19:59:54.128 -> void Button_Power_ON 19:59:54.128 -> hardware_type_temp: 12 19:59:54.128 -> last_state: 0 19:59:54.128 -> Normal Fan Start. 19:59:54.128 -> void Button_FAN_Speed2 19:59:54.128 -> Hardware 11 & 12 19:59:54.128 -> Set FAN Speed 12v mid 19:59:54.128 -> SetFanMid: 130 19:59:54.128 -> Cold_start: 0 19:59:54.128 -> Re_start: 1 19:59:55.627 -> Fan2-Lamps Re_start 19:59:55.627 -> void Button_Lamp1_ON 19:59:55.627 -> LAMP1_ON 19:59:57.158 -> void Button_Lamp2_ON 19:59:57.158 -> LAMP2_ON 19:59:57.158 -> EXIT type 11 & 12 19:59:57.158 -> EXIT Button_FAN_Speed2 19:59:57.158 -> RPC_Response variant: 19:59:57.158 -> {"RPC_response":true} 19:59:57.158 -> [TB] Unable to serialize json data

using LIB 6.21.5

// Define the capacity of the JSON document globally const size_t JSON_CAPACITY_POWER = JSON_OBJECT_SIZE(1);

// Create a global JSON document for power RPC response StaticJsonDocument<JSON_CAPACITY_POWER> doc_power;

RPC_Response processPower(const RPC_Data &data) { Serial.println("RPC_Response processPower"); Stream_T2 = millis(); heartbeat = millis();

const size_t JSON_CAPACITY = JSON_OBJECT_SIZE(1); StaticJsonDocument<JSON_CAPACITY> doc; doc.clear();

if (data[RPC_POWER_METHOD].isNull()) { // get the current state Serial.println(" #### Exit NULL processPower ####"); if (Power_State == 0) { Serial.println(" Power_State-0"); doc[RPC_RESPONSE_KEY] = false;
} else { Serial.println(" Power_State-1"); doc[RPC_RESPONSE_KEY] = true; } String strData; serializeJson(doc, strData); DynamicJsonDocument dynamicDoc(512); JsonVariant variant = dynamicDoc.to<JsonVariant>(); deserializeJson(dynamicDoc, strData); Serial.println("Null RPC_Response variant:"); serializeJson(variant, Serial); Serial.println(); return RPC_Response(variant); }

bool Power_temp = data[RPC_POWER_METHOD]; if (Power_temp) { Serial.println("RPC_Response Power ON"); Button_Power_ON(); doc[RPC_RESPONSE_KEY] = true; } else { Serial.println("RPC_Response Power OFF"); Button_Power_OFF(); doc[RPC_RESPONSE_KEY] = false; }

String strData; serializeJson(doc, strData); DynamicJsonDocument dynamicDoc(512); JsonVariant variant = dynamicDoc.to<JsonVariant>(); deserializeJson(dynamicDoc, strData);

Serial.println("RPC_Response variant:"); serializeJson(variant, Serial); Serial.println(); return RPC_Response(variant); }

TPCQitek avatar Feb 09 '24 09:02 TPCQitek

Mathew, Hello, apologies, been off on another project,

doing a review of the ThingsBoard and ArduinoJson version..

was that completed to support version 7? , my code is on ver 6.21.5

I had reverted to using the telmentarySend method,

also

the ThingsBoard Switch Widget (typically) no longer updated the current State when the Dashboard page opened..

I am assuming that the Dashboard version had changed? and did not work on the older ver 6.21.5

any update? or timing / options.

thanks in advance...

Regards:

Terence Curtis

CONTACTS: SKYPE: terencepcurtis

Mobile: Australia +61 412 262 646


On Tue, 6 Feb 2024 at 19:51, Terence Curtis @.***> wrote:

Mathew, hi, apologies not sure what you mean ??

simply move the StaticJsonDocument into global scope an call clear at the top of the method.

Regards:

Terence Curtis

CONTACTS: SKYPE: terencepcurtis

Mobile: Australia +61 412 262 646


On Mon, 5 Feb 2024 at 17:50, MathewHDYT @.***> wrote:

That is because of another issue. As a quick fix until the PR is merged, simply move the StaticJsonDocument into global scope an call clear at the top of the method.

— Reply to this email directly, view it on GitHub https://github.com/thingsboard/thingsboard-client-sdk/issues/186#issuecomment-1926331196, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANLX6T2T56EOGVQQ4SLTMGDYSB6LVAVCNFSM6AAAAABBNKRBSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRWGMZTCMJZGY . You are receiving this because you were mentioned.Message ID: @.***>

TPCQitek avatar Mar 27 '24 05:03 TPCQitek

You are still entering a JsonVaraint from a JsonDocument into the RPC_Response. This results in the data being deleted before it can be used resulting in never sending an answer for any RPC requests.

Simply move the JsonDocument used into global scope so it is not deleted after it has gotten a value. Additionaly if you do that call clear() on it before entering new values.

DynamicJsonDocument dynamicDoc(512);

RPC_Response processPower(const RPC_Data &data) {
    dynamicDoc.clear();
    ...
}

See this for more information https://github.com/thingsboard/thingsboard-client-sdk/issues/167#issuecomment-1855768052.

MathewHDYT avatar Mar 27 '24 18:03 MathewHDYT