Skip to content
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.

Sign up for GitHub

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

Jump to bottom

sys: some simple xtimer->ztimer conversions #17693

Closed
wants to merge 5 commits into from
Closed

sys: some simple xtimer->ztimer conversions #17693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d5a475
sys/Makefile.dep: add fallback timer evaluation
kfessel Mar 3, 2022
24262ad
net/uhcp: do not require xtimer
kfessel Feb 23, 2022
11e6d2a
posix/sockets: do not require timer
kfessel Feb 23, 2022
6c4baee
sys/suit_transport_coap: do not require xtimer
kfessel Feb 23, 2022
3cf5f64
net/csma_sender: do not require xtimer
kfessel Feb 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions sys/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ endif

ifneq (,$(filter csma_sender,$(USEMODULE)))
USEMODULE += random
USEMODULE += xtimer
USEMODULE += xztimer_fallback
endif

ifneq (,$(filter dhcpv6_%,$(USEMODULE)))
Expand Down Expand Up @@ -167,7 +167,7 @@ endif

ifneq (,$(filter uhcpc,$(USEMODULE)))
USEMODULE += posix_inet
USEMODULE += xtimer
USEMODULE += xztimer_fallback
endif

ifneq (,$(filter netdev_tap,$(USEMODULE)))
Expand Down Expand Up @@ -274,7 +274,6 @@ ifneq (,$(filter posix_sockets,$(USEMODULE)))
USEMODULE += random
USEMODULE += vfs
USEMODULE += posix_headers
USEMODULE += xtimer
endif

ifneq (,$(filter shell,$(USEMODULE)))
Expand Down Expand Up @@ -818,7 +817,7 @@ endif

ifneq (,$(filter suit_transport_coap, $(USEMODULE)))
USEMODULE += nanocoap_sock
USEMODULE += xtimer
USEMODULE += xztimer_fallback
USEMODULE += sock_util
endif

Expand Down Expand Up @@ -858,6 +857,13 @@ ifneq (,$(filter evtimer,$(USEMODULE)))
endif
endif

ifneq (,$(filter xztimer_fallback,$(USEMODULE)))
USEMODULE := $(filter-out xztimer_fallback,$(USEMODULE))
ifeq (,$(filter ztimer_usec,$(USEMODULE)))
USEMODULE += xtimer
endif
endif
Comment on lines +860 to +865
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of pattern is not used and is not a good idea to use in the build system, it is very very prone to dependency order issues.

Are we sure this is still worth it with xtimer_ztimer_compat being used by default now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a step to remove the xtimer support from these modules they should be easy conversion to ztimer. We may just remove the xtimer support from these modules entirely and skip this step.

I do not like writing these:

if there is ztimer
   do this 
elseif there is xtimer
   do that

Even reading this kind of code not nice since it distracts from what is the main purpose of that module.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then instead how about adapting the coccinelle scripts to have two versions a ztimer_xtimer_compat version and a ztimer64_xtimer_compat version, and simply spatch those files and directly make the conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#17892 has that done (without the coccinelle part)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This kind of pattern is not used and is not a good idea to use in the build system, it is very very prone to dependency order issues.

somehow i think the xztimer_fallback solves a dependency order issue for modules that support ztimer as well as xtimer through delaying the decision until it is necessary

If the first module in Makefile.dep supports x and z timer the dependancy will be decided at that point and every later module will see the decision done there but in reality the module did not care. - may be this needs another PR


# handle xtimer's deps. Needs to be done *after* ztimer
ifneq (,$(filter xtimer,$(USEMODULE)))
include $(RIOTBASE)/sys/xtimer/Makefile.dep
Expand Down
4 changes: 4 additions & 0 deletions sys/net/application_layer/uhcp/uhcpc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@
#include "net/af.h"
#include "net/sock/udp.h"
#include "net/uhcp.h"
#if IS_USED(MODULE_ZTIMER_USEC)
#include "ztimer/xtimer_compat.h"
#else
#include "xtimer.h"
#endif

/**
* @brief Request prefix from uhcp server
Expand Down
5 changes: 5 additions & 0 deletions sys/net/link_layer/csma_sender/csma_sender.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@
#include <errno.h>
#include <stdbool.h>
#include <inttypes.h>
#include <kernel_defines.h>

#if IS_USED(MODULE_ZTIMER_USEC)
#include "ztimer/xtimer_compat.h"
#else
#include "xtimer.h"
#endif
#include "random.h"
#include "net/netdev.h"
#include "net/netopt.h"
Expand Down
4 changes: 4 additions & 0 deletions sys/suit/transport/coap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
#include "net/nanocoap_sock.h"
#include "thread.h"
#include "periph/pm.h"
#if IS_USED(MODULE_ZTIMER_USEC)
#include "ztimer/xtimer_compat.h"
#else
#include "xtimer.h"
#endif

#include "suit/transport/coap.h"
#include "net/sock/util.h"
Expand Down