Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
19343: ztimer: add ztimer_stopwatch convenience functions r=benpicco a=benpicco



19349: cpu/native: Switch to ztimer for gettimeofday r=benpicco a=MrKevinWeiss

### Contribution description

A xtimer is somewhat taken over by ztimer this explicitly uses ztimer instead of relying on the compatibility layer.

### Testing procedure

`make all test -C tests/cpp11_mutex/`

and green murdock I guess.

### Issues/PRs references


19353: doc: add quicklink to boards in navbar r=benpicco a=OlegHahm

### Contribution description

Finding a list of supported boards and how to use them is an essential information. Currently this list is somewhat hidden under "Modules" which is not very intuitive. Hence, I propose to (at least) put a link in the side menu to this overview page.

### Testing procedure

1. Call `make doc`
2. Check the sidebar `${RIOT_BASE}/doc/doxygen/html/index.html` for an entry "Supported Boards"

19361: nanocoap_sock: ensure response address is the same as request address r=benpicco a=benpicco



19363: Fix stm32 timer periodic r=benpicco a=Enoch247

### Contribution description

From the commit msg:

>     cpu/stm32/periph/timer: remove unneeded header
>     
>     I see no reason this header should be included. It does not exist in
>     RIOT's source tree. This patch removes the include.

and 

>     cpu/stm32/periph/timer: fix execution flow
>     
>     The implmentation of `timer_set_absolute()` has The following problems.
>     First, it attempts to restore the auto reload register (ARR) to it's
>     default if the ARR was previosly set by `timer_set_periodic()` by
>     comparing it to the channel's capture compare (CC) register _after_ it
>     has already set the CC register. Secondly, it clears spurious IRQs
>     _after_ the CC register has been set. If the value being set is equal to
>     the timer's current count (or the two become equal before the supurios
>     IRQ clearing happens), this could cause a legitimate IRQ to be cleared.
>     
>     The implmentation of `timer_set()` has the same error in handling the
>     ARR as described above.
>     
>     This patch reorders the operations of both functions to do:
>     
>     1. handle ARR
>     2. clear spurious IRQs
>     3. set channel's CC
>     4. enable IRQ
>     
>     Additionally, the calulation of `value` in `timer_set()` is moved
>     earlier in the function's exec path as a pedantic measure.


### Testing procedure

I tested by doing the following:

1. `make -C tests/periph_timer BOARD=nucleo-f767zi all flash term`
2. press s
3. press [ENTER]
4. observe test passes
5. `make -C tests/periph_timer_periodic BOARD=nucleo-f767zi all flash term`
6. press s
7. press [ENTER]
8. observe test passes
9. `make -C tests/periph_timer_short_relative_set BOARD=nucleo-f767zi all flash term`
10. press s
11. press [ENTER]
12. observe test passes


### Issues/PRs references

- none known


Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
Co-authored-by: Oleg Hahm <oleg@hobbykeller.org>
Co-authored-by: Benjamin Valentin <benjamin.valentin@bht-berlin.de>
Co-authored-by: Joshua DeWeese <jdeweese@primecontrols.com>
  • Loading branch information
6 people authored Mar 8, 2023
6 parents 3575106 + d91c0e8 + 89f2ae7 + 9aa0c0f + ad8dd8d + eeb359e commit fae992e
Show file tree
Hide file tree
Showing 13 changed files with 193 additions and 62 deletions.
6 changes: 1 addition & 5 deletions cpu/native/Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ ifneq (,$(filter socket_zep,$(USEMODULE)))
endif

ifneq (,$(filter libc_gettimeofday,$(USEMODULE)))
USEMODULE += xtimer
ifneq (,$(filter ztimer_xtimer_compat,$(USEMODULE)))
# requires 64bit timestamps
USEMODULE += ztimer64_xtimer_compat
endif
USEMODULE += ztimer64_usec
endif

USEMODULE += periph
Expand Down
2 changes: 1 addition & 1 deletion cpu/native/periph/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* Uses POSIX realtime clock and POSIX itimer to mimic hardware.
*
* This is based on native's hwtimer implementation by Ludwig Knüpfer.
* I removed the multiplexing, as xtimer does the same. (kaspar)
* I removed the multiplexing, as ztimer does the same. (kaspar)
*
* @author Ludwig Knüpfer <ludwig.knuepfer@fu-berlin.de>
* @author Kaspar Schleiser <kaspar@schleiser.de>
Expand Down
9 changes: 6 additions & 3 deletions cpu/native/syscalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@
#include <stdlib.h>
#include <stdio.h>
#include <stdarg.h>
#ifdef MODULE_XTIMER
#ifdef MODULE_LIBC_GETTIMEOFDAY
#include <sys/time.h>
#endif
#include <ifaddrs.h>
#include <sys/stat.h>

#include "cpu.h"
#include "irq.h"
#include "xtimer.h"
#ifdef MODULE_LIBC_GETTIMEOFDAY
#include "time_units.h"
#include "ztimer64.h"
#endif
#include "stdio_base.h"

#include "kernel_defines.h"
Expand Down Expand Up @@ -485,7 +488,7 @@ int getpid(void)
int _gettimeofday(struct timeval *tp, void *restrict tzp)
{
(void)tzp;
uint64_t now = xtimer_now_usec64();
uint64_t now = ztimer64_now(ZTIMER64_USEC);
tp->tv_sec = now / US_PER_SEC;
tp->tv_usec = now - tp->tv_sec;
return 0;
Expand Down
20 changes: 10 additions & 10 deletions cpu/stm32/periph/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

#include "cpu.h"
#include "periph/timer.h"
#include <sys/_intsup.h>

/**
* @brief Interrupt context for each configured timer
Expand Down Expand Up @@ -129,8 +128,6 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
unsigned irqstate = irq_disable();
set_oneshot(tim, channel);

TIM_CHAN(tim, channel) = (value & timer_config[tim].max);

#ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max;
Expand All @@ -140,6 +137,8 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
/* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);

TIM_CHAN(tim, channel) = (value & timer_config[tim].max);

/* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);
irq_restore(irqstate);
Expand All @@ -149,28 +148,29 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)

int timer_set(tim_t tim, int channel, unsigned int timeout)
{
unsigned value = (dev(tim)->CNT + timeout) & timer_config[tim].max;

if (channel >= (int)TIMER_CHANNEL_NUMOF) {
return -1;
}

unsigned irqstate = irq_disable();
set_oneshot(tim, channel);

#ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max;
}
#endif

/* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);

unsigned value = (dev(tim)->CNT + timeout) & timer_config[tim].max;
TIM_CHAN(tim, channel) = value;

/* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);

#ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max;
}
#endif

/* calculate time till timeout */
value = (value - dev(tim)->CNT) & timer_config[tim].max;

Expand Down
1 change: 1 addition & 0 deletions doc/doxygen/RIOTDoxygenLayout.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<navindex>
<tab type="mainpage" visible="yes" title=""/>
<tab type="pages" visible="yes" title="" intro=""/>
<tab type="user" url="@ref boards" title="Supported Boards"/>
<tab type="modules" visible="yes" title="" intro=""/>
<tab type="namespaces" visible="yes" title="">
<tab type="namespacelist" visible="yes" title="" intro=""/>
Expand Down
3 changes: 1 addition & 2 deletions sys/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ endchoice

config MODULE_LIBC_GETTIMEOFDAY
bool "Support for gettimeofday()"
select MODULE_XTIMER
select MODULE_ZTIMER64_XTIMER_COMPAT if MODULE_ZTIMER_XTIMER_COMPAT
select ZTIMER64_USEC

rsource "Kconfig.newlib"
rsource "Kconfig.picolibc"
Expand Down
9 changes: 5 additions & 4 deletions sys/include/benchmark.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <stdint.h>

#include "irq.h"
#include "ztimer.h"
#include "ztimer/stopwatch.h"

#ifdef __cplusplus
extern "C" {
Expand All @@ -43,12 +43,13 @@ extern "C" {
*/
#define BENCHMARK_FUNC(name, runs, func) \
do { \
uint32_t _benchmark_time = ztimer_now(ZTIMER_USEC); \
ztimer_stopwatch_t timer = { .clock = ZTIMER_USEC }; \
ztimer_stopwatch_start(&timer); \
for (unsigned long i = 0; i < runs; i++) { \
func; \
} \
_benchmark_time = (ztimer_now(ZTIMER_USEC) - _benchmark_time);\
benchmark_print_time(_benchmark_time, runs, name); \
benchmark_print_time(ztimer_stopwatch_measure(&timer), runs, name); \
ztimer_stopwatch_stop(&timer); \
} while (0)

/**
Expand Down
29 changes: 29 additions & 0 deletions sys/include/net/sock/udp.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@
#ifndef NET_SOCK_UDP_H
#define NET_SOCK_UDP_H

#include <assert.h>
#include <errno.h>
#include <stdint.h>
#include <stdlib.h>
Expand All @@ -280,7 +281,10 @@
# pragma clang diagnostic ignored "-Wtypedef-redefinition"
#endif

#include "net/af.h"
#include "net/sock.h"
#include "net/ipv4/addr.h"
#include "net/ipv6/addr.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -785,6 +789,31 @@ static inline ssize_t sock_udp_sendv(sock_udp_t *sock,
return sock_udp_sendv_aux(sock, snips, remote, NULL);
}

/**
* @brief Checks if the IP address of an endpoint is multicast
*
* @param[in] ep end point to check
*
* @returns true if end point is multicast
*/
static inline bool sock_udp_ep_is_multicast(const sock_udp_ep_t *ep)
{
switch (ep->family) {
#ifdef SOCK_HAS_IPV6
case AF_INET6:
return ipv6_addr_is_multicast((const ipv6_addr_t *)&ep->addr.ipv6);
#endif
#ifdef SOCK_HAS_IPV4
case AF_INET:
return ipv4_addr_is_multicast((const ipv4_addr_t *)&ep->addr.ipv4);
#endif
default:
assert(0);
}

return false;
}

#include "sock_types.h"

#ifdef __cplusplus
Expand Down
102 changes: 102 additions & 0 deletions sys/include/ztimer/stopwatch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright (C) 2023 ML!PA Consulting GmbH
*
* 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.
*/

/**
* @defgroup sys_ztimer_stopwatch ztimer stop watch
* @ingroup sys_ztimer
* @brief Measure time with ztimer
*
* @author Benjamin Valentin <benjamin.valentin@ml-pa.com>
* @{
*/
#ifndef ZTIMER_STOPWATCH_H
#define ZTIMER_STOPWATCH_H

#include "ztimer.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief ztimer stop watch struct
*/
typedef struct {
ztimer_clock_t *clock; /**< the clock to use */
uint32_t start_time; /**< start of measurement */
} ztimer_stopwatch_t;

/**
* @brief Initialize a ztimer stop watch
* The stop watch is not running yet.
*
* @param[in] clock The clock to use for the stopwatch
* @param[out] timer The stop watch clock to initialize
*/
static inline void ztimer_stopwatch_init(ztimer_clock_t *clock, ztimer_stopwatch_t *timer)
{
timer->clock = clock;
}

/**
* @brief Start the stop watch timer
*
* @param[in] timer The stop watch timer to start
*/
static inline void ztimer_stopwatch_start(ztimer_stopwatch_t *timer)
{
ztimer_acquire(timer->clock);
timer->start_time = ztimer_now(timer->clock);
}

/**
* @brief Take a measurement from the stop watch timer
*
* @param[in] timer The stop watch timer to take a measurement of
*
* @return Timer ticks since the stop watch was started / reset
*/
static inline uint32_t ztimer_stopwatch_measure(ztimer_stopwatch_t *timer)
{
return ztimer_now(timer->clock) - timer->start_time;
}

/**
* @brief Reset the stop watch start time.
* The Stop Watch will start counting from 0 again.
*
* @param[in] timer The stop watch timer to reset
*
* @return Timer ticks since the last reset / start of the watch
*/
static inline uint32_t ztimer_stopwatch_reset(ztimer_stopwatch_t *timer)
{
uint32_t now = ztimer_now(timer->clock);
uint32_t diff = now - timer->start_time;

timer->start_time = now;
return diff;
}

/**
* @brief Stop the stop watch.
* The stop watch will no longer run.
*
* @param[in] timer The stop watch timer to stop
*/
static inline void ztimer_stopwatch_stop(ztimer_stopwatch_t *timer)
{
ztimer_release(timer->clock);
}

#ifdef __cplusplus
}
#endif

#endif /* ZTIMER_STOPWATCH_H */
/** @} */
23 changes: 2 additions & 21 deletions sys/net/application_layer/gcoap/gcoap.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ static void _cache_process(gcoap_request_memo_t *memo,
static ssize_t _cache_build_response(nanocoap_cache_entry_t *ce, coap_pkt_t *pdu,
uint8_t *buf, size_t len);
static void _receive_from_cache_cb(void *arg);
static bool _ep_is_multicast(const sock_udp_ep_t *remote_ep);

static int _request_matcher_default(gcoap_listener_t *listener,
const coap_resource_t **resource,
Expand Down Expand Up @@ -353,7 +352,7 @@ static void _on_sock_udp_evt(sock_udp_t *sock, sock_async_flags_t type, void *ar
.flags = SOCK_AUX_SET_LOCAL,
.local = aux_in.local,
};
if (_ep_is_multicast(&aux_in.local)) {
if (sock_udp_ep_is_multicast(&aux_in.local)) {
/* This eventually gets passed to sock_udp_send_aux, where NULL
* simply does not set any flags */
aux_out_ptr = NULL;
Expand Down Expand Up @@ -845,24 +844,6 @@ static int _find_resource(gcoap_socket_type_t tl_type,
return ret;
}

static bool _ep_is_multicast(const sock_udp_ep_t *remote_ep)
{
switch (remote_ep->family) {
#ifdef SOCK_HAS_IPV6
case AF_INET6:
return ipv6_addr_is_multicast((const ipv6_addr_t *)&remote_ep->addr.ipv6);
#endif
#ifdef SOCK_HAS_IPV4
case AF_INET:
return ipv4_addr_is_multicast((const ipv4_addr_t *)&remote_ep->addr.ipv4);
#endif
default:
assert(0);
}

return false;
}

/*
* Finds the memo for an outstanding request within the _coap_state.open_reqs
* array. Matches on remote endpoint and token.
Expand Down Expand Up @@ -900,7 +881,7 @@ static void _find_req_memo(gcoap_request_memo_t **memo_ptr, coap_pkt_t *src_pdu,
if ((memcmp(coap_get_token(src_pdu), coap_get_token(memo_pdu), cmplen) == 0)
&& (sock_udp_ep_equal(&memo->remote_ep, remote)
/* Multicast addresses are not considered in matching responses */
|| _ep_is_multicast(&memo->remote_ep)
|| sock_udp_ep_is_multicast(&memo->remote_ep)
)) {
*memo_ptr = memo;
break;
Expand Down
Loading

0 comments on commit fae992e

Please sign in to comment.