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

Consolidate module sample into one, and add reconnection logic #151

Merged
merged 6 commits into from
Oct 6, 2020

Conversation

abhipsaMisra
Copy link
Contributor

@abhipsaMisra abhipsaMisra commented Oct 5, 2020

Link to readme: https://github.com/Azure-Samples/azure-iot-samples-csharp/tree/abmisr/moduleSample/iot-hub/Samples/module/ModuleSample

The reconnection logic and sample structure is the same as the DeviceReconnectionSample. I've added some additional setup info and code snippet into the readme (to help people who simply want to check the API usage, and not a complete module simulation).

@abhipsaMisra abhipsaMisra requested a review from drwill-ms October 5, 2020 21:54
ContentType = "application/json",
};

await moduleClient.SendEventAsync(message);

Choose a reason for hiding this comment

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

worth talking about which classes of exceptions one might expect, and what app behavior is required based on that. Meaning, you might retry, reinit your client, or quit.

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 point, how about I add a link to our sample where we handle the most common cases?

Choose a reason for hiding this comment

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

Elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We touch on those in the section "Module reconnection" below. I feel that if we add those instructions here again, it might be repetitive.
I don't necessarily want to list exceptions based on operation - i.e. you might see these exceptions while sending telemetry - since the list might not be exhaustive (we'd don't have an operation: exception mapping yet).

Thoughts?

@drwill-ms
Copy link

What samples were consolidated?

@drwill-ms
Copy link

I lament we copy code from another sample, meaning we'll need to update it in multiple places. What do you think about making a sample helper library for common stuff?

@abhipsaMisra
Copy link
Contributor Author

What samples were consolidated?

The module messaging sample, and the module twin sample.

@abhipsaMisra
Copy link
Contributor Author

I lament we copy code from another sample, meaning we'll need to update it in multiple places. What do you think about making a sample helper library for common stuff?

I totally agree, although, we might be better off consolidating it as a part of the "samples-refresh", what do you think?

  • We can take in the messaging code, the twin update code, etc as sub-modules; and refer to it in both the samples.
  • We can create a pluggable client, which performs these operations, and pass in a device client/ module client as required.

I think we might come up with more ideas as we clean up the samples, and that might help us in getting to a better (more efficient/ cleaner) approach.

@abhipsaMisra abhipsaMisra merged commit b757af5 into master Oct 6, 2020
@abhipsaMisra abhipsaMisra deleted the abmisr/moduleSample branch October 6, 2020 17:00
rido-min pushed a commit to rido-min/azure-iot-samples-csharp that referenced this pull request Oct 19, 2020
…-Samples#151)

* samples(iot-device): Consolidate module sample into one, and add reconnection logic
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.

3 participants