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

Provisioning Client requires optional SocketIO implementation in the PAL layer. #456

Closed
seank-com opened this issue Apr 14, 2018 · 6 comments
Assignees

Comments

@seank-com
Copy link

Found while trying to port 2018-03-02 on master to the TI CC3220SF

Description of the issue:

mqtt_transport_io and amqp_transport_io both take a HTTP_PROXY_OPTIONS* as a second parameter. Even if you pass NULL the existence of http_proxy_io_get_interface_description still needs to link with code implemented by the SocketIO component of the PAL layer. The SocketIO component is optional according to the porting guide.

Code sample exhibiting the issue:

We had to work around this with code like the following:

#ifndef TI_RTOS
    HTTP_PROXY_IO_CONFIG proxy_config;

    if (proxy_info != NULL)
    {
        /* Codes_PROV_TRANSPORT_MQTT_CLIENT_07_012: [ If proxy_info is not NULL, amqp_transport_io shall construct a HTTP_PROXY_IO_CONFIG object and assign it to TLSIO_CONFIG underlying_io_parameters ] */
        proxy_config.hostname = tls_io_config.hostname;
        proxy_config.port = MQTT_PORT_NUM;
        proxy_config.proxy_hostname = proxy_info->host_address;
        proxy_config.proxy_port = proxy_info->port;
        proxy_config.username = proxy_info->username;
        proxy_config.password = proxy_info->password;

        tls_io_config.underlying_io_interface = http_proxy_io_get_interface_description();
        tls_io_config.underlying_io_parameters = &proxy_config;
    }
#endif

Recommendation

Ideally, the provisioning client would require you pass NULL, otherwise error if the PAL layer did not implement the SocketIO component.

@jebrando jebrando self-assigned this Apr 17, 2018
@jebrando
Copy link
Contributor

The only time that proxy_info would not be NULL is if the application has called the following API:

Prov_Device_LL_SetOption(handle, OPTION_HTTP_PROXY, &http_proxy)

I'm unable to see where the proxy would be getting set. is this the case in the application?

@seank-com
Copy link
Author

Sorry for not being more clear. The PAL layer for the TI CC3220 does not implement the socketio PAL module because it is optional (which is good because there is no easy way to implement it). Consequently, when you try to compile the provisioning_client, the call to http_proxy_io_get_interface_description in the code fragment above will generate a link error because the socketio interface is not defined.

@jebrando
Copy link
Contributor

jebrando commented May 1, 2018

Gotcha, ok let me see how I can fix this. Give me a day or two.

@jebrando jebrando added the bug label May 1, 2018
@seank-com
Copy link
Author

Great! I was thinking if there is a way to determine at compile time if the socketio module is implemented in the PAL layer then #ifdef the code to return an error if you try to call it with a non-null value. I just didn't know the code well enough to put together a PR.

@BigPeteB
Copy link
Contributor

This is fixed by adding an HTTP proxy stub implementation that will satisfy the linker, but does nothing and shouldn't be called. This will be available in the next release when the c-utility submodule is updated.

@az-iot-builder-01
Copy link
Collaborator

@seank-com, @BigPeteB, thank you for your contribution to our open-sourced project! Please help us improve by filling out this 2-minute customer satisfaction survey

cringti pushed a commit to TexasInstruments/azure-iot-pal-simplelink that referenced this issue Nov 7, 2018
This commit consolidates work done for the Azure IoT 2.10 Plugin
release, based on TI's SimpleLink 2.30 SDKs (AZUREIOT-29) and
Microsoft's Azure IoT SDK LTS Oct 2018 build (AZUREIOT-28).

DPS support was added (AZUREIOT-26).  This required the addition of
the Parson library, as well as the socketio layer to the SimpleLink
PAL.  Notably:

- By default, Parson requires files support, but it was changed to a
  'lite' version (no file dependencies) to be friendlier to
  embedded devices.
- We can revisit (and ideally remove) the socketio dependency when
  Microsoft addresses issue #456:
  Azure/azure-iot-sdk-c#456

Logging was added to the SDK and PAL debug libraries (AZUREIOT-24).

products.mak was made more intelligent for Plugin users by picking up
the installation locations of the various components if they were
previously set in the plugin or SDK (AZUREIOT-33).

We addressed the following items based on feedback from Microsoft
(AZUREIOT-41):

- Null argument checks in functions
- Use of macros from shared_util_options.h
- Additional error messages
- Redundant/Unnecessary assignments
- Better error handling in HTTPAPI_ExecuteRequest
- Replacement of tickcounter_sl.c with tickcounter_linux.c from Azure SDK
- Use mallocAndStrcpy_s() to simplify code
- Allocate contentBuf in HTTPAPI_ExecuteRequest from the heap to reduce
  stack space requirements
- Discard response body when responseContent buffer is NULL
- Add comment on receive buffer size in tlsio

Signed-off-by: Chris Ring <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants