hirotakaster / MQTT

MQTT for Photon, Spark Core
Other
217 stars 118 forks source link

Could you clarify this please? #55

Closed ScruffR closed 7 years ago

ScruffR commented 7 years ago

Your samples do feature this callback

void callback(char* topic, byte* payload, unsigned int length) {
    char p[length + 1];
    memcpy(p, payload, length);
    p[length] = NULL;
    String message(p);

    if (message.equals("RED"))
        RGB.color(255, 0, 0);
    else if (message.equals("GREEN"))
        RGB.color(0, 255, 0);
    else if (message.equals("BLUE"))
        RGB.color(0, 0, 255);
    else
        RGB.color(255, 255, 255);
    delay(1000);
}

Could you clarify why char p[] is created, populated and terminated but never used? Also in order to keep the system stable for a long time String should be avoided as it may lead to heap fragmentation. If I had the choice, I'd keep p[] and drop String message and do the string compare like this

    if (!strcmp(p, "RED"))
        RGB.color(255, 0, 0);
    else if (!strcmp(p, "GREEN"))
        RGB.color(0, 255, 0);
    else if (!strcmp(p, "BLUE"))
        RGB.color(0, 0, 255);
    else
        RGB.color(255, 255, 255);
hirotakaster commented 7 years ago

Hi @ScruffR your means, https://docs.particle.io/reference/firmware/photon/#string-class is occur heap fragmentation bad class?

ScruffR commented 7 years ago

Yes, that's true but not documented - unfortunately. The thing is that the actual string content is stored on the heap in 16 byte chuncks to start with. Once the string outgrows that chunck it may need to be relocated to a new place of 32byte, leaving the 16 byte on the previous spot virtually unusable and the same will happen when 32 gets too small, then 64 and so on. When this happens multiple times with multiple strings all over a project, your device might run out of big enough chuncks for other objects causing unexpected troubles where SOS panic is the least problematic as it causes a cleaning reset. But in less intrusive cases the device will just stall. Hence String should be used only sparingly and/or in short lived applications (e.g. regular reset or deep sleep)

hirotakaster commented 7 years ago

okay, I understand String class problem I will update this sample source. But it's very sorry that the String class will cause of SOS panic & reset trouble.

hirotakaster commented 7 years ago

Hi @ScruffR , I update all sample and upload to the Particle libraries version 0.4.14, I checked on Particle Dev. But it's still private state because of can't use on Particle WebIDE maybe based #51 , spark/particle-dev-libraries#26 problem. This problem will clear?, after that I will publish.

ScruffR commented 7 years ago

Web IDE should not have a problem with that issue. It's a Dev & CLI issue. What problem do you encounter with Web IDE?

hirotakaster commented 7 years ago

Hi @ScruffR , I can check on WebIDE thank you. I could compile same source code(this github) on Previous WebIDE/Particle Dev a little old days but can't compile on WebIDE. Then I change the all directory on the Particle Dev and published, compile is success!! I think problem is this library directory hierarchy on Particle Dev. Previous WebIDE is easy to collaborate with github...

I think it have to sync directory hierarchy this MQTT github and Particle Dev for maintenance. Thank you. close this issue.

ScruffR commented 7 years ago

I'm not sure if that's possible, but if you sent me a Web IDE snapshot (new feature) of your project I could have a look.

hirotakaster commented 7 years ago

Now latest version can compile and no problem. thank you.