Azure / azure-iot-arduino

Azure IoT library for the Arduino
Other
168 stars 95 forks source link

payLoad get from DeviceTwin in not a valid JSON #46

Closed yuwzho closed 7 years ago

yuwzho commented 7 years ago

I try to set a callback for device twin desired data, but when the callback method invoked, I received the payLoad is only a part of JSON. I think it should be the whole part of desired data, don't include any of the reported data

{
  "desired": {
    "$version": 3
  },
  "reported": {
    "$versi+øÊ-.î®…%ƒ10,"humë†Çò
f   3.20}iotëÇ2�
tameraw commented 7 years ago

@yuwzho - Can you please provide repro steps and the code you created? Thanks.

yuwzho commented 7 years ago

Hi @tameraw , pls follow these steps:

  1. (Necessary) Set a device desired message, such as "interval": 3000. It should has the properties as:

    desired:
    interval:  3000
    $metadata:
      $lastUpdated:        2017-04-06T08:10:52.2335433Z
      $lastUpdatedVersion: 22
      interval:
        $lastUpdated:        2017-04-06T08:10:52.2335433Z
        $lastUpdatedVersion: 22
    $version:  22
    reported:
    $metadata:
      $lastUpdated: 2017-02-17T02:19:02.4116343Z
    $version:  1
  2. Use a board (I use HUZZAH ESP8266 here) to run these code:

    IoTHubClient_LL_SetDeviceTwinCallback(iotHubClientHandle, twinCallback, NULL);
    
    void twinCallback(
      DEVICE_TWIN_UPDATE_STATE updateState,
      const unsigned char *payLoad,
      size_t size,
      void *userContextCallback)
    {
      char *temp = (char *)malloc(size + 1);
      strncpy(temp, payLoad, size);
      temp[size] = '\0';
      printf("%s", temp);
      free(temp);
    }
  3. Start to run the piece of code,

    1. at the first time the device get device twin message, it will get all properties (including desired and reported parts), but it is an invalid json.
    2. After the first time, when I update the desired properties, the device only receives desired part as a valid json
markrad commented 7 years ago

Hi @yuwzho,

I'll take a look at this and see if I can reproduce it.

Mark Radbourne MSFT

markrad commented 7 years ago

HI @yuwzho,

You didn't mention if you are using the serialization API, the standard API or the low level API. Can you let me know which you are using please?

Mark Radbourne MSFT

yuwzho commented 7 years ago

Hi @markrad

I didn't see there is any doc to metions the serialization/standard/low level API. But according to the method name IoTHubClient_LL_SetDeviceTwinCallback, I think I use a low level API.

BTW, it there a document to descripe these APIs you metioned?

markrad commented 7 years ago

Hi @yuwzho,

Not that I've found. I can't even find a device twin sample except for the serialization version and that didn't seem to work correctly. I've contacted the product group for anything they might have.

Yes you are correct, you are using the low level version of the API. I'll put something together and test it for myself.

Mark Radbourne MSFT

markrad commented 7 years ago

Hi @yuwzho,

So far I have been unable to reproduce your issue. Is your first post the bad JSON you are receiving? Looking at that, I would suspect that the buffer is being overwritten. Is your source code available to me?

Thanks.

Mark Radbourne MSFT

markrad commented 7 years ago

Hi @yuwzho,

Finally after testing for a significant period I was able to reproduce your issue. I'm not sure exactly why the buffer is corrupt since I've not seen this behaviour on other C platforms. I will need to do further investigation.

markrad commented 7 years ago

Hi @yuwzho,

After a day of revisiting the joys of printf debugging I have root caused this issue. I will need to consult with the product group to decide how they wish to address this but the quick fix is trivial. In the file _\<arduinoSDKRoot>\libraries\AzureIoTUtility\src\adapters\tlsioarduino.c find the #define for _RECEIVE_BUFFERSIZE and modify it from 128 to 256. The device twin MQTT packet is longer than 128 bytes and the code does not have any mechanism to cope with that. Obviously just setting the buffer length to 256 does not resolve the issue, it simply reduces the likelihood of occurrence. However, I would contend that the device twin messages have the potential to exceed a length of 256 also.

Mark Radbourne MSFT

markrad commented 7 years ago

Hi @yuwzho,

Oddly enough, when I modified the buffer size to 64 in order to try and create a more catastrophic error, the issue failed to reproduce. The device twin packet was successfully reconstructed from two or more separate packets. Not sure what to make of it but it seems to be a viable circumvention.

Mark Radbourne MSFT

markrad commented 7 years ago

Hi @yuwzho,

Finally I have managed to reproduce this error in a consistent manner. It happens because the layer that reads the socket does not drain the socket but leaves data waiting until the next call. If the client code sends a packet during that time then the receive buffer is corrupted. Now I have a good repro I will investigate what is required to fix it.

Mark Radbourne MSFT

yuwzho commented 7 years ago

@markrad thanks for your efforts. 👍

markrad commented 7 years ago

Hi @yuwzho,

A fix has been checked into master to address this.

Mark Radbourne MSFT