Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
19402: sys/net/gnrc/netif: fixing no global address wait r=benpicco a=jan-mo

### Contribution description

The function `gnrc_netif_ipv6_wait_global_address()` will always return true, even if no global address is attached to the interface.
Currently the function only waits for any message and does not check if it was from the bus or not. So in `msg.content.ptr` is no valid address and therefore it returns true.

I added just the check, if the message is from the bus of any interface and then checking the address. We could also first check if the address in `msg.content.ptr` is valid, but this will just hide the bug. Also the timeout was never checked. It was just assuming that no other message will be received during the wait.


### Testing procedure

Use two devices, one works as a border router and supports the global address, the other will wait for the global address. You can call the function `gnrc_netif_ipv6_wait_global_address()` on the waiting node and see whether it returns true and finds the global address in the given time-range.


19404: sys/trickle: cleanup deps r=benpicco a=MrKevinWeiss

### Contribution description

Cleans the dependencies of the `trickle` module. This removes the deprecated xtimer and models kconfig.

### Testing procedure

Green murdock

### Issues/PRs references


19405: cpu/efm32: pwm_init errors are zeros r=benpicco a=chrysn

### Contribution description

pwm_init is documented to return 0 on errors, and has an unsigned return value.

EFM32's initialization function returned negative values, which through implicit casting become 0xffffffff or 0xfffffffe, which are successful by the documentation.

This makes all the EFM32 error paths return 0 as they should.

Also, it fixes a variable name and the corresponding error message that used to talk of "initiation" (which would be the start of a process) rather than "initialization" (which is a process that is completed before something else can happen).

### Testing procedure

* on stk3700, tests/periph_pwm, run `init 0 0 10 1000` / `set 0 0 500`
* The init used to respond with "The pwm frequency is set to 4294967294", and the set does nothing.
* The init now responds with "Error: device is not <del>initiated</del><ins>initialized</ins>". The set still does nothing, but then one doesn't expect it to any more.

(But really, looking at the patch and the docs should suffice).

### Issues/PRs references

By-catch from testing the Rust wrappers provided by `@remmirad` at RIOT-OS/rust-riot-wrappers#38

Co-authored-by: Jan Mohr <[email protected]>
Co-authored-by: MrKevinWeiss <[email protected]>
Co-authored-by: chrysn <[email protected]>
  • Loading branch information
4 people committed Mar 17, 2023
4 parents c4400e8 + 72107a0 + bdd8819 + 6c8d1eb commit 2fcedf9
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 48 deletions.
4 changes: 2 additions & 2 deletions cpu/efm32/periph/pwm.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ uint32_t pwm_init(pwm_t dev, pwm_mode_t mode, uint32_t freq, uint16_t res)
{
/* check if device is valid */
if (dev >= PWM_NUMOF) {
return -1;
return 0;
}

/* enable clocks */
Expand All @@ -46,7 +46,7 @@ uint32_t pwm_init(pwm_t dev, pwm_mode_t mode, uint32_t freq, uint16_t res)
freq_timer);

if (prescaler > timerPrescale1024) {
return -2;
return 0;
}

/* reset and initialize peripheral */
Expand Down
1 change: 1 addition & 0 deletions sys/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ rsource "test_utils/Kconfig"
rsource "timex/Kconfig"
rsource "tiny_strerror/Kconfig"
rsource "trace/Kconfig"
rsource "trickle/Kconfig"
rsource "tsrb/Kconfig"
rsource "uri_parser/Kconfig"
rsource "usb/Kconfig"
Expand Down
7 changes: 1 addition & 6 deletions sys/Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,7 @@ endif

ifneq (,$(filter trickle,$(USEMODULE)))
USEMODULE += random
ifeq (,$(filter ztimer_msec,$(USEMODULE)))
USEMODULE += xtimer
ifneq (,$(filter ztimer_xtimer_compat,$(USEMODULE)))
USEMODULE += ztimer_msec
endif
endif
USEMODULE += ztimer_msec
endif

ifneq (,$(filter eui_provider,$(USEMODULE)))
Expand Down
9 changes: 0 additions & 9 deletions sys/include/trickle.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@
#endif

#include "thread.h"
#if IS_USED(MODULE_ZTIMER_MSEC)
#include "ztimer.h"
#else
#include "xtimer.h"
#endif

/**
* @brief Trickle callback function with arguments
Expand All @@ -63,13 +59,8 @@ typedef struct {
trickle_callback_t callback; /**< callback function and parameter that
trickle calls after each interval */
msg_t msg; /**< the msg_t to use for intervals */
#if IS_USED(MODULE_ZTIMER_MSEC)
ztimer_t msg_timer; /**< timer to send a msg_t to the target
thread for a new interval */
#else
xtimer_t msg_timer; /**< xtimer to send a msg_t to the target
thread for a new interval */
#endif
} trickle_t;

/**
Expand Down
31 changes: 25 additions & 6 deletions sys/net/gnrc/netif/gnrc_netif.c
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,15 @@ static void _netif_bus_detach(gnrc_netif_t *netif, msg_bus_entry_t *sub)
msg_bus_detach(bus, sub);
}

static void _netif_bus_msg_global(gnrc_netif_t *netif, msg_t* m, bool *has_global)
{
msg_bus_t* bus = gnrc_netif_get_bus(netif, GNRC_NETIF_BUS_IPV6);
if (msg_is_from_bus(bus, m) && ipv6_addr_is_global(m->content.ptr)) {
DEBUG_PUTS("gnrc_netif: got global address");
*has_global = true;
}
}

bool gnrc_netif_ipv6_wait_for_global_address(gnrc_netif_t *netif,
uint32_t timeout_ms)
{
Expand All @@ -1385,7 +1394,7 @@ bool gnrc_netif_ipv6_wait_for_global_address(gnrc_netif_t *netif,
msg_bus_entry_t subs[netif_numof];
bool has_global = false;

if (netif) {
if (netif != NULL) {
if (_has_global_addr(netif)) {
return true;
}
Expand All @@ -1406,17 +1415,27 @@ bool gnrc_netif_ipv6_wait_for_global_address(gnrc_netif_t *netif,

/* wait for global address */
msg_t m;
ztimer_now_t t_stop = ztimer_now(ZTIMER_MSEC) + timeout_ms;
while (!has_global) {
/* stop if timeout reached */
if (ztimer_now(ZTIMER_MSEC) > t_stop) {
DEBUG_PUTS("gnrc_netif: timeout reached");
break;
}
/* waiting for any message */
if (ztimer_msg_receive_timeout(ZTIMER_MSEC, &m, timeout_ms) < 0) {
DEBUG_PUTS("gnrc_netif: timeout waiting for prefix");
break;
}

if (ipv6_addr_is_link_local(m.content.ptr)) {
DEBUG_PUTS("gnrc_netif: got link-local address");
if (netif != NULL) {
_netif_bus_msg_global(netif, &m, &has_global);
} else {
DEBUG_PUTS("gnrc_netif: got global address");
has_global = true;
/* scan all interfaces */
gnrc_netif_t *_netif = NULL;
while ((_netif = gnrc_netif_iter(_netif))) {
_netif_bus_msg_global(_netif, &m, &has_global);
}
}
}

Expand Down Expand Up @@ -1738,7 +1757,7 @@ static void _send_queued_pkt(gnrc_netif_t *netif)
}

static void _tx_done(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt,
gnrc_pktsnip_t *tx_sync , int res, bool push_back)
gnrc_pktsnip_t *tx_sync, int res, bool push_back)
{
(void)push_back; /* only used with IS_USED(MODULE_GNRC_NETIF_PKTQ) */

Expand Down
12 changes: 12 additions & 0 deletions sys/trickle/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) 2023 HAW Hamburg
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#

config MODULE_TRICKLE
bool "Trickle Algorithm (RFC 6206)"
depends on TEST_KCONFIG
select ZTIMER_MSEC
select MODULE_RANDOM
11 changes: 1 addition & 10 deletions sys/trickle/trickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "inttypes.h"
#include "random.h"
#include "trickle.h"
#include "ztimer.h"

#define ENABLE_DEBUG 0
#include "debug.h"
Expand Down Expand Up @@ -53,14 +54,8 @@ void trickle_interval(trickle_t *trickle)
/* old_interval == trickle->I / 2 */
trickle->t = random_uint32_range(old_interval, trickle->I);

#if IS_USED(MODULE_ZTIMER_MSEC)
ztimer_set_msg(ZTIMER_MSEC, &trickle->msg_timer, (trickle->t + diff),
&trickle->msg, trickle->pid);
#else
uint64_t msg_time = (trickle->t + diff) * US_PER_MS;
xtimer_set_msg64(&trickle->msg_timer, msg_time, &trickle->msg,
trickle->pid);
#endif
}

void trickle_reset_timer(trickle_t *trickle)
Expand Down Expand Up @@ -93,11 +88,7 @@ void trickle_start(kernel_pid_t pid, trickle_t *trickle, uint16_t msg_type,

void trickle_stop(trickle_t *trickle)
{
#if IS_USED(MODULE_ZTIMER_MSEC)
ztimer_remove(ZTIMER_MSEC, &trickle->msg_timer);
#else
xtimer_remove(&trickle->msg_timer);
#endif
}

void trickle_increment_counter(trickle_t *trickle)
Expand Down
12 changes: 6 additions & 6 deletions tests/periph_pwm/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#define OSC_STEPS (256U)
#define PWR_SLEEP (1U)

static uint32_t initiated;
static uint32_t initialized;

static unsigned _get_dev(const char *dev_str)
{
Expand Down Expand Up @@ -94,11 +94,11 @@ static int _init(int argc, char** argv)
(uint16_t)atoi(argv[4]));
if (pwm_freq != 0) {
printf("The pwm frequency is set to %" PRIu32 "\n", pwm_freq);
initiated |= (1 << dev);
initialized |= (1 << dev);
return 0;
}

puts("Error: device is not initiated");
puts("Error: device is not initialized");
return 1;
}

Expand All @@ -117,8 +117,8 @@ static int _set(int argc, char**argv)
return 1;
}

if ((initiated & (1 << dev)) == 0) {
puts("Error: pwm is not initiated.\n");
if ((initialized & (1 << dev)) == 0) {
puts("Error: pwm is not initialized.\n");
puts("Execute init function first.\n");
return 1;
}
Expand Down Expand Up @@ -245,7 +245,7 @@ static const shell_command_t shell_commands[] = {
int main(void)
{
puts("PWM peripheral driver test\n");
initiated = 0;
initialized = 0;

char line_buf[SHELL_DEFAULT_BUFSIZE];
shell_run(shell_commands, line_buf, SHELL_DEFAULT_BUFSIZE);
Expand Down
1 change: 1 addition & 0 deletions tests/trickle/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include ../Makefile.tests_common

USEMODULE += trickle
USEMODULE += ztimer_msec

# microbit qemu lacks rtt
TEST_ON_CI_BLACKLIST += microbit
Expand Down
3 changes: 3 additions & 0 deletions tests/trickle/app.config.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CONFIG_MODULE_TRICKLE=y
CONFIG_MODULE_ZTIMER=y
CONFIG_MODULE_ZTIMER_MSEC=y
10 changes: 1 addition & 9 deletions tests/trickle/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@
#include "trickle.h"
#include "thread.h"
#include "msg.h"
#if IS_USED(MODULE_ZTIMER_MSEC)
#include "ztimer.h"
#else
#include "xtimer.h"
#endif


#define TRICKLE_MSG (0xfeef)
#define TR_IMIN (8)
Expand All @@ -50,12 +47,7 @@ static trickle_t trickle = { .callback = { .func = &callback,
static void callback(void *args)
{
(void) args;
#if IS_USED(MODULE_ZTIMER_MSEC)
uint32_t now = ztimer_now(ZTIMER_MSEC);
#else
uint32_t now = xtimer_now_usec();
#endif

printf("now = %" PRIu32 ", t = %" PRIu32 "\n", now, trickle.t);

/* previous `t` is chosen from a smaller interval [I/2, I).
Expand Down

0 comments on commit 2fcedf9

Please sign in to comment.