[riot-notifications] [RIOT-OS/RIOT] cpu/nrf52/radio: initial support for nrf52's ieee802.15.4 radio (#10268)

Hauke Petersen notifications at github.com
Fri Mar 1 15:33:43 CET 2019


haukepetersen requested changes on this pull request.

There is always something :-)

Tested this a little bit against my local border router, and havn't found any faults, everything runs as expected when doing flood pings, large packets, etc. So IMO just these minor cosmetic fixes and we can merge!

> @@ -0,0 +1,7 @@
+ifneq (,$(filter nrf802154,$(USEMODULE)))
+  FEATURES_REQUIRED += periph_timer
+  FEATURES_REQUIRED += radio_nrf802154
+  USEMODULE += netif

If I am not mistaken, the driver does not have any dependency into `netif`, right?

> @@ -7,5 +7,7 @@ export MCUBOOT_SLOT0_SIZE = 0x8000
 export MCUBOOT_SLOT1_SIZE = 0x3C000
 export MCUBOOT_SLOT2_SIZE = 0x3C000
 
+CFLAGS += -DGNRC_NETIF_MSG_QUEUE_SIZE=16

Does the driver really require a larger message queue than the default value (`8`)? If so, I strongly suggest to move this define somehwere into the scope of the radio driver. E.g. we add it to a new `cpu/nrf52/radio/nrf802154/Makefile.include` file, and include this file here on the condition, that the driver is actually included in the build. 

If we'd leave this as is, we would increase the queue size for any driver build with this board, even when not using the `nrf802154` code.

Also, better make it `CFLAGS ?= -DGNRC_...`, else it would clash if this value is changed somewhere else (e.g. from the application makefile...)

> + * - NETDEV_EVENT_RX_COMPLETE
+ * - NETDEV_EVENT_TX_COMPLETE
+ *
+ * Transmission options not yet impemented:
+ * - Send acknowledgement for packages
+ * - Request acknowledgement
+ * - Retransmit unacked packages
+ * - Carrier Sense Multiple Access (CSMA) and Implementation of Clear Channel
+ *   Assessment Control (CCACTRL)
+ *
+ * @{
+ *
+ * @file
+ * @brief       Driver interface for using the nRF52 in IEEE802.15.4 mode
+ *
+ * @author      Hauke Petersen <hauke.petersen at fu-berlin.de>

Semjon Kerner missing here :-)

> +extern "C" {
+#endif
+
+/**
+ * @brief   Export the netdev device descriptor
+ */
+extern netdev_ieee802154_t nrf802154_dev;
+
+/**
+ * @brief   IEEE 802.15.4 radio timer configuration
+ *
+ *          this radio relies on a dedicated hardware timer to maintain IFS
+ *          the default timer may be overwritten in the board configuration
+ */
+#ifndef NRF_IEEE802154_TIMER
+#define NRF_IEEE802154_TIMER TIMER_DEV(1)

for consistency, I'd prefer `NRF802154_TIMER` as a name of this define, as that is the actual module name...

> +    .long_addr = { 0, 0, 0, 0, 0, 0, 0, 0 },
+    .chan = IEEE802154_DEFAULT_CHANNEL,
+    .flags = 0
+};
+
+static uint8_t rxbuf[IEEE802154_FRAME_LEN_MAX + 3]; /* len PHR + PSDU + LQI */
+static uint8_t txbuf[IEEE802154_FRAME_LEN_MAX + 3]; /* len PHR + PSDU + LQI */
+
+#define RX_COMPLETE         (0x1)
+#define TX_COMPLETE         (0x2)
+#define LIFS                (40U)
+#define SIFS                (12U)
+#define SIFS_MAXPKTSIZE     (18U)
+#define TIMER_FREQ          (250000UL)
+static volatile uint8_t _state;
+static mutex_t txlock;

nitpicky: shouldn't this variable also start with an underscore?

-- 
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/10268#pullrequestreview-209562998
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190301/d9e0580d/attachment.html>


More information about the notifications mailing list