[riot-notifications] [RIOT-OS/RIOT] net: add CoRE RD lookup client implementation (#10222)

Hauke Petersen notifications at github.com
Mon May 18 17:06:26 CEST 2020


@haukepetersen requested changes on this pull request.

I just see this one little optimization potential for the dependency declarations, and of course the segfaulting... Else I am ok with proceeding.

> @@ -773,6 +773,15 @@ ifneq (,$(filter cord_ep,$(USEMODULE)))
   endif
 endif
 
+ifneq (,$(filter cord_lc,$(USEMODULE)))
+  USEMODULE += cord_common
+  USEMODULE += core_thread_flags
+  USEMODULE += gcoap

Ok, let me try to illustrate this a little clearer. We might want to do the following:
- move `gcoap` dep into `cord_common` (as it is used by all three, `cord_epsim`, `cord_ep`, and `cord_lc`)
- factor out the shared modules that are deps for both `cord_ep` and `cord_lt`, something like this:
```
ifneq (,$(filter cord_lc,$(USEMODULE)))
  USEMODULE += clif
endif

ifneq (,$(filter cord_lc cord_ep,$(USEMODULE)))
  USEMODULE += core_thread_flags
  USEMODULE += cord_common
  ifneq (,$(filter shell_commands,$(USEMODULE)))
    USEMODULE += sock_util
  endif
endif  

ifneq (,$(filter cord_common,$(USEMODULE)))
  USEMODULE += gcoap
  USEMODULE += fmt
  USEMODULE += luid
endif
```

This way there is not duplicate deps declaration for those modules anymore.

> +static size_t _res_buf_len;
+
+static char _rd_lookif[NANOCOAP_URI_MAX];
+
+static mutex_t _mutex = MUTEX_INIT;
+static volatile thread_t *_waiter;
+
+static uint8_t buf[BUFSIZE];
+
+static void _lock(void)
+{
+    mutex_lock(&_mutex);
+    _waiter = sched_active_thread;
+}
+
+static int _sync(void)

It would still be nice not to duplicate this code, but then again I don't see this as a show stopper for now. So lets postpone this step until after this PR is merged...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10222#pullrequestreview-413677971
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200518/7e383f42/attachment.htm>


More information about the notifications mailing list