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

cpu/native: Switch to ztimer for gettimeofday #19349

Merged
merged 2 commits into from
Mar 8, 2023
Merged

cpu/native: Switch to ztimer for gettimeofday #19349

merged 2 commits into from
Mar 8, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

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

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Platform: native Platform: This PR/issue effects the native platform labels Mar 6, 2023
@MrKevinWeiss MrKevinWeiss requested a review from benpicco March 6, 2023 11:08
@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 6, 2023
@MrKevinWeiss
Copy link
Contributor Author

I don't know if we would want to keep xtimer stuff around. Open to opinions!

@MrKevinWeiss MrKevinWeiss requested a review from kaspar030 March 6, 2023 11:08
kaspar030
kaspar030 reviewed Mar 6, 2023
View reviewed changes
cpu/native/Makefile.dep Outdated
# requires 64bit timestamps
USEMODULE += ztimer64_xtimer_compat
endif
USEMODULE += ztimer
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be just ztimer64_usec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many places use the USEMODULE += ztimer as an entry point and ztimer_usec to select the backend. I can remove it if you want...

Copy link
Contributor

@kaspar030 kaspar030 Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

hm, one implies the other. no need to list the deps of the deps IMO. as you like! but, it's "ztimer64_usec" that is used, not "ztimer_usec"

@riot-ci
Copy link

riot-ci commented Mar 6, 2023 •
edited
Loading

Murdock results

✔️ PASSED

89f2ae7 tests/cpp11_mutex: Cleanup deps

Success Failures Total Runtime
6919 0 6919 12m:45s

Artifacts

kaspar030
kaspar030 requested changes Mar 6, 2023
View reviewed changes
cpu/native/Makefile.dep Outdated
Comment on lines 44 to 45
USEMODULE += ztimer
USEMODULE += ztimer_usec
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
USEMODULE += ztimer
USEMODULE += ztimer_usec
USEMODULE += ztimer64_usec

Copy link
Contributor Author

@MrKevinWeiss MrKevinWeiss Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's my phone's diff view, but it still shows ztimer_msec vs. ztimer64_msec (also for Kconfig)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, makes sense...

@MrKevinWeiss MrKevinWeiss added CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Mar 6, 2023
@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Mar 6, 2023
@bors
Try #19349:
470e03a
@bors
Copy link
Contributor

bors bot commented Mar 6, 2023

try

Build succeeded:

kaspar030
kaspar030 approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 7, 2023

👎 Rejected by PR status

@MrKevinWeiss MrKevinWeiss force-pushed the pr/remove/xtimernative branch from 5b5a383 to 89f2ae7 Compare March 7, 2023 12:09
@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 7, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@benpicco
Copy link
Contributor

benpicco commented Mar 8, 2023

bors merge

@bors bors bot merged commit fae992e into RIOT-OS:master Mar 8, 2023
@bors
Copy link
Contributor

bors bot commented Mar 8, 2023

Build succeeded:

@MrKevinWeiss MrKevinWeiss deleted the pr/remove/xtimernative branch March 8, 2023 13:01
@MrKevinWeiss
Copy link
Contributor Author

thanks!

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaspar030 kaspar030 kaspar030 approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@smlng smlng Awaiting requested review from smlng

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@benpicco benpicco Awaiting requested review from benpicco

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
4 participants
@MrKevinWeiss @riot-ci @benpicco @kaspar030