-
Notifications
You must be signed in to change notification settings - Fork 2.1k
pkg/openwsn: re-integrate the network stack as a package #8570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Useful hint no 1: For easier debugging I'm using a local copy of the adapted
|
Useful hint no 2: RIOT provides some build commands to easily submit experiments to the FIT IoT.LAB testbed. For documentation see here. TL;DR: From the test application directory use this command to build, submit an experiment with three nodes and start a tmux session to the aggregated serial output of that experiment: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some preliminary comments.
|
||
void eui64_get(uint8_t *addressToWrite) | ||
{ | ||
luid_get((void *)addressToWrite, IEEE802154_LONG_ADDRESS_LEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will get you a different EUI-64 everytime the function is called or is this intended (couldn't find any documentation, on this but I saw it is called twice here)?
If not, why not get the long-address from the device somehow? E.g. have
netdev_t *openwsn_netdev;
void eui64_get(uint8_t *addressToWrite)
{
eui64_t eui64;
if (openwsn_netdev->driver->get(dev, NETOPT_ADDRESS_LONG, &eui64, sizeof(eui64_t)) == sizeof(eui64_t)) {
memcpy(addressToWrite, eui64.u8, sizeof(eui64.u8));
}
else {
/* Use your code as a fallback or print an error message here */
luid_get((void *)addressToWrite, IEEE802154_LONG_ADDRESS_LEN);
}
}
And set netdev in radio_init()
:
extern netdev_t *openwsn_netdev;
void radio_init(void)
{
/* ... */
#ifdef MODULE_AT86RF2XX
openwsn_netdev = (netdev_t *)&at86rf2xx_dev.netdev.netdev;
radio_vars.dev = openwsn_netdev;
at86rf2xx_setup(&at86rf2xx_dev, &at86rf2xx_params[0]);
#endif
If you fear memory waste, take netdev
out of radio_vars_t
and use openwsn_netdev
instead of radio_vars.dev
.
/cc @fjmolinas
#define RTT_FREQUENCY (1) /* in Hz */ | ||
#define RTT_PRESCALER (0x7fff) /* run with 1 Hz */ | ||
#define RTT_FREQUENCY (16384U) /* in Hz */ | ||
#define RTT_PRESCALER (0x1) /* run with ~16 kHz */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably still a TODO, but these should be configured at compile-time, when OpenWSN is present.
@@ -147,6 +147,9 @@ void at86rf2xx_tx_prepare(at86rf2xx_t *dev) | |||
if (state != AT86RF2XX_STATE_TX_ARET_ON) { | |||
dev->idle_state = state; | |||
} | |||
|
|||
at86rf2xx_set_state(dev, AT86RF2XX_STATE_PLL_ON); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably also a TODO, but hard-wiring states required for OpenWSN seems like a bad idea to me.
@@ -213,7 +213,7 @@ static int _set_state(at86rf2xx_t *dev, netopt_state_t state) | |||
* know when to switch back to the idle state. */ | |||
++dev->pending_tx; | |||
} | |||
at86rf2xx_set_state(dev, AT86RF2XX_STATE_TX_ARET_ON); | |||
at86rf2xx_set_state(dev, AT86RF2XX_STATE_PLL_ON); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito.
} else { | ||
DEBUG("[at86rf2xx] Unhandled TRAC_STATUS: %d\n",trac_status >> 5); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to separate PR?
netdev_t *dev; | ||
} radio_vars_t; | ||
|
||
radio_vars_t radio_vars; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't exposed, so static
?
396db24
to
1030668
Compare
The above mentioned issues have been solved. Furthermore, I squashed and rebased both RIOT as well as openwsn. Help in form of reviewing and testing is happily welcome. There is still work TODO which can be taken from the doc.txt. Most importantly:
|
Can #9592 help with that? |
Able to send a simple UDP packet on IoT-LAB nodes using the test application. It's a start! FWIW, unable to compile with CFLAGS=-DOW_RIOT_SINGLE_CHANNEL=26. Lots of errors. But if channel hopping works, that's fine with me. Next I'll see if I can use OpenWSN as a sock-like provider for nanocoap.
udp server start 61617
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I had these review comments never commited and they seem to be in here for a while now ...
void sctimer_setCompare(uint32_t val) | ||
{ | ||
DEBUG("sctimer_setCompare\n"); | ||
// enable the compare interrupt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment style
DEBUG("sctimer_setCompare\n"); | ||
// enable the compare interrupt | ||
/* | ||
if (current Timer counter - val < TIMERLOOP_THRESHOLD){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed, I guess
} | ||
|
||
/** | ||
\brief set compare interrupt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doxygen style
From 0c967954f444044cbc5c51d25230a5ca6cfa1383 Mon Sep 17 00:00:00 2001 | ||
From: PeterKietzmann <peter.kietzmann@haw-hamburg.de> | ||
Date: Thu, 8 Feb 2018 10:19:38 +0100 | ||
Subject: [PATCH 1/6] Add Makefiles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at the current lwIP package. Adding makefiles is now possible without a patch ;-).
From 587609720250906a6ae5b6d5bd5c1daca2dd3349 Mon Sep 17 00:00:00 2001 | ||
From: PeterKietzmann <peter.kietzmann@haw-hamburg.de> | ||
Date: Thu, 8 Feb 2018 11:25:18 +0100 | ||
Subject: [PATCH 3/6] Comment out problematic includes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are they problematic? A short reasoning in the description might be beneficial
Ping @PeterKietzmann? |
Thanks for the feedback, I'll address what I can. There was some "major" reconstruction and added features to this work which is why I'm closing the PR for now. Will re-open once ready. |
Contribution description
This PR re-integrates the OpenWSN network stack as a pkg to RIOT. It is marked as WIP because it does not yet work stable and I'm asking for your help in terms of debugging, testing and review.
An example application is provided, including documentation in the README next to it. Furthermore, I added documentation about the port in general, the status of this implementation as well as additional information about debugging capabilities and currently open issues/TODOs. The two commits covering Atmel radio driver changes and iotlab-m3 timer configuration are not intended to be merged but rather to be included as a patch later.
I have a collection of additional test applications for different wrappers lying around, which I would provide in a separate PR if we agree on how to integrate them into the repository.
Implementation Issues
- Nodes reset after some time (here?). Without channel hopping this effect does not show up that often. With channel hopping it gets worse- After a node restarts, it won't join the topology any time soon, even though neighbored nodes still send DIOsPeople that might be interested and in charge to support
(alphabetical order of last name)
@emmanuelsearch, @kb2ma, @changtengfei, @OlegHahm, @sieben, @fjmolinas, @Lotterleben, @twatteyne
Issues/PRs references
Relates to #3039 and #7893