Introduce the network-socket API to mbed #2216
Conversation
Responded to feedback from mbed-client implementation to introduce a full feature set that should support most of the use cases for the API.
Pros - Allows memory to be statically allocated - Avoids issues with Thread creation before entering main - Matches existing APIs such as FunctionPointer and Ticker Cons - Does not enforce passing a NetworkInterface
Pros - Simplifies interface - Easier base implementation Cons - May need shutdown functionality, in this case shutdown can be added as another function in the future
Pros - Easier to implement - More similar to SIGIO in BDS sockets Cons - Less information, but this information had a high risk of being faulty/spurious
- socket_create -> socket_open - socket_destroy -> socket_close
Bytes are stored by default, however enough space is allocated in a SocketAddress to generate the string representation if necessary. Currently there is no support for shortened addresses
Correctly set and return the ipv6 address.
Bind can operate on any IP socket and is not specific to a protocol
Adds the following functions for direct configuration of interface - (set|get)stackopt - (set|get)sockopt
supported:
1.2.3.4
1:2:3:4:5:6:7:8
1:2::7:8
::
currently not supported:
1:2:3:4:5:6:1.2.3.4
- Not supported by TCP/UDP protocols - Uncommon and less useful with proper error handling
NetworkInterface -> NetworkStack EthernetInterface -> EthernetStack WiFiInterface -> WiFiStack CellularInterface -> CellularStack MeshInterface -> MeshStack
- Avoids ambiguity when both are used - Matches Python behaviour
mirrored from: https://developer.mbed.org/teams/NetworkSocketAPI/code/NetworkSocketAPI/ - Fix bug with SocketAddress init per @c1728p9 - Fix issue with not passing interface through accept call - Fix port issue in SocketAddress constructor
Add mutexes to protect the network socket API. Also use semaphores to wait for read/write events. Also fix a typo in the comments for timeout.
Fix typo causing ipv6 addresses in the constructor to fail.
*cough* esp8266 *cough* this is especially important for event-loop based systems where excessive events results in problematic memory consumption.
This patch consists of:
-Add NetworkInterface to wrap objects bound to a stack and update
socket code to handle this in addition to NetworkStacks
-Update MeshInterface to inherit from NetworkInterface
-Update NanostackInterface so it only inherits from NetworkStack
-Add MeshInterfaceNanostack and update LoWPANNDInterface and
ThreadInterface to inherit from this
Remove read and write mutexes since multiple calls to send or multiple calls to recv on different threads is undefined behavior. This is because the size of data sent or received by these calls is undefined and could lead to the data being interleaved. The code now asserts that there are not multiple threads calling send at the same or calling recv at the same time. Note that calling send and recv from different threads at the same time is still safe and well defined behavior. By removing the read and write mutexes and associated timeout it guarantees that a stack call will always be made and thus the value NSAPI_ERROR_TIMEOUT cannot get falsely returned.
- Provide nsapi.h as central entry point - Provide nsapi_types.h as c-compatible destination for nsapi types - Standardize include paths
Provides primitive type for network address that is more constrained than "pointer to array of bytes". - POD type avoids issues with constructors - Removes complexity from SocketAddress class - Easily passed through C interfaces - Easily casted to SocketAddress and vice-versa
typedef void *nsapi_socket_t Helps avoid confusion with multiple void*s floating around, especially when passing a nsapi_socket_t by pointer as a destination. The nsapi_socket_t is just an opaque handle, implementations still need a cast to obtain implementation specific socket pointer.
Provides a primitive structure for explicitly instantiating network stack structures/vtables. - Avoids reoccuring issue with non-gced vtables - Provides thick api layer for additional nsapi features - Provides more explicit seperation between implementation and user api Now implementors have two options: 1. Extend NetworkStack and implement all of the abstract member functions 2. Fill out a nsapi_stack_api_t and provide a NetworkStack with nsapi_create_stack option 1 provides an easy route for porting single drivers such as the esp8266 or the c027 and can easily benefit from other C++ classes. option 2 provides an easy route for porting full network-stacks such as lwip or nanostack, which provide their own host of networking utilities and only need a minimal socket interface.
Mostly reduced code duplication for NetworkInterface/NetworkStack arguments. Moved to templated Socket constructors/open member functions. This allows multiple inheritance to be used for classes that provide both a NetworkInterface and NetworkStack, such as the esp8266.
NetworkStack/NetworkInterface overloads - Multiple inheritance results in ambiguity NetworkStack with templated get_stack overload - get_stack hidden behind protected access prevents template match - explicit call to NetworkInterface::get_stack does not follow vtable Solution is to use two overloaded function for NetworkStack/NetworkInterface, and an overload for a templated type that supports a static_cast to NetworkStack. This resolves the ambiguity without being blocked by protected access. Moved duplicated overloads out to overload of common nsapi_create_stack function.
|
What is the purpose of having this change of commits detached from master? This layout prevents git bisect from being used withing these new commits since they no longer build standalone. |
|
The content of this PR looks great! Aside from the previous question about this history, this PR LGTM. |
|
@c1728p9, I can probably rebase these commits on top of current master, although there's no garuntees that they will compile or reflect valid history. |
|
@c1728p9, looking into this more, it's actually non-trivial to rebase subtrees (ref). I could rebase and manually adjust each commit that modifies the folder structure, but that seems intractable for other similar prs. How do you feel about leaving this pr history as it is? Alternatively I can squash the history into a single commit? If anyone else has strong thoughts feel free to voice them. |
|
Did you ever test this? Previously client was forced to use Now you renamed the path so all Socket users need to change their apps/libs. |
|
Ah, very sorry about this. Unfortunately this was unavoidable with the impending rename. Adopting the nsapi.h should avoid this issue in the future (brought in here). Fortunately, we can avoid breakage as long as #2205 is updated to match the mbedmicro/mbed structure. |
|
No, we cannot avoid breakages until #2240 is fixed. I'm not saying it should be fixed now, but it should be there as reported. |
|
Ah, I meant we can avoid immediate breakage. Besides needing to update the incoming client codebase, is there a bigger issue with this restructure? I can continue the conversation on header collisions in #2240, you're right that we need a documented solution. |
|
No immediate breakages once you merge in the PR #2205 |
This patch introduces the network-socket API into to core mbed repo.
cc @sg-, @c1728p9