Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Iothub delivery notification #392

Merged

Conversation

villepalo
Copy link
Contributor

Here is what I was talking about in #391 ('Message Delivered'-notification from IotHub). This feature is mandatory for us, so I just implemented it. If you are interested in having this kind of feature, here it is. If this is not useful to you, feel free to say no.

@Azure Azure deleted a comment from msftclas Sep 27, 2017
@darobs darobs self-requested a review December 20, 2017 00:01
IoTHubMessage_Destroy(iotHubMessage);
ConstMap_Destroy(properties);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to return here (and in the section above) or make a best effort at sending the message to IoTHub?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. If malloc fails how likely it is that sending message will success? If user needs a notification about delivery, and doesn't get one, wouldn't that be a huge problem too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most likely way is if the "iotHubMessageId" string gets trashed in memory and the malloc_and_Strcpy_s fails, but even then it's unlikely that the IoTHubClient_SendEventAsync will succeed as well.

So it's pretty unlikely it worth trying to send the message. I'm OK with leaving it alone, just more curious if you had a different answer..

if (!userContextCallback)
{
LogError("Context was null");
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return [](start = 8, length = 6)

The coding standards for our C code is to have one return call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@az-iot-builder-01 az-iot-builder-01 merged commit b5df470 into Azure:master Jan 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants