[riot-notifications] [RIOT-OS/RIOT] pkg/semtech-loramac: rework TX/RX interaction with the MAC (#11541)

Francisco notifications at github.com
Thu May 23 17:42:40 CEST 2019


fjmolinas requested changes on this pull request.

Some initial comments.


> @@ -839,7 +837,7 @@ uint8_t semtech_loramac_send(semtech_loramac_t *mac, uint8_t *data, uint8_t len)
         return SEMTECH_LORAMAC_NOT_JOINED;
     }
 
-    /* Correctly set the caller pid */
+    /* Correctly set the sender thread pid */

I think the comment should be inline with the variable names.

> @@ -110,6 +110,7 @@ typedef struct {
 typedef struct {
     mutex_t lock;                                /**< loramac access lock */
     uint8_t caller_pid;                          /**< pid of caller thread */

Maybe it makes more sense if this is `tx_pid` if I understand right this thread is in charge of join procedure and sending packets, so tx. Would be more inline with rx_pid (receiver thread)

>  #include "msg.h"
 #include "shell.h"
 #include "fmt.h"
 
 #include "net/loramac.h"
 #include "semtech_loramac.h"
 
+#define LORAMAC_RECV_MSG_QUEUE                   (4U)

Why 4? Could it be less? Does it need to be more?

> @@ -95,12 +95,6 @@
  *         return 1;
  *     }
  *
- *     /* 5. wait for any potentially received data */

Document starting and usage of a receiving thread.

-- 
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/11541#pullrequestreview-241269835
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190523/ff1d5af1/attachment.html>


More information about the notifications mailing list