pkg: provide sock_udp support for lwip by miri64 · Pull Request #5937 · RIOT-OS/RIOT

miri64 · 2016-10-12T23:06:59Z

This was taken out of #5802 and only provides the sock_udp part and the tests for it.

This contains duplicate changes to #5936, so merging any of these might require rebase.

miri64 · 2016-10-31T12:08:45Z

Needs adaptation to #5945.

kaspar030 · 2016-11-18T11:45:23Z

No time, reassigning.

miri64 · 2016-11-18T13:33:09Z

Adaption to #6137 (comment) also needed...

kYc0o · 2016-11-29T17:27:02Z

What's the latest on this?

miri64 · 2016-11-29T19:28:51Z

Rebased and made adaptation.

miri64 · 2017-01-13T16:07:18Z

Ping?

miri64 · 2017-01-17T12:39:17Z

Rebased and squashed to latest master.

miri64 · 2017-01-24T14:33:57Z

Squashed

miri64 · 2017-01-24T15:34:33Z

Added nucleo32-f042 to boards that do not have sufficient memory and squashed immediately.

miri64 · 2017-01-24T15:49:57Z

Jenkin's happy. I'm adding Murdock :-)

PeterKietzmann · 2017-01-25T08:54:29Z

I guess it's known that tests/lwip and examples/gnrc_networking is not interoperable?

PeterKietzmann · 2017-01-25T09:07:08Z

On a phytec board I don't get an IPv6 address. ps() crashes the node but this is probably an issue of #6372.

kYc0o · 2017-01-26T13:18:23Z

Well I think for interoperability lwip is using IPv4 and gnrc_networking IPv6 isn't it? @miri64 ?

miri64 · 2017-01-26T13:34:36Z

I guess it's known that tests/lwip and examples/gnrc_networking is not interoperable?

Yes, there is a bug in lwIP preventing this

Well I think for interoperability lwip is using IPv4 and gnrc_networking IPv6 isn't it? @miri64 ?

Huh? lwIP also is able to speak IPv6, but its not parsing IEEE 802.15.4 headers correctly.

miri64 · 2017-01-26T13:35:34Z

@PeterKietzmann are the extensive tests in tests/lwip_sock_udp working?

PeterKietzmann · 2017-01-26T14:02:03Z

As said, tests/lwip_sock_udp and with #6372 and tests/lwip I was able to send local udp packets between two atmel nodes. ps() crashes nodes (more related to #6372. I guess) and data transmission between GNRC and LWIP does not work (known issue). I just scanned over the code which looked ok at the first sight. I hoped to get @kYc0o's opinion about this PR and merging. BTW: I will announce feature freeze soooon

kYc0o

Just some minor comments, I'll test and then ACK.

kYc0o · 2017-02-07T13:59:20Z

pkg/lwip/contrib/sock/udp/lwip_sock_udp.c

+
+ssize_t sock_udp_recv(sock_udp_t *sock, void *data, size_t max_len,
+                      uint32_t timeout, sock_udp_ep_t *remote)
+{


Why ssize_t? As far as I can see you're returning int and at the end you cast another int to this type...

This is unrelated to this PR. If you want to change the sock API, please do so in a separate PR. Otherwise I can't change it, because the API demands it.

OK I didn't know that the sock API forced that type. Then for this PR it's OK.

Please don't change the API again :-(

kYc0o · 2017-02-07T14:02:01Z

tests/lwip_sock_udp/stack.c

+#define _MSG_QUEUE_SIZE     (1)
+#define _SEND_DONE  (0x92d7)
+#define _NETDEV_BUFFER_SIZE     (128)
+


Maybe these could be aligned?

kYc0o · 2017-02-07T14:10:40Z

Tested with success on native.

miri64 · 2017-02-07T14:37:04Z

Adressed comments

kYc0o · 2017-02-07T14:39:39Z

Then everything is OK, you squash and merge when Murdock agrees.

kYc0o

ACK

PeterKietzmann · 2017-02-07T14:42:05Z

@kYc0o did you experience the same problems with ps() on native as I described above?

kYc0o · 2017-02-07T14:44:08Z

I'll check it asap, since I just ran make test and all the tests passed.

miri64 · 2017-02-07T14:49:29Z

Squashed

@kYc0o did you experience the same problems with ps() on native as I described above?

@PeterKietzmann I highly doubt this is related to this PR.

kYc0o · 2017-02-07T14:52:33Z

@PeterKietzmann I don't find a way to test ps() since the only example provided is the test, which just call and test the functions, I don't have access to any shell... You want me to modify the test to call ps() at the end?

miri64 · 2017-02-07T14:57:01Z

@kYc0o Peter was refering to the test provided in #6372.

PeterKietzmann · 2017-02-07T14:57:17Z

As said, tests/lwip_sock_udp and with #6372 and tests/lwip I was able to send local udp packets between two atmel nodes. ps() crashes nodes (more related to #6372. I guess) and data transmission between GNRC and LWIP does not work (known issue).

@miri64, @kYc0o I've tested with #6372 and I would also assume the problem in there. However, I intended to compare test results. We can continue the discussion in there.

miri64 · 2017-02-07T16:34:28Z

Yapp, but are you okay with merging this one for now?

kYc0o · 2017-02-07T22:29:12Z

I'd say merge everything necessary for #6732 and test there, if there is something pointing here then we can find it easily I think.

So, merge at will.

PeterKietzmann · 2017-02-08T08:43:18Z

@kYc0o did I scare you to hit the button :-) ? I wanted to motivate more detailed testing. However, I'm fine with merging this one now but please provide help in #6732.

miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 12, 2016

miri64 assigned kaspar030 Oct 12, 2016

miri64 mentioned this pull request Oct 12, 2016

pkg: provide sock support for lwIP #5802

Closed

5 tasks

kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 13, 2016

miri64 mentioned this pull request Oct 14, 2016

sock: change IPv4 address type to array #5945

Merged

miri64 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 5, 2016

kaspar030 assigned kYc0o and unassigned kaspar030 Nov 18, 2016

miri64 force-pushed the pkg/feat/lwip-sock-udp branch from 5a1e6b2 to 05f6a12 Compare November 29, 2016 19:28

miri64 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Nov 29, 2016

This was referenced Nov 29, 2016

pkg: provide sock_ip support for lwip #5936

Merged

pkg: provide sock_tcp support for lwip #5938

Merged

miri64 mentioned this pull request Jan 13, 2017

posix_sockets: port to sock #6004

Merged

miri64 mentioned this pull request Jan 15, 2017

tests: port tests/lwip to sock #6372

Merged

miri64 force-pushed the pkg/feat/lwip-sock-udp branch 2 times, most recently from 5125e2b to 0be4ebb Compare January 17, 2017 12:39

miri64 force-pushed the pkg/feat/lwip-sock-udp branch 2 times, most recently from ef282dc to 0be4ebb Compare January 17, 2017 12:45

miri64 force-pushed the pkg/feat/lwip-sock-udp branch from a2c8f03 to 1f83319 Compare January 24, 2017 14:33

miri64 force-pushed the pkg/feat/lwip-sock-udp branch from 1f83319 to e801839 Compare January 24, 2017 15:34

miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 24, 2017

kYc0o requested changes Feb 7, 2017

View reviewed changes

kYc0o approved these changes Feb 7, 2017

View reviewed changes

pkg: provide sock_udp support for lwip

a45256a

miri64 force-pushed the pkg/feat/lwip-sock-udp branch from 83407ea to a45256a Compare February 7, 2017 14:48

PeterKietzmann merged commit e5e79c2 into RIOT-OS:master Feb 8, 2017

miri64 deleted the pkg/feat/lwip-sock-udp branch February 8, 2017 09:04

Conversation

miri64 commented Oct 12, 2016

Uh oh!

miri64 commented Oct 31, 2016

Uh oh!

kaspar030 commented Nov 18, 2016

Uh oh!

miri64 commented Nov 18, 2016

Uh oh!

kYc0o commented Nov 29, 2016

Uh oh!

miri64 commented Nov 29, 2016

Uh oh!

miri64 commented Jan 13, 2017

Uh oh!

miri64 commented Jan 17, 2017

Uh oh!

miri64 commented Jan 24, 2017

Uh oh!

miri64 commented Jan 24, 2017

Uh oh!

miri64 commented Jan 24, 2017

Uh oh!

PeterKietzmann commented Jan 25, 2017

Uh oh!

PeterKietzmann commented Jan 25, 2017

Uh oh!

kYc0o commented Jan 26, 2017

Uh oh!

miri64 commented Jan 26, 2017

Uh oh!

miri64 commented Jan 26, 2017

Uh oh!

PeterKietzmann commented Jan 26, 2017

Uh oh!

kYc0o left a comment

Choose a reason for hiding this comment

Uh oh!

kYc0o Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

miri64 Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

kYc0o Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

PeterKietzmann Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

kYc0o Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

kYc0o commented Feb 7, 2017

Uh oh!

miri64 commented Feb 7, 2017

Uh oh!

kYc0o commented Feb 7, 2017

Uh oh!

kYc0o left a comment

Choose a reason for hiding this comment

Uh oh!

PeterKietzmann commented Feb 7, 2017

Uh oh!

kYc0o commented Feb 7, 2017

Uh oh!

miri64 commented Feb 7, 2017

Uh oh!

kYc0o commented Feb 7, 2017

Uh oh!

miri64 commented Feb 7, 2017

Uh oh!

PeterKietzmann commented Feb 7, 2017

Uh oh!

miri64 commented Feb 7, 2017

Uh oh!

kYc0o commented Feb 7, 2017

Uh oh!

PeterKietzmann commented Feb 8, 2017

Uh oh!