sock: doc fixes by miri64 · Pull Request #5884 · RIOT-OS/RIOT

miri64 · 2016-09-28T13:19:13Z

Fixes the comments in #5758.

miri64 · 2016-09-28T13:20:03Z

Also includes the HOWTOs requested by @emmanuelsearch.

aabadie

I had a "brief" look at this one and put some comments (typo, rephrasing). Good job !

aabadie · 2016-09-28T13:21:28Z

sys/include/net/sock.h

- * they serve as a common API that connects application- and network stack code.
+ * This module provides a set of functions to establish connections or send and
+ * receive datagrams using different types of protocols. Together, they serve as
+ * an API allows an application or library to connect to a network.


that allows or allowing

aabadie · 2016-09-28T13:23:17Z

sys/include/net/sock.h

+ * Implementors Notes
+ * ==================
+ * ### Type definition
+ * For simplicity and modularity this API doesn't put any restriction of the


aabadie · 2016-09-28T13:23:44Z

sys/include/net/sock.h

+ * ### Type definition
+ * For simplicity and modularity this API doesn't put any restriction of the
+ * actual implementation of the type. For example, one implementation might
+ * choose to have all `sock` types have a common base class or use the raw IP


types having

aabadie · 2016-09-28T13:26:39Z

sys/include/net/sock/ip.h

+ * res = sock_ip_send(NULL, data, data_len, PROTNUM, &remote);
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * With `data` being the data send, `data_len` the length of `data`, `PROTNUM`


aabadie · 2016-09-28T13:27:13Z

sys/include/net/sock/ip.h

+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * With `data` being the data send, `data_len` the length of `data`, `PROTNUM`
+ * the next header number for the send packet and `remote` the remote end point


sent packet

aabadie · 2016-09-28T13:31:13Z

sys/include/net/sock/ip.h

+ *     }
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * We then create a remote end point for the link-local all nodes multicast


not sure I correctly understand this.

If you tell me how you understand it I maybe can clear that up. As is the sentence seems straight forward to me... :-/

if I understand well (I hope I do ;) ), I would rephrase like this :
create a remote end point with the link-local multicast address of all nodes and send

Not completely. It creates a remote end point with the link-local all-nodes multicast address (which is a defined address (ff02::1) and is called as such)

Better now?

yes, thanks for the clarification :)

aabadie · 2016-09-28T13:32:34Z

sys/include/net/sock/ip.h

 *          (remote->netif != SOCK_ADDR_ANY_NETIF))` if neither is `NULL`).
- * @return  -ENOMEM, if the stack can't provide enough resources for `sock` to
- *          be created.
+ * @return  -ENOMEM, if not enough resources for `sock` to be created can be


strange rephrasing ;)

maybe ... not enough resources can be provided for sock to be created ?

aabadie · 2016-09-28T13:38:01Z

sys/include/net/sock/udp.h

+ * uint8_t buf[128];
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * To be able to listen for incoming packets we want to bind the sock so we need


we want to bind the sock so we need, etc
=> we bind the sock by setting a local end point with a port ?

aabadie · 2016-09-28T13:42:38Z

sys/include/net/sock/udp.h

+ *     }
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * We then wait indefinitely for an incoming message in `buf` from `remote`.


I think using 'we' when explaining the behavior of the code should be replaced with something more non personal.
Example:
The program then wait indefinitely for an incoming message...

aabadie · 2016-09-28T13:43:32Z

sys/include/net/sock/udp.h

+ * With `data` being the data send, `data_len` the length of `data` and `remote`
+ * the remote end point the packet is to be sent.
+ *
+ * To see some other utilization we look at a more complex example in form of


See #5884 (comment)

Address RIOT-OS#5884 (review)

miri64 · 2016-09-28T14:16:41Z

Applied requested changes.

kaspar030 · 2016-09-28T14:29:37Z

sys/include/net/sock.h

- * choose to have all sock types have a common base class or use the raw IP
- * sock type to send e.g. UDP packets, while others will keep them
- * completely separate from each other.
+ * Note that there might be no relation between the different `sock` types.


Do you think users might try that?

People tried it with conn so why wouldn't they try it with sock?

kaspar030 · 2016-09-28T14:30:31Z

sys/include/net/sock.h

+ * The actual code very much depends on the used `sock` type. Please refer to
+ * their documentation for specific examples.
+ *
+ * Implementors Notes


Implementors -> implementor?

This is a caption... Why should I remove the capitalization?

kaspar030 · 2016-09-28T15:07:28Z

Implementors Notes

This is a caption... Why should I remove the capitalization?

just remove the plural s at the end of Implementors, capital I is fine.

kaspar030 · 2016-09-28T15:11:22Z

People tried it with conn so why wouldn't they try it with sock?

Someone tried to re-cast the objects into each other?

miri64 · 2016-09-28T15:58:00Z

just remove the plural s at the end of Implementors, capital I is fine.

oops I actually overlooked that.

miri64 · 2016-09-28T16:00:16Z

People tried it with conn so why wouldn't they try it with sock?

Someone tried to re-cast the objects into each other?

Yes some testers get funny ideas when you do not explicitly state that a behavior is forbidden ;-). And actually I can understand that when you come from the socket world. Also: casting sock_udp_t to gnrc_ip_t and vice-versa makes also sense to a degree.

miri64 · 2016-09-28T16:01:11Z

Mind you, they didn't cast a conn that was already established, but used the same type for different operations, after the previous operation was finished.

immesys · 2016-09-29T18:39:29Z

I just tried testing this and I get a missing "sock_types.h' from udp.h:198, I have gnrc_sock_udp and gnrc_ipv6_default included. I also cannot find sock_types.h anywhere in the codebase. Any tips?

immesys · 2016-09-29T18:50:59Z

My humblest apologies, I did not see #5772.

miri64 · 2016-10-04T12:17:17Z

Ping?

aabadie

This looks like a good documentation with good examples.

miri64 · 2016-10-04T14:12:27Z

@aabadie was that an ACK?

aabadie · 2016-10-04T15:05:38Z

@miri64, Murdock is asking for squashing. Then I'll give an ACK.

miri64 · 2016-10-04T15:23:06Z

Squashed.

miri64 · 2016-10-04T15:53:37Z

Murdock is happy 😀

aabadie · 2016-10-04T16:33:21Z

ACK and Go !

miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Area: doc Area: Documentation labels Sep 28, 2016

miri64 assigned cgundogan and kaspar030 Sep 28, 2016

aabadie requested changes Sep 28, 2016

View reviewed changes

miri64 added a commit to miri64/RIOT that referenced this pull request Sep 28, 2016

fixup! sock: doc fixes

7ee47c2

Address RIOT-OS#5884 (review)

kaspar030 reviewed Sep 28, 2016

View reviewed changes

aabadie approved these changes Oct 4, 2016

View reviewed changes

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 4, 2016

sock: doc fixes

f4d3927

miri64 force-pushed the sock/doc/fix branch from deefcc0 to f4d3927 Compare October 4, 2016 15:23

aabadie merged commit b016fb4 into RIOT-OS:master Oct 4, 2016

miri64 deleted the sock/doc/fix branch October 4, 2016 16:33

miri64 mentioned this pull request Oct 4, 2016

pkg: provide sock support for lwIP #5802

Closed

5 tasks

miri64 mentioned this pull request Oct 4, 2016

gnrc_sock: provide port for sock_ip and sock_udp #5772

Merged

miri64 added this to the Release 2016.10 milestone Oct 17, 2016

Conversation

miri64 commented Sep 28, 2016

Uh oh!

miri64 commented Sep 28, 2016

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie Sep 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Sep 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Sep 28, 2016

Uh oh!

kaspar030 commented Sep 28, 2016

Uh oh!

miri64 commented Sep 28, 2016

Uh oh!

miri64 commented Sep 28, 2016

Uh oh!

miri64 commented Sep 28, 2016

Uh oh!

immesys commented Sep 29, 2016

Uh oh!

immesys commented Sep 29, 2016

Uh oh!

miri64 commented Oct 4, 2016

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 commented Oct 4, 2016

Uh oh!

aabadie commented Oct 4, 2016

Uh oh!

miri64 commented Oct 4, 2016

aabadie Sep 28, 2016 •

edited

Loading