Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for cloud to device messages #1490

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jltorresm
Copy link
Contributor

Adds support for cloud-to-device messages.

@demoray
Copy link
Contributor

demoray commented Dec 5, 2023

@heaths I would like your feedback on this from a future-project direction standpoint.

For other languages, separate libraries are maintained outside of the Azure SDK.

Thus far, we've had limited updates for the azure_iot_hub crate maintained in this repo.

How should we move forward with regards to this and future features for azure_iot_hub?

@jltorresm
Copy link
Contributor Author

Are there any updates on this topic? I would love to have support for cloud-to-device messages in the current crate. I don't oppose having it as a separate repo, but in any case, it would be nice to have this merged before that happens.

@heaths
Copy link
Member

heaths commented Jan 18, 2024

Sorry for the delay. This dropped off my radar.

I believe the other languages, it was done that way because of legacy. The service team supports them themselves but still mostly follows the Azure SDK guidelines. For the Rust SDKs, we'll need to find out. I'll figure out appropriate contacts and add them to this PR to discuss.

Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

One nit, but the secrets must be scrubbed and you should rotate them if you still have that resource imediately.

/// use azure_iot_hub::service::ServiceClient;
/// use std::collections::HashMap;
///
/// # let connection_string = "HostName=cool-iot-hub.azure-devices.net;SharedAccessKeyName=iot_hubowner;SharedAccessKey=YSB2ZXJ5IHNlY3VyZSBrZXkgaXMgaW1wb3J0YW50Cg==";
Copy link
Member

Choose a reason for hiding this comment

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

This secret must be removed and you should rotate it immediately. Do not put secrets in the repo. Mark this code fence no_run and use environment variables in your example.

Copy link
Member

Choose a reason for hiding this comment

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

Use the same ones as your other example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a real secret, if you base64 decode it you'll realize.

Copy link
Member

Choose a reason for hiding this comment

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

Our secret scanner may still flag it. It doesn't decode the values (most won't be ASCII anyway). Since the value won't work anyway, you could just leave it blank.

let properties = std::env::args().nth(3).map(|s| {
s.split(',')
.map(|s| {
let mut split = s.split('=');
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'd use splitn(2, '=') here to get the rest as the value.

@jltorresm
Copy link
Contributor Author

Thanks for the comments. I'll clean this up and push the fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants