drivers/nrf24l01+ : added netdev implementation for nrf24l01+ transceiver #1808

PeterKietzmann · 2014-10-13T14:12:17Z

This PR is based on #1704. It contains a connection between the lowlevel part for the nrf24l01+ transceiver-driver and the netdev interface. I assume there will be several deficits, especially because this device fulfills no standard. So let's discuss this!

PeterKietzmann · 2014-10-13T14:25:24Z

tests/nrf24l01p_test_netdev/main.c

+void print_register(char reg, int num_bytes);
+
+#define STATIC_RX_BUF_SIZE 128
+static char static_rx_buf[STATIC_RX_BUF_SIZE];


Maybe this static receive-buffer can be replaced by the "global packet buffer" in future?

It should. Otherwise it's no use ;-)

miri64 · 2014-10-13T14:40:14Z

I don't have time for a in-depth review right now, but things I've spotted so far:

add your device to drivers/include/netdev/default.h
would it be possible to replace your test application with a slight adaption of tests/netdev? Test applications for every device even though we have a general interface seem a little overkill for me.
Further stuff are line comments…

miri64 · 2014-10-13T14:41:59Z

drivers/include/netdev/base.h

+ *
+ * @detail Sends any frames, handled internal.
+ */
+ NETDEV_PROTO_PROPRIETARY = 0x0008,


Why is it up here? Setting this value to 8 but leaving the lower values below makes the code harder to read, I think.

Also: this name is pretty arbitrary. Should other proprietary protocols also use this protocol type? If yes, what good does this information make? The idea of this type definition was to give a higher layer an idea of what to expect from the device or protocol layer (sorry if this was not clear from the documentation) and just saying "this is basically a black box, deal with it" is not very helpful and opens the door to future device driver development just using this flag.

A better name would be NETDEV_PROTO_NRV24L01X (X for +), I guess. We have room for at least 65536 different protocols here, so don't be shy to be specific. ;-)

mehlis · 2014-10-14T15:18:25Z

drivers/nrf24l01p/include/nrf24l01p_settings.h

+#ifndef __REGS_H
+#define __REGS_H
+
+#define INITIAL_ADDRESS_WIDTH 5


tabs in the middle of the line

PeterKietzmann · 2014-10-15T05:56:18Z

@authmillenon, I started using my implementation with the tests/netdev. Especially for the address setting there are some specialities, if you remember out last (private) mails. So as we don't want to increase the numer of tests, you think it's a reasonable solution to use #ifdef MODULE_NRF24L01p inside the application code to select/unselect the specific parts? Or what was in you mind when you said "replace your test application with a slight adaption of tests/netdev"

miri64 · 2014-10-16T11:19:43Z

drivers/include/netdev/default.h

 #endif /* NETDEV_DEFAULT */
-#endif /* MODULE_NATIVENET */
+#endif /* MODULE_NRF24L01P */


Hey!! :(

diff --git a/drivers/include/netdev/default.h b/drivers/include/netdev/default.h index 50d4983..d044a93 100644 --- a/drivers/include/netdev/default.h +++ b/drivers/include/netdev/default.h @@ -36,4 +36,12 @@ #endif /* NETDEV_DEFAULT */ #endif /* MODULE_NATIVENET */ +#ifdef MODULE_NRF24L01P +#include "nrf24l01p.h" + +#ifndef NETDEV_DEFAULT +#define NETDEV_DEFAULT ((netdev_t *)(&nrf24l01p_netdev)) +#endif /* NETDEV_DEFAULT */ +#endif /* MODULE_NRF24L01P */ + #endif /* __NETDEV_DEFAULT_H_ */

PeterKietzmann · 2014-10-21T15:46:17Z

I adapted the driver and the netdev-test so one can use them together

PeterKietzmann · 2014-10-22T06:37:14Z

Some tests failed for stm32f4discovery board. Any ideas why? Is this maybe because I introduced the feature transceiver for this board?

LudwigKnuepfer · 2014-10-23T08:27:30Z

It does not make sense add the transceiver feature to the board if the transceiver is not part of the board itself.

PeterKietzmann · 2014-10-23T09:10:32Z

Where else can I put it instead? Because transceiver is a required feature for the netdev-test.

LudwigKnuepfer · 2014-10-23T09:13:22Z

That is a conceptional problem. I'll think about it. Maybe @Kijewski wants to do the same.

LudwigKnuepfer · 2014-10-23T09:29:07Z

tests/netdev/main.c

@@ -849,7 +860,7 @@ static int send_packet(void)
 }

 puts("");
-
+ 


trailing whitespace

miri64 · 2014-10-23T12:41:04Z

tests/netdev/main.c

+ uint32_t address = NETDEV_TEST_ADDRESS;
+ int res = dev->driver->set_option(dev, NETDEV_OPT_ADDRESS,
+ &address, sizeof(uint32_t));
+#else


Your driver should handle the case of smaller values, not the application.

I thought we agreed having 4 bytes is fine? What didn't I understand?

If a user sets the option with a 1 byte (or 2 byte, 3 byte maybe but this is ridiculous :-D) number this should be excepted. If the address is 4 byte long in fact only the lowest byte is set [edit] from the 1 byte number and the rest is 0.

LudwigKnuepfer · 2014-10-28T15:25:34Z

@PeterKietzmann #1879 got merged, rebase and refer to it for usage.

This reverts commit cfea86a.

PeterKietzmann · 2015-01-27T14:24:29Z

Hi. Just for clarification: I will follow up with this PR when the network stack is "stable". Maybe the NetworkStackTaskForce will do this :-)

haukepetersen · 2015-01-27T14:56:38Z

Sounds like the only valid plan to me!

PeterKietzmann · 2015-03-03T17:23:43Z

I will close this PR and open a new one when I adapted the device to the new ng_netdev

PeterKietzmann assigned miri64 and haukepetersen and unassigned miri64 and haukepetersen Oct 13, 2014

PeterKietzmann reviewed Oct 13, 2014
View reviewed changes

miri64 reviewed Oct 13, 2014
View reviewed changes

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch from 779da69 to 754756a Compare October 14, 2014 11:44

mehlis reviewed Oct 14, 2014
View reviewed changes

miri64 reviewed Oct 16, 2014
View reviewed changes

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch 2 times, most recently from 8dd9e0a to 235e09d Compare October 21, 2014 15:43

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch from 235e09d to e80efd2 Compare October 22, 2014 05:46

LudwigKnuepfer reviewed Oct 23, 2014
View reviewed changes

tests/netdev/main.c

@@ -849,7 +860,7 @@ static int send_packet(void)

}

puts("");

Copy link

Member

LudwigKnuepfer Oct 23, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing whitespace

miri64 reviewed Oct 23, 2014
View reviewed changes

Kijewski mentioned this pull request Oct 26, 2014

make: fix FEATURES_REQUIRED and FEATURES_PROVIDED a bit #1879

Merged

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch from e76fd90 to 8430555 Compare October 28, 2014 15:36

PeterKietzmann added 6 commits October 29, 2014 08:58

Initial import of nrf24l01p netdev implementation

e2fb3ed

[SQUASH ME] refactored the driver(and test) so the tests/netdev runs

085f324

[SQUASH ME] added newline at end of Makefile.include

9b46b20

[SQUASH ME] removed MODULE transceiver from stm32f4discovery board

628d3da

[SQUASH ME] renamed new basic networxm decive options

ebff3f2

Revert "[SQUASH ME] renamed new basic networxm decive options"

39efa40

This reverts commit cfea86a.

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch from 8430555 to 39efa40 Compare October 29, 2014 08:39

PeterKietzmann added 3 commits October 29, 2014 09:41

[SQUASH ME] added feature transceiver to drivers

f5b044c

[SQUASH ME] added cpp compatible headers

16f4722

[SQUASH ME] corrected preprocessor commands for NETDEV_DEFAULT

43c1a34

PeterKietzmann mentioned this pull request Nov 17, 2014

drivers/nrf24l01+: netdev adaption of the proprietary transceiver #2021

Closed

OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 6, 2015

[SQUASH ME] some unfinal changes

fd3913a

PeterKietzmann force-pushed the add_nrf24l01p_netdev branch from 62c435d to fd3913a Compare January 8, 2015 11:08

PeterKietzmann closed this Mar 3, 2015

PeterKietzmann deleted the add_nrf24l01p_netdev branch June 5, 2015 12:19

fabian18 mentioned this pull request Mar 28, 2020

drivers/nrf24l01p: Netdev driver for nrf24l01p #13743

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/nrf24l01+ : added netdev implementation for nrf24l01+ transceiver #1808

drivers/nrf24l01+ : added netdev implementation for nrf24l01+ transceiver #1808

PeterKietzmann commented Oct 13, 2014

PeterKietzmann Oct 13, 2014

miri64 Oct 14, 2014

miri64 commented Oct 13, 2014

miri64 Oct 13, 2014

miri64 Oct 13, 2014

mehlis Oct 14, 2014

PeterKietzmann commented Oct 15, 2014

miri64 Oct 16, 2014

PeterKietzmann commented Oct 21, 2014

PeterKietzmann commented Oct 22, 2014

LudwigKnuepfer commented Oct 23, 2014

PeterKietzmann commented Oct 23, 2014

LudwigKnuepfer commented Oct 23, 2014

LudwigKnuepfer Oct 23, 2014

miri64 Oct 23, 2014

PeterKietzmann Oct 23, 2014

miri64 Oct 23, 2014

LudwigKnuepfer commented Oct 28, 2014

PeterKietzmann commented Jan 27, 2015

haukepetersen commented Jan 27, 2015

PeterKietzmann commented Mar 3, 2015

drivers/nrf24l01+ : added netdev implementation for nrf24l01+ transceiver #1808

drivers/nrf24l01+ : added netdev implementation for nrf24l01+ transceiver #1808

Conversation

PeterKietzmann commented Oct 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miri64 commented Oct 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterKietzmann commented Oct 15, 2014

Choose a reason for hiding this comment

PeterKietzmann commented Oct 21, 2014

PeterKietzmann commented Oct 22, 2014

LudwigKnuepfer commented Oct 23, 2014

PeterKietzmann commented Oct 23, 2014

LudwigKnuepfer commented Oct 23, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LudwigKnuepfer commented Oct 28, 2014

PeterKietzmann commented Jan 27, 2015

haukepetersen commented Jan 27, 2015

PeterKietzmann commented Mar 3, 2015