Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sketches don't run with ArduinoJSON ver. 7.0.0 #186

Closed
PauloMtz10 opened this issue Jan 4, 2024 · 12 comments · Fixed by #180
Closed

Sketches don't run with ArduinoJSON ver. 7.0.0 #186

PauloMtz10 opened this issue Jan 4, 2024 · 12 comments · Fixed by #180

Comments

@PauloMtz10
Copy link

No description provided.

@MathewHDYT
Copy link
Contributor

MathewHDYT commented Jan 4, 2024

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.

@ChrSchultz
Copy link
Contributor

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

@MathewHDYT
Copy link
Contributor

MathewHDYT commented Jan 5, 2024

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

@TPCQitek
Copy link

TPCQitek commented Feb 5, 2024

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

@MathewHDYT
Copy link
Contributor

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

@TPCQitek
Copy link

TPCQitek commented Feb 5, 2024 via email

@MathewHDYT
Copy link
Contributor

MathewHDYT commented Feb 5, 2024

@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.

@TPCQitek
Copy link

TPCQitek commented Feb 6, 2024 via email

@MathewHDYT
Copy link
Contributor

@TPCQitek See this #167 (comment).

@TPCQitek
Copy link

TPCQitek commented Feb 9, 2024

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();
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();
deserializeJson(dynamicDoc, strData);

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

@TPCQitek
Copy link

TPCQitek commented Mar 27, 2024 via email

@MathewHDYT
Copy link
Contributor

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 #167 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants