Azure / iot-central-firmware

Azure IoT Device Samples ✨ 📟 🔌 🔋💡 ✨
Other
128 stars 96 forks source link

[ESP32] deviceTwinGetStateCallback triggers LoadProhibitedError when device receive settings from IoT Central #63

Open alwint3r opened 5 years ago

alwint3r commented 5 years ago

I am trying the ESP32 firmware and I noticed a bug that triggered LoadProhibitedError which then reboot the ESP32 whenever the device receives setting from Azure IoT Central.

Here's the payload of the setting

{
  "desired": {
    "toggle_led": {
      "value": false
    },
    "$version": 5
  },
  "reported": {
    "toggle_led": {
      "value": 0,
      "statusCode": 200,
      "status": "completed",
      "desiredVersion": 5
    },
    "$version": 5
  }
}

The code that triggers LoadProhibitedError is https://github.com/Azure/iot-central-firmware/blob/a71a1bf5407cc9b8e98870afae75481db621c542/ESP32/esp-azure/main/iotc/iotc.cpp#L419

See this code https://github.com/Azure/iot-central-firmware/blob/a71a1bf5407cc9b8e98870afae75481db621c542/ESP32/esp-azure/main/iotc/iotc.cpp#L408-L417

desiredVersion, version, desiredValue, and value are initially null and will always be null in my case because the value of these keys is not a string.

The workaround for avoiding the LoadProhibitedError is adding a check to desiredVersion and version to line 419.

if (containsKey && desiredVersion && version && strcmp(desiredVersion, version) == 0) {

This workaround also can be applied to line 421 by checking desiredValue and value instead of desiredVersion and version.

} else if (containsKey && desiredvalue && value && strcmp(desiredValue, value) != 0){

Or we can change the type of desiredVersion and version to double and use getNumberByName instead of getStringByName, so we can change the line 419 to the following line:

if (containsKey && desiredVersion == version) {

But there is another issue with the type of desiredValue and value. The existing code only handles the case where the type of the value of these variables is a string. The value could be boolean, string, or number.

obastemur commented 5 years ago

Surely the ESP32 sample here has an expected scenario. I’ll be more than happy to make it suitable for your case. Would you like to send a PR?

alwint3r commented 5 years ago

I don't mind creating a PR once I figure out how to make it work in my case and another case in general.