Remove failure if version not present in the reported properties response #2162
Conversation
…es response. It will be present from IoT Hub, but absent from IoT Edge.
momuno
requested review from
CIPop,
danewalton-msft,
ericwolz,
ewertons and
jspaith
as code owners
…and only set version to EMPTY in that specific error case.
|
While not an API breaking change, this is a behavior breaking change: apps relying on finding a version if the parser passed will now find an AZ_SPAN_EMPTY instead. We already know that devices connecting to Edge will fail so I believe the risk is very small. @ahsonkhan @RickWinter FYI. |
I agree Cris and was thinking about this as well. I would say app "could" now find an AZ_SPAN_EMPTY instead. This all works if the Hub does not stop sending version in the topic. But if they stop, it could break apps that are expecting version and are only checking for AZ_OK. |
|
@CIPop @danewalton UT added. |
momuno
deleted the
dev/update-version-check-reported-properties-response
branch
| result = az_iot_message_properties_find( | ||
| &props, az_iot_hub_twin_version_prop, &out_response->version); | ||
| if (result == AZ_ERROR_ITEM_NOT_FOUND) | ||
| { | ||
| out_response->version = AZ_SPAN_EMPTY; | ||
| } | ||
| else | ||
| { | ||
| _az_RETURN_IF_FAILED(result); | ||
| } |
There was a problem hiding this comment.
Edit: I see now, if the received topic doesn't have a version "query parameter" then we still want to succeed. Gotcha.
Are we sure this is what we want? We are changing AZ_ERROR_ITEM_NOT_FOUND return value to AZ_OK on line 195 now.
Do we need to update the docs here:
azure-sdk-for-c/sdk/inc/azure/iot/az_iot_hub_client.h
Lines 625 to 632 in cc7beaf
There was a problem hiding this comment.
I am curious to know how did we discover this?
And if we would consider this a bug fix, that we want to highlight as something relevant to the end customer, then maybe add a changelog entry.
There was a problem hiding this comment.
We were notified via a colleague on Azure RTOS who had a customer issue. Through debugging, it was found that IoT Edge did not supply the $version in the reported properties response while IoT Hub did. This was confirmed by IoT Edge.
Can you please remind me where I create the changelog entry? Thanks!
I don't think the documentation needs to be updated. We do not currently specify what is considered a valid topic within that description, only that a topic which is valid will pass with out_response populated. This is still true.
There was a problem hiding this comment.
Can you please remind me where I create the changelog entry? Thanks!
Here:
https://github.com/Azure/azure-sdk-for-c/blob/main/CHANGELOG.md#bugs-fixed
Remove the failure if version is not present in the reported properties response.
It will be present from IoT Hub, but absent from IoT Edge.