Initial commit with azure-sdk-for-c Arduino library #1

ewertons · 2021-12-01T06:51:18Z

No description provided.

library.properties

CIPop

Left a few comments. The one thing I care strongly about is moving az_cert.h to each sample instead of the main lib.

CIPop · 2021-12-01T21:30:50Z

library.properties

@@ -0,0 +1,10 @@
+name=azure-sdk-for-c
+version=1.0.0


To simplify things, can we keep this version in-sync with the SDK version?

I think we talked about this in the meetings we had with Azure SDK team, but I do not find notes about it. Since the versions will be sequential anyway, I don't see why not. I updated the Update-library.ps1 to implement that as well.

Posting conversation we had offline, for reference:

Cris, little problem with the version suggestion you made....

Linking the version of the SDK to the version of the Arduino library makes it impractical for us to make changes to the samples in the Arduino repo itself and update the version. In spite of that, what do you think about doing this:

name=Azure SDK for C
version=1.0.0
author=Microsoft Corporation
maintainer=Microsoft iotcert@microsoft.com
sentence=Azure SDK for C library (1.3.0-beta.2) for Arduino.

instead of

name=Azure SDK for C
version=1.3.0-beta.2
author=Microsoft Corporation
maintainer=Microsoft iotcert@microsoft.com
sentence=Azure SDK for C library for Arduino.

Done as suggested above.

What are your thoughts on trying to keep the major version aligned, even as changes are made to samples/etc. independently of the SDK?

I myself think that it makes more sense to keep the versions independent, but I would not oppose keeping the major versions in sync.

library.properties

src/azure-sdk-for-c.h

src/az_version.h

tools/Update-Library.ps1

src/azure_ca.h

CIPop · 2021-12-01T21:51:23Z

library.properties

+category=Communication
+url=https://github.com/Azure/azure-sdk-for-c
+architectures=*
+includes=az_core.h,az_iot.h,azure_ca.h


We should remove azure_ca.h from here and, instead, instruct devs to update their devices/modems with the latest CA certificates. For official Arduino boards, the IDE has tools that support the update.

The include can be made by samples for a quick OOB experience.

Cris, that has been discussed before. There is a reason not to do that way.

If someone add the azure-sdk-for-c Arduino library (not using one of the built-in samples, the Root CA cert will be missing. To make the experience easy and match the standard out-of-the-box-smooth way expected from Arduino libraries, the ca cert header must be in the library.

If you are still in favor of doing differently, let's talk offline so we can find the best solution.

What about my proposal to move the include in the samples? Instead of adding the include in the library, you can add it in each of the samples folders. This will automatically bring it in for samples but not for new projects. It will also make it clear that the file is part of the sample, not our SDK.

The include can be made by samples for a quick OOB experience.

But that is exactly the situation the previous comment is addressing.
If you do that, people that include the library on their own projects won't have all the components for an out-of-the-box experience like what's expected when using Arduino.

I see. Yeah: with my proposal only the samples will work by default.
Let's take this offline and see if we can integrate with Arduino's WiFi FW update system such as this one for WiFiNINA:

Yeah, that is a nice tool they have for the Wifi101 module.
Unfortunately Espressif does not have something similar in Arduino for their boards (nor has Realtek).
With those limitations, we don't have an alternative to work with as of now. I agree with you they should and it would be better, but that's what we have to work with as of now.

I will keep this comment unresolved for tracking purposes.
One aspect to address in a next release of this Arduino library will be to add the other root cas for other regions as well (!).

tools/Update-Library.ps1

README.md

examples/AzureIoTHub_ESP32/AzureIoTHub_ESP32.ino

examples/AzureIoTHub_ESP8266/AzureIoTHub_ESP8266.ino

examples/AzureIoTHub_RealtekAmebaD/AzureIoTHub_RealtekAmebaD.ino

examples/AzureIoTHub_RealtekAmebaD/readme.md

library.properties

examples/AzureIoTHub_ESP32/AzureIoTHub_ESP32.ino

examples/AzureIoTHub_ESP8266/AzureIoTHub_ESP8266.ino

examples/AzureIoTHub_RealtekAmebaD/AzureIoTHub_RealtekAmebaD.ino

ahsonkhan · 2021-12-03T23:04:14Z

@ewertons What files and changes within this PR should we explicitly review vs what can we assume are coming verbatim from latest released tag of the Azure SDK for C repo?

Can we assume everything under src/ is the same as the main repo (other than header fix-ups)?

ahsonkhan · 2021-12-03T23:05:49Z

And do we have any plans to do any build verification or basic sanity testing in the CI for this repo? What steps do you currently take to manually test things?

It might be a good idea to at least have a step that verifies the repo builds in the CI to catch any typos or accidental changes that might cause it to break.

ewertons · 2021-12-03T23:08:38Z

And do we have any plans to do any build verification or basic sanity testing in the CI for this repo? What steps do you currently take to manually test things?

It might be a good idea to at least have a step that verifies the repo builds in the CI to catch any typos or accidental changes that might cause it to break.

The tests I have done are manually using the latest version of Arduino IDE.
In our plans we listed adding automation for the verification going forward, which I'll work on this December.

ewertons · 2021-12-03T23:11:25Z

@ewertons What files and changes within this PR should we explicitly review vs what can we assume are coming verbatim from latest released tag of the Azure SDK for C repo?

Can we assume everything under src/ is the same as the main repo (other than header fix-ups)?

Yes, that's correct. Everything under src/ is coming straight from azure/azure-sdk-for-c, with the files processed to be in a flat structure required for Arduino libraries.
The other folders contain the Examples and the tool/script to update the library (a powershell script).
The other important file is library.properties, which provides the main definition of the library.

All of the files here come almost as-is from the initial repo I created, which had been previously used in our discussions.

ahsonkhan

Overall looks great, but did some have comments and questions that I was hoping to understand our thinking on, before approving. That said, once addressed, no need to block on my approval :)

Thanks @ewertons. Looking forward to the customer experience and feedback with this!

cc @antkmsft, @vhvb1989 just an FYI (once storage comes online) or in case you have any other thoughts to share.

src/az_base64.h

src/az_config_internal.h

src/az_core.h

library.properties

src/az_iot_hub_client.c

README.md

examples/Azure_IoT_Hub_RealtekAmebaD/readme.md

library.properties

ahsonkhan · 2021-12-04T00:26:28Z

And do we have any plans to do any build verification or basic sanity testing in the CI for this repo? What steps do you currently take to manually test things?
It might be a good idea to at least have a step that verifies the repo builds in the CI to catch any typos or accidental changes that might cause it to break.

The tests I have done are manually using the latest version of Arduino IDE. In our plans we listed adding automation for the verification going forward, which I'll work on this December.

👍

Unless we automate most of "Ewerton magic", whatever basic manual steps you do take now, or know you plan to take in the future, it might be a good idea to have a quick "what to do" steps documented somewhere accessible, and might even check-it-in somewhere in this repo (which you are probably the best person to decide, so up to you). This way, any new person on the team would know what to do to maintain and test this repo and the syncing mechanism.

examples/Azure_IoT_Hub_ESP32/AzIoTSasToken.cpp

library.properties

ahsonkhan

Some typos and comment fix-up suggestions. Also, fix-up the contact email/alias. Otherwise, looks good to me :)

README.md

library.properties

tools/Update-Library.ps1

ewertons · 2021-12-06T23:20:45Z

Tested all samples one final time. All worked with the latest changes at merge time.
Tested on: Arduino IDE 1.8.16, ESP32 board lib 2.0.1, ESP8266 board lib 3.0.1, Realtek Ameba board lib 3.0.7, PubSubClient 2.8.0.

ewertons · 2021-12-06T23:34:11Z

For reference, this is the PR adding this new library to the official Arduino package manager:
arduino/library-registry#768

Updates Nano Connect sample for IoT Hub: - removes x509 cert device authentication (unused) - makes code C++ friendly - removes az_span where not directly used for Embedded C SDK - defines macros to replace magic numbers - refactors functions as needed - combines SAS material together (intentionally fewer functions to not distract from core sample) - fixes logging - fixes C2D messaging - Adds commenting - makes return code printouts consistent

Initial commit with azure-sdk-for-c Arduino library

4dc0fdf

wduraes self-requested a review December 1, 2021 18:21

ewertons requested review from ahsonkhan, danieljurek, momuno, danewalton, ericwolz, JeffreyRichter, ryanwinter and CIPop December 1, 2021 19:08

ericwolz reviewed Dec 1, 2021

View reviewed changes

library.properties Outdated Show resolved Hide resolved

ericwolz approved these changes Dec 1, 2021

View reviewed changes

CIPop requested changes Dec 1, 2021

View reviewed changes

Improve Update-Library.ps1, update SDK bits

63673a6

ewertons requested a review from CIPop December 2, 2021 00:32

danewalton approved these changes Dec 2, 2021

View reviewed changes

CIPop approved these changes Dec 2, 2021

View reviewed changes

wduraes reviewed Dec 3, 2021

View reviewed changes

library.properties Outdated Show resolved Hide resolved

wduraes approved these changes Dec 3, 2021

View reviewed changes

examples/AzureIoTHub_ESP32/AzureIoTHub_ESP32.ino Outdated Show resolved Hide resolved

examples/AzureIoTHub_ESP8266/AzureIoTHub_ESP8266.ino Outdated Show resolved Hide resolved

examples/AzureIoTHub_RealtekAmebaD/AzureIoTHub_RealtekAmebaD.ino Outdated Show resolved Hide resolved

ewertons added 4 commits December 2, 2021 16:38

Update library name to show Azure SDK for C instead of azure-sdk-for-c

559b241

Remove unnecessary data from telemetry in samples

004e4a3

Address CR comments.

6cebfac

Rename samples for better readability

9f3a583

ewertons force-pushed the ewertons/initiallibrary branch from 3565a5f to 9f3a583 Compare December 3, 2021 22:26

ahsonkhan requested a review from RickWinter December 3, 2021 22:49

ewertons removed the request for review from ryanwinter December 3, 2021 23:12

ahsonkhan reviewed Dec 4, 2021

View reviewed changes

Remove extra endline on azure-sdk-for-c files and fix script

ccbaf91

ahsonkhan reviewed Dec 4, 2021

View reviewed changes

examples/Azure_IoT_Hub_ESP32/AzIoTSasToken.cpp Show resolved Hide resolved

ahsonkhan reviewed Dec 4, 2021

View reviewed changes

library.properties Outdated Show resolved Hide resolved

Update language in library description, add license to README.md

45d5a31

ahsonkhan mentioned this pull request Dec 4, 2021

Add comment about not using internal or private symbols to all the relevant files Azure/azure-sdk-for-c#2035

Closed

Update version of SDK, version strings

d1ea868

ewertons force-pushed the ewertons/initiallibrary branch from a3b077d to d1ea868 Compare December 4, 2021 18:27

ahsonkhan approved these changes Dec 6, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

library.properties Outdated Show resolved Hide resolved

library.properties Show resolved Hide resolved

tools/Update-Library.ps1 Outdated Show resolved Hide resolved

tools/Update-Library.ps1 Outdated Show resolved Hide resolved

ewertons added 2 commits December 6, 2021 14:11

Update maintainer email address

bd07c01

Fix latest comments from PR review (typos, logging)

1450e34

ewertons merged commit e1b30ae into main Dec 6, 2021

ewertons deleted the ewertons/initiallibrary branch December 6, 2021 23:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial commit with azure-sdk-for-c Arduino library #1

Initial commit with azure-sdk-for-c Arduino library #1

ewertons commented Dec 1, 2021

CIPop left a comment

CIPop Dec 1, 2021

ewertons Dec 2, 2021

ewertons Dec 3, 2021

ewertons Dec 3, 2021

ahsonkhan Dec 3, 2021

ewertons Dec 3, 2021

CIPop Dec 1, 2021

ewertons Dec 1, 2021

CIPop Dec 2, 2021

ewertons Dec 2, 2021

CIPop Dec 2, 2021

ewertons Dec 3, 2021

ahsonkhan commented Dec 3, 2021

ahsonkhan commented Dec 3, 2021

ewertons commented Dec 3, 2021

ewertons commented Dec 3, 2021

ahsonkhan left a comment

ahsonkhan commented Dec 4, 2021

ahsonkhan left a comment •

edited

Loading

ewertons commented Dec 6, 2021

ewertons commented Dec 6, 2021

Initial commit with azure-sdk-for-c Arduino library #1

Initial commit with azure-sdk-for-c Arduino library #1

Conversation

ewertons commented Dec 1, 2021

CIPop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahsonkhan commented Dec 3, 2021

ahsonkhan commented Dec 3, 2021

ewertons commented Dec 3, 2021

ewertons commented Dec 3, 2021

ahsonkhan left a comment

Choose a reason for hiding this comment

ahsonkhan commented Dec 4, 2021

ahsonkhan left a comment • edited Loading

Choose a reason for hiding this comment

ewertons commented Dec 6, 2021

ewertons commented Dec 6, 2021

ahsonkhan left a comment •

edited

Loading