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.

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

sys/cpp11-compat: Remove xtimer deps #19369

Merged
merged 4 commits into from
Mar 27, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Mar 9, 2023

Contribution description

This replaces the xtimer calls with explicit ztimer64 calls, since xtimer is somewhat deprecated.

Testing procedure

  • Green murdock
  • I suppose the following tests/
c11_atomics_cpp_compat
cpp11_condition_variable
cpp11_mutex
cpp11_thread
cpp_ctors
cpp_exclude
cpp_ext
irq_cpp
rmutex_cp
  • run compile_and_test_for_board.py and compile_like_murdock.py for the subset of tests.

Issues/PRs references

@github-actions github-actions bot added Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework labels Mar 9, 2023
@benpicco benpicco requested a review from kaspar030 March 9, 2023 13:30
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 9, 2023
@riot-ci
Copy link

riot-ci commented Mar 9, 2023

Murdock results

✔️ PASSED

95c238a tests/cpp11_thread: Update BOARD_INSUFFICIENT_MEMORY

Success Failures Total Runtime
6882 0 6882 09m:48s

Artifacts

@MrKevinWeiss
Copy link
Contributor Author

Any blockers? @josephnoir originally did this?

@benpicco benpicco requested a review from kfessel March 15, 2023 15:15
xtimer_now_timex(&before);
xtimer_t timer;
xtimer_set_wakeup(&timer, timex_uint64(timeout), thread_getpid());
before = timex_from_uint64(ztimer64_now(ZTIMER64_USEC));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is duration cast to timex_t (l176 to 179) ? (not to 64bit microseconds directly)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

cv.wait_for(lk, chrono::seconds(timeout));
xtimer_now_timex(&after);
after = timex_from_uint64(ztimer64_now(ZTIMER64_USEC));
Copy link
Contributor

Choose a reason for hiding this comment

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

seems also like unforced use of timex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Copy link
Contributor Author

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

Note that there is still a dependency on timex for the type...

xtimer_now_timex(&before);
xtimer_t timer;
xtimer_set_wakeup(&timer, timex_uint64(timeout), thread_getpid());
before = timex_from_uint64(ztimer64_now(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.

Done

cv.wait_for(lk, chrono::seconds(timeout));
xtimer_now_timex(&after);
after = timex_from_uint64(ztimer64_now(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.

Removed

Comment on lines 177 to 178
timeout = s.count() * US_PER_SEC;
timeout += (duration_cast<microseconds>(timeout_duration - s)).count();
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
timeout = s.count() * US_PER_SEC;
timeout += (duration_cast<microseconds>(timeout_duration - s)).count();
timeout = duration_cast<microseconds>(timeout_duration).count();

and remove line 176

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I thought that microseconds overflow at 1 second but I guess that was done as a result of timex. Done.

Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

reads good, tested good

please squash,
while doing that please correct the commit messages to "*: Use ztimer64_usec instead of xtimer" as these are ports to ztimer64 (which requires no aquire and release and has the timer (and therefor clock) always running)

@MrKevinWeiss
Copy link
Contributor Author

Thanks!

@MrKevinWeiss
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Mar 24, 2023
19369: sys/cpp11-compat: Remove xtimer deps r=MrKevinWeiss a=MrKevinWeiss

### Contribution description

This replaces the `xtimer` calls with explicit `ztimer64` calls, since `xtimer` is somewhat deprecated. 

### Testing procedure

- Green murdock
- I suppose the following `tests/`
```
c11_atomics_cpp_compat
cpp11_condition_variable
cpp11_mutex
cpp11_thread
cpp_ctors
cpp_exclude
cpp_ext
irq_cpp
rmutex_cp
```
- run `compile_and_test_for_board.py` and `compile_like_murdock.py` for the subset of tests.

### Issues/PRs references



Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 24, 2023

Build failed:

@MrKevinWeiss
Copy link
Contributor Author

Hmm I don't like how it is bigger...

@kfessel
Copy link
Contributor

kfessel commented Mar 24, 2023

it shrunk for stm32f7 (m7) and stm32f3(m4) and increased for stm32l0 (m0+)

   text    data     bss     dec     hex filename
  36364     196    2476   39036    987c cpppr/tests/cpp11_thread/bin/nucleo-f767zi/tests_cpp11_thread.elf
  36584     196    2476   39256    9958 master/tests/cpp11_thread/bin/nucleo-f767zi/tests_cpp11_thread.elf
  36072     192    2468   38732    974c cpppr/tests/cpp11_thread/bin/nucleo-l053r8/tests_cpp11_thread.elf
  35908     192    2468   38568    96a8 master/tests/cpp11_thread/bin/nucleo-l053r8/tests_cpp11_thread.elf
  35784     192    2476   38452    9634 cpppr/tests/cpp11_thread/bin/nucleo-f303re/tests_cpp11_thread.elf
  36004     192    2476   38672    9710 master/tests/cpp11_thread/bin/nucleo-f303re/tests_cpp11_thread.elf

@MrKevinWeiss
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Mar 27, 2023
19369: sys/cpp11-compat: Remove xtimer deps r=MrKevinWeiss a=MrKevinWeiss

### Contribution description

This replaces the `xtimer` calls with explicit `ztimer64` calls, since `xtimer` is somewhat deprecated. 

### Testing procedure

- Green murdock
- I suppose the following `tests/`
```
c11_atomics_cpp_compat
cpp11_condition_variable
cpp11_mutex
cpp11_thread
cpp_ctors
cpp_exclude
cpp_ext
irq_cpp
rmutex_cp
```
- run `compile_and_test_for_board.py` and `compile_like_murdock.py` for the subset of tests.

### Issues/PRs references



Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 27, 2023

Build failed:

@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 27, 2023

Build succeeded:

@bors bors bot merged commit de8cabc into RIOT-OS:master Mar 27, 2023
@MrKevinWeiss MrKevinWeiss deleted the pr/remove/xtimercpp branch March 28, 2023 09:54
@MrKevinWeiss
Copy link
Contributor Author

thanks

bors bot added a commit that referenced this pull request Apr 17, 2023
19411: cpu/gd32v: add riotboot support r=benpicco a=gschorcht

### Contribution description

This PR provides `riotboot` support for GD32V.

### Testing procedure

Use any GD32V board with a JTAG adapter and flash the bootloader:
```python
PROGRAMMER=openocd BOARD=sipeed-longan-nano make -C bootloaders/riotboot flash
```
Flash slot 0 and set `RIOT_VERSION` to 1
```python
USEMODULE=stdio_uart FEATURES_REQUIRED=riotboot RIOT_VERSION=1 \
PROGRAMMER=openocd BOARD=sipeed-longan-nano make -C tests/shell riotboot/flash-slot0
...
### Flashing Target ###
Binfile detected, adding ROM base address: 0x08000000
Flashing with IMAGE_OFFSET: 0x08001000
```
```python
> main(): This is RIOT! (Version: 1)
test_shell.
```
Flash slot 1 and set `RIOT_VERSION` to 2
```python
USEMODULE=stdio_uart FEATURES_REQUIRED=riotboot RIOT_VERSION=2 \
PROGRAMMER=openocd BOARD=sipeed-longan-nano make -C tests/shell riotboot/flash-slot1
...
### Flashing Target ###
Binfile detected, adding ROM base address: 0x08000000
Flashing with IMAGE_OFFSET: 0x08010800
```
```python
> main(): This is RIOT! (Version: 2)
test_shell.
```

### Issues/PRs references


19436: cpp11-compat: thread::sleep_for in microseconds r=benpicco a=kfessel

### Contribution description

after reviewing #19369  i found that there is a conversion to nanoseconds just to convert it to microseconds some instrunctions later for ztimer64_usec to handle it this removes one of the conversions (convert once direct to microseconds)

### Testing procedure

run the cpp tests

### Issues/PRs references

#19369

19450: cpu/esp32: fix compilation issues with GCC 12.2 r=benpicco a=gschorcht

### Contribution description

This PR provides the changes in `cpu/esp32` and `cpu/esp_common` to fix the compilation issues with GCC v12.2.  It is required as the first step in the preparation of the upgrade to ESP-IDF version 5.1.

**Please note**: Insead of fixing the ESP-IDF 4.4 code itself by a big bunch of patches to fix the compilation problems with GCC v12.2, it temporarily disables some warnings. The reason is that the ESP-IDF 5.1 requires GCC v12.2 and should be fixed for this compiler version by the vendor.

### Testing procedure

Green CI

The change were already tested with all ESP-specific modules like `esp_now`, `esp_wifi`, `esp_spi`  and `esp_ble` for all supported ESP platforms.

### Issues/PRs references

Prerequisite for RIOT-OS/riotdocker#227
Fixes issue #19421

19476: native/syscalls: rename real_clock_gettime to clock_gettime r=benpicco a=Teufelchen1

### Contribution description

When compiling RIOT for native using a recent LLVM and enabling ASAN, one might encounter "Duplicated symbol".

This is due to a name clash with `real_clock_gettime()` in compiler-rt from [LLVM](llvm/llvm-project@f50246d), I renamed RIOTs `real_clock_gettime` and just default to the posix function `clock_gettime`. The wrapper existed, most likely, for consistency only.

(The best solution would probably to convince the LLVM folks to declare their symbol as `static` and refactor a bit)

### Testing procedure

Passing CI should be enough.


Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Karl Fessel <karl.fessel@ovgu.de>
Co-authored-by: Teufelchen1 <bennet.blischke@haw-hamburg.de>
bors bot added a commit that referenced this pull request Apr 17, 2023
19436: cpp11-compat: thread::sleep_for in microseconds r=benpicco a=kfessel

### Contribution description

after reviewing #19369  i found that there is a conversion to nanoseconds just to convert it to microseconds some instrunctions later for ztimer64_usec to handle it this removes one of the conversions (convert once direct to microseconds)

### Testing procedure

run the cpp tests

### Issues/PRs references

#19369

Co-authored-by: Karl Fessel <karl.fessel@ovgu.de>
@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
Labels
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants