bblanchon/ArduinoJson

Add `JsonVariant::printTo(char[N])`

Opened this issue · 7 comments

This is with arduino json 7.2

inline void
jsUiManager::executeUiUpdates(const ArduinoJson::JsonDocument &doc) {
    auto& self = instance();
    for (ArduinoJson::JsonPairConst kv :
         doc.as<ArduinoJson::JsonObjectConst>()) {
        int id = atoi(kv.key().c_str());
        const char* value = kv.value().as<const char*>();   // <------ nullptr

        // double loop to avoid copying the string
        for (auto it = self.mComponents.begin(); it != self.mComponents.end();) {
            if (auto component = it->lock()) {
                ++it;
                if (component->id() == id) {
                    component->update(value);
                }
            } else {
                it = self.mComponents.erase(it);
            }
        }
    }
}
inline void
jsUiManager::executeUiUpdates(const ArduinoJson::JsonDocument &doc) {
    auto& self = instance();
    for (ArduinoJson::JsonPairConst kv :
         doc.as<ArduinoJson::JsonObjectConst>()) {
        int id = atoi(kv.key().c_str());
        std::string valueStr = kv.value().as<std::string>();  // <------ has expected value
        const char *value = valueStr.c_str();

        // double loop to avoid copying the string
        for (auto it = self.mComponents.begin(); it != self.mComponents.end();) {
            if (auto component = it->lock()) {
                ++it;
                if (component->id() == id) {
                    component->update(value);
                }
            } else {
                it = self.mComponents.erase(it);
            }
        }
    }
}

I would like to NOT have to create a bunch of memory strings in a tight loop. I'd like to just have JsonPairConst::value().printTo(charBuff);

This also fails to compile:

inline void
jsUiManager::executeUiUpdates(const ArduinoJson::JsonDocument &doc) {
    auto& self = instance();
    typedef char CharBuff[1024];
    CharBuff value = {0};
    for (ArduinoJson::JsonPairConst kv :
         doc.as<ArduinoJson::JsonObjectConst>()) {
        int id = atoi(kv.key().c_str());
        value = kv.value().as<CharBuff>();
        // double loop to avoid copying the string
        for (auto it = self.mComponents.begin(); it != self.mComponents.end();) {
            if (auto component = it->lock()) {
                ++it;
                if (component->id() == id) {
                    component->update(value);
                }
            } else {
                it = self.mComponents.erase(it);
            }
        }
    }
}

You should at least make as<const char*> static_assert so we don't get a nullptr exception during runtime.

Hi @zackees,

kv.value() is a regular JsonVariant (or JsonVariantConst), so you can call serializeJson() as usual:

char buffer[1024];
serializeJson(kv.value(), buffer);

I cannot exclude as<const char*>() with a static assert because there are many legitimate uses.

Please read the documentation.

Best regards,
Benoit

It would be nice to have a comment in the source code. I dont' see why serializing to a static sized char buffer is an issue.

I don't think something like as<char[64]>() is possible because the array will decay to a pointer if it's used as a return type.

I don't think something like as<char[64]>() is possible because the array will decay to a pointer if it's used as a return type.

Well then why not just inline a printTo function right into the JsonVariant?

This would be obvious.

Right now the solution is "read the documentation"

which is claimed to be 700 pages long.

This isn't even a corner case. Reading values out of a document in a tight loop is like the most mainstream use case imaginable.

This seems like a one liner. Do I need to fork and submit a PR to move this forward?

Users around the world would rejoice.

Adding printTo() with all the legitimate overloads to JsonArray, JsonArrayConst, JsonDocument, JsonObject, JsonObjectConst, JsonVariant, and JsonVariantConst, is not exactly a one-liner.
Besides, it would add more pages to the documentation 😉

What's wrong with calling serializeJson()?

Just do

'''
template<int N>
void printTo(char[N] out);
'''

You don't have to support all overloads. Just give us the common path.