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 "Upload To Blob" feature (CA-19) #68

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

giroudon
Copy link

@giroudon giroudon commented Nov 8, 2019

Hello Esspressif Crew,

Here are some edits to support "Upload To Blob" operations in esp-azure.
This required 2 things :

  • Enable HTTP(S) upload feature by adding httpapi_compact.c adapter in the component.mk list
  • Upgrade to azure-iot-sdk-c version 1.3.1 to avoid a NULL reference bug (issue #814)

Regards

@frederic-thibault
Copy link

Hi,

I checkout and try to build it. I received the following error.

CMake Error at D:/ESP/esp-idf/tools/cmake/component.cmake:449 (add_library):
Cannot find source file:

D:/ESP/esp-azure-goobie/azure-iot-sdk-c/c-utility/src/base64.c

The base64.c has been rename azure_base64.c

@frederic-thibault
Copy link

Which version of esp-idf are you using? I try to build the with the release/v4.0

Another error that I can't fix.

D:/ESP/esp-azure-goobie/azure-iot-sdk-c/c-utility/inc/azure_c_shared_utility/strings.h:18:1: error: unknown type name 'MU_IF'
MOCKABLE_FUNCTION(, STRING_HANDLE, STRING_new);

@giroudon
Copy link
Author

giroudon commented Nov 8, 2019

I am using esp-idf v3.2.3 with the Makefile build system.
Renaming of c-utility/src/base64.c to c-utility/src/azure_base64.c was done in commit 927f0a0 and should be replicated to CMake gears.

@frederic-thibault
Copy link

Hi,

I finally make it works with CMakefile changes and idf v3 but my problem with the upload don't works when connecting to iot hub using x509 certificate. Sending messsage, direct method, registration using provisioning service works fine, but not upload. Upload works with sas but not certificate.

Thanks,

@giroudon
Copy link
Author

giroudon commented Dec 3, 2019

Hi Frederic.
After testing, I also noticed this issue when using x509 certificates. On devices configured using DPS provisioning, it fails with the x509 certificate method and works with the Symmetric Key method.

@B-Temia
Copy link

B-Temia commented Dec 4, 2019

Hi,

I didn't find where is the probleme exactly. IoTHub SDK doesn't upload the file directly to the hub. It is supposed to implement the valet key pattern. I believe that the problem is at the level of the request to receive the key and not for the upload .... for the moment at least.

I fix it by upload the file to a .net core 3 api with certificate authentication. The same certificate use for the provisioning. I cannot upload it directly to the blob using the valet key pattern because the blob doesn't support uploading with many chunk... or I didn't find how to do it.

Hopefully they will fix it one day!

@projectgus projectgus changed the title Add support for "Upload To Blob" feature Add support for "Upload To Blob" feature (CA-19) Jan 1, 2020
@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

@shahpiyushv
Copy link
Collaborator

@giroudon , thanks for the contribution. Do you mind adding examples for the features, in line with the other examples? That will make this PR complete.

@giroudon giroudon force-pushed the master branch 2 times, most recently from 096166c to a701533 Compare September 15, 2020 09:47
Co-authored-by: Aurélien Chevé
@giroudon
Copy link
Author

MQTT-over-WebSocket support now added to iothub_client_sample_mqtt/. Can be enabled from menuconfig (option "Example Configuration/MQTT over WebSocket").
This commit also fixes a TLS renegotiation option management, required for MQTT-over-WebSocket.

@shahpiyushv
Copy link
Collaborator

@giroudon , sorry for the late response. Can you make these changes

  • Remove the updates made to the azure-iot-sdk-c submodule (wouldn't the current LTS release work)
  • Check if the TLS renegotiation logic can be removed, since it has been fixed in ESP IDF and backported till ESP IDF release/4.0.
  • Squash some commits so that the required changes are much more self explanatory, and probably split the MR into 2 (blob upload and websocket)

If you do not have enough time to make these changes, can you atleast take care of the last point and we will try to take up the rest internally.

@vidarmikaelsson
Copy link

@giroudon , sorry for the late response. Can you make these changes

  • Remove the updates made to the azure-iot-sdk-c submodule (wouldn't the current LTS release work)
  • Check if the TLS renegotiation logic can be removed, since it has been fixed in ESP IDF and backported till ESP IDF release/4.0.
  • Squash some commits so that the required changes are much more self explanatory, and probably split the MR into 2 (blob upload and websocket)

If you do not have enough time to make these changes, can you atleast take care of the last point and we will try to take up the rest internally.

Hi guys! Is there any progress on this issue? The option to have MQTT over WebSocket is really quite cruical, and to have an officially supported version of this by Espressif is something that we would need in order to use your product on a large scale.

@daniel-grabner
Copy link

As we are urgently waiting for the MQTT over WebSocket feature I have tested whether the implementation by @giroudon works for us. I have cloned the fork from goobie-lab:master, used it in our project and unfortunately it does not work for us.

Even the example iothub_client_sample_mqtt does not work, once I enable the SAMPLE_MQTT_OVER_WEBSOCKET config via menuconfig. It gives me the following linker error:
undefined reference to MQTT_WebSocket_Protocol

Tested with esp-idf version v4.1.

@giroudon Does the example work for you? Which esp-idf version are you using for your tests? Please find the compiler log attached.

iothub_client_sample_mqtt_BuildCmdLineOutput.txt

@giroudon
Copy link
Author

@daniel-grabner, the MQTT over WS is currently integrated on a commercial product using esp-idf 3.3.5. I confirm it is not yet operational on esp-idf 4. I will try to solve this issue as soon as possible (well I hope so... I do not have much time these days).

On esp-idf 3, you also need to checkout a recent version of mbedtls (2.16.7-idf) with the following command:
(cd "$IDF_PATH/components/mbedtls/mbedtls/" && git checkout mbedtls-2.16.7-idf)

@jspngh
Copy link
Contributor

jspngh commented Feb 18, 2021

Since I would like to have these fixes for WebSockets available, I decided to give a hand
and split off the WebSocket-part of this PR into #111.

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.

8 participants