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

Fixes GCC virtual device running with UDP protocol #1462

Merged
merged 1 commit into from Jan 22, 2018

Conversation

@avtolstoy
Copy link
Member

commented Jan 16, 2018

submission notes
**Important:** Please sanitize/remove any confidential info like usernames, passwords, org names, product names/ids, access tokens, client ids/secrets, or anything else you don't wish to share.

Please Read and Sign the Contributor License Agreement ([Info here](https://github.com/spark/firmware/blob/develop/CONTRIBUTING.md)).

You may also delete this submission notes header if you'd like. Thank you for contributing!

Problem

There are two issues that prevent GCC virtual device to successfully connect to the cloud with UDP:

  1. (main) The UDP socket is being bound to a multicast address 224.0.0.251, instead of 0.0.0.0 or some interface address
  2. Cloud sockets use the same source UDP port, which prevents from running several virtual devices on the same host.

Solution

  1. Bind to 0.0.0.0
  2. Use ephemeral source port when creating UDP cloud socket

Steps to Test

Compile PLATFORM=gcc and test whether it can connect to the cloud with UDP protocol.

Example App

N/A

References

  • [CH10641]

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)
Fixes UDP cloud connection on GCC platform: socket needs to be bound …
…to 0.0.0.0 and in order to run multiple clients on same host, an ephemeral port needs to be used, instead of a static one

@avtolstoy avtolstoy added the bug label Jan 16, 2018

@avtolstoy avtolstoy requested a review from m-mcgowan Jan 16, 2018

@@ -1104,8 +1104,13 @@ int spark_cloud_socket_connect()
ip_address_error = determine_connection_address(ip_addr, port, server_addr, udp);
if (!ip_address_error)
{
uint8_t local_port_offset = (PLATFORM_ID==3) ? 100 : 0;
sparkSocket = socket_create(AF_INET, udp ? SOCK_DGRAM : SOCK_STREAM, udp ? IPPROTO_UDP : IPPROTO_TCP, port+local_port_offset, NIF_DEFAULT);
#if PLATFORM_ID == 3

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 16, 2018

Contributor

can we push this down to the HAL to avoid platform specifics in non-HAL code

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Jan 16, 2018

Author Member

I'd disagree. In that case all UDP sockets would use ephemeral ports. For some specific use-cases we might need to use a specific UDP port.

We have something similar on Electron: https://github.com/particle-iot/firmware/blob/develop/system/src/system_task.cpp#L292

Might be a good idea to move this block there.

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 16, 2018

Contributor

What I mean is an option that lets the platform HAL decide the source port specifically for the cloud socket. Perhaps an option "UseEphemeralPortForCloud" that is sourced from the HAL, so there is no platform conditionality in the non-HAL code.

@m-mcgowan m-mcgowan added this to the 0.8.0-rc.2 milestone Jan 18, 2018

@m-mcgowan m-mcgowan merged commit 41c6f73 into develop Jan 22, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@m-mcgowan m-mcgowan deleted the fix/gcc-virtual-udp branch Jan 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.