Skip to content

sock_async: initial import of msg-based implementation#12601

Closed
miri64 wants to merge 3 commits intoRIOT-OS:masterfrom
miri64:sock_async_msg/feat/initial
Closed

sock_async: initial import of msg-based implementation#12601
miri64 wants to merge 3 commits intoRIOT-OS:masterfrom
miri64:sock_async_msg/feat/initial

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented Oct 29, 2019

Contribution description

This provides a core_msg-based front-end implementation for sock_async (see #11723).

Testing procedure

There is no back-end implementation yet, so just read.

Issues/PRs references

Requires #11723 (merged) and #12921 (merged).

@miri64 miri64 added Area: network Area: Networking Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: waiting for other PR State: The PR requires another PR to be merged first State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. labels Oct 29, 2019
@miri64 miri64 force-pushed the sock_async_msg/feat/initial branch from 7b3b03c to 207d54a Compare December 6, 2019 15:55
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 6, 2019
@miri64
Copy link
Member Author

miri64 commented Dec 6, 2019

Rebased and adapted to current master.

@miri64
Copy link
Member Author

miri64 commented Dec 11, 2019

Added #12921 as a dependency for the cyclic header dependency problem (see #12602 (comment)).

@miri64 miri64 force-pushed the sock_async_msg/feat/initial branch from 207d54a to 389289b Compare December 11, 2019 09:36
@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 11, 2019
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 9, 2020
@miri64 miri64 force-pushed the sock_async_msg/feat/initial branch from 389289b to e6faccc Compare January 9, 2020 11:25
@miri64
Copy link
Member Author

miri64 commented Jan 9, 2020

Rebased. No longer waiting for another PR.

@fjmolinas
Copy link
Contributor

@miri64 why is this one a demostrator and #12602 not?

@miri64
Copy link
Member Author

miri64 commented Jan 14, 2020

Because this one does not really need to be merged and just serves as a demonstration for an alternate approach to #12602.

@miri64 miri64 force-pushed the sock_async_msg/feat/initial branch from e6faccc to f00d13a Compare June 24, 2020 10:05
@miri64
Copy link
Member Author

miri64 commented Jun 24, 2020

Because this one does not really need to be merged and just serves as a demonstration for an alternate approach to #12602.

Though this upholds, I put some work into aligning it with the current state in master:

  • Rebased to master (duh)
  • Applied same abstraction scheme we established in sock_async_event (a callback of the same signature as the low-level callback needs to be registered on initialization which is called by an encapsulated handler)
  • Updated doc accordingly
  • Added a test application (which apart minor adaptations for sock_async_msg is a copy of tests/gnrc_sock_async_event) (that's the nice thing about the aforementioned abstraction scheme)
diff -ru tests/gnrc_sock_async_event/ tests/gnrc_sock_async_msg/
diff -ru tests/gnrc_sock_async_event/main.c tests/gnrc_sock_async_msg/main.c
--- tests/gnrc_sock_async_event/main.c	2020-06-24 11:53:11.522976048 +0200
+++ tests/gnrc_sock_async_msg/main.c	2020-06-24 11:45:41.749486218 +0200
@@ -18,7 +18,7 @@
  */
 
 #include <stdio.h>
-#include "event.h"
+#include "msg.h"
 #include "net/ipv6/addr.h"
 #include "net/ipv6/hdr.h"
 #include "net/sock/ip.h"
@@ -32,10 +32,12 @@
 #include "test_utils/expect.h"
 #include "thread.h"
 
-#include "net/sock/async/event.h"
+#include "net/sock/async/msg.h"
 
 #define RECV_BUFFER_SIZE        (128)
 
+#define MSG_QUEUE_SIZE          (4U)
+
 #define TEST_PORT               (38664U)
 #define TEST_LOCAL              { 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
                                   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 }
@@ -50,7 +52,6 @@
 static gnrc_netreg_entry_t _pktdump;
 
 static char _addr_str[IPV6_ADDR_MAX_STR_LEN];
-static event_queue_t _ev_queue;
 static uint8_t _buffer[128];
 static sock_ip_t _ip_sock;
 static sock_udp_t _udp_sock;
@@ -118,8 +119,10 @@
     gnrc_pktsnip_t *pkt;
     sock_udp_ep_t local = SOCK_IPV6_EP_ANY;
     sock_udp_ep_t remote = SOCK_IPV6_EP_ANY;
+    msg_t msgq[MSG_QUEUE_SIZE];
+
+    msg_init_queue(msgq, ARRAY_SIZE(msgq));
 
-    event_queue_init(&_ev_queue);
     /* register for IPv6 to have a target */
     gnrc_netreg_entry_init_pid(&_pktdump, GNRC_NETREG_DEMUX_CTX_ALL,
                                gnrc_pktdump_pid);
@@ -129,8 +132,8 @@
     sock_udp_create(&_udp_sock, &local, NULL, 0);
     sock_ip_create(&_ip_sock, (sock_ip_ep_t *)&local, NULL, PROTNUM_UDP, 0);
 
-    sock_udp_event_init(&_udp_sock, &_ev_queue, _recv_udp, "test");
-    sock_ip_event_init(&_ip_sock, &_ev_queue, _recv_ip, "test");
+    sock_udp_msg_init(&_udp_sock, thread_getpid(), _recv_udp, "test");
+    sock_ip_msg_init(&_ip_sock, thread_getpid(), _recv_ip, "test");
     memcpy(remote.addr.ipv6, _test_remote, sizeof(_test_remote));
     remote.port = TEST_PORT - 1;
 
@@ -159,7 +162,15 @@
     gnrc_netapi_dispatch_receive(GNRC_NETTYPE_UDP, TEST_PORT, pkt);
     /* trigger receive on IP sock */
     gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, PROTNUM_UDP, pkt);
-    event_loop(&_ev_queue);
+    while (true) {
+        msg_t msg;
+
+        msg_receive(&msg);
+
+        if (sock_async_msg_is(&msg)) {
+            sock_async_msg_handler(&msg);
+        }
+    }
     return 0;
 }
 
diff -ru tests/gnrc_sock_async_event/Makefile tests/gnrc_sock_async_msg/Makefile
--- tests/gnrc_sock_async_event/Makefile	2020-06-24 11:53:11.522976048 +0200
+++ tests/gnrc_sock_async_msg/Makefile	2020-06-24 10:58:30.430949928 +0200
@@ -6,7 +6,7 @@
 USEMODULE += gnrc_sock_async
 USEMODULE += gnrc_sock_ip
 USEMODULE += gnrc_sock_udp
-USEMODULE += sock_async_event
+USEMODULE += sock_async_msg
 USEMODULE += od
 USEMODULE += xtimer
 
diff -ru tests/gnrc_sock_async_event/tests/01-run.py tests/gnrc_sock_async_msg/tests/01-run.py
--- tests/gnrc_sock_async_event/tests/01-run.py	2020-06-24 11:53:11.522976048 +0200
+++ tests/gnrc_sock_async_msg/tests/01-run.py	2020-06-24 11:50:26.155102207 +0200
@@ -11,14 +11,16 @@
 
 
 def testfunc(child):
-    child.expect_exact("UDP event triggered: 0030")
+    child.expect_exact("UDP event triggered: 0020")
+    child.expect_exact("UDP message successfully sent")
+    child.expect_exact("IP event triggered: 0020")
+    child.expect_exact("IP message successfully sent")
+    child.expect_exact("UDP event triggered: 0010")
     child.expect_exact("Received UDP packet from [fe80::2]:38663:")
     child.expect_exact("00000000  01  23  45  67  89  AB  CD  EF")
-    child.expect_exact("UDP message successfully sent")
-    child.expect_exact("IP event triggered: 0030")
+    child.expect_exact("IP event triggered: 0010")
     child.expect_exact("Received IP packet from [fe80::2]:")
     child.expect_exact("00000000  01  23  45  67  89  AB  CD  EF")
-    child.expect_exact("IP message successfully sent")
 
 
 if __name__ == "__main__":

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@miri64 miri64 force-pushed the sock_async_msg/feat/initial branch from f00d13a to 44bc1a3 Compare May 24, 2022 09:46
@miri64 miri64 requested a review from maribu as a code owner May 24, 2022 09:46
@github-actions github-actions bot added Area: sys Area: System Area: tests Area: tests and testing framework labels May 24, 2022
@maribu
Copy link
Member

maribu commented May 24, 2022

Summing up what I said on matrix: It is nice that this PR is there, as adding implementations and users of an API is the best way to expose rough edges and limitations. Hence, I am grateful that this PR was created.

However, I personally think with #12602 merged we don't need an alternative implementation merged.

@miri64
Copy link
Member Author

miri64 commented May 24, 2022

However, I personally think with #12602 merged we don't need an alternative implementation merged.

At least not one that is less useful that sock_async_event ;-). Ok, I will close this one then.

@miri64 miri64 closed this May 24, 2022
@miri64 miri64 deleted the sock_async_msg/feat/initial branch May 24, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants