sys/fmt: optimize scn_u32_dec scn_u32_hex by kfessel · Pull Request #19027 · RIOT-OS/RIOT

kfessel · 2022-12-08T12:44:41Z

Contribution description

Improves the compilation result for scn_u32_dec scn_u32_hex especially on cortex-m reducing either stack usage and or code size.

This makes use of unsigned int overflow (slightly less better without doing that hexn).

See godbolt (original versions got an o attached, modified versions got ks) all functions are marked _S_ defined to static

by assigning the global at end the compiled function can be changed (deco deck hexo hexk hexkk hexn)

this PR is hexkk and deck

Testing procedure

run unit-test/test-fmt

<RIOT>/tests/unittests$ make tests-fmt
<RIOT>/tests/unittests$ make term

Issues/PRs references

godbolt

kaspar030 · 2022-12-08T13:28:45Z

Improves the compilation result for scn_u32_dec scn_u32_hex especially on cortex-m reducing either stack usage and or code size.

Comparing just the code size of tests-fmt, my gcc 12.2.0 produces identical size w/wo this PR. How did you measure the stack usage?

In godbolt, do I compare e.g., scn_u32_hexo with scn_u32_hexk?

kfessel · 2022-12-08T14:03:15Z

this pr is hexkk (compare to hexo) and deck (compare to deco)

arm-gcc 12.2(linux) and arm-gcc 11.2.1 (none) -Os -mcpu=cortex-m4

scn_u32_hexkk(char const*, unsigned int):
        mov     r2, r0
        add     r1, r1, r0
        movs    r0, #0
.L2:
        cmp     r2, r1
        bne     .L7
.L1:
        bx      lr
.L7:
        ldrb    r3, [r2], #1    @ zero_extendqisi2
        cmp     r3, #57
        bhi     .L3
        subs    r3, r3, #48
.L4:
        orr     r0, r3, r0, lsl #4
        b       .L2
.L3:
        cmp     r3, #70
        bhi     .L5
        subs    r3, r3, #55
        b       .L4
.L5:
        cmp     r3, #102
        bhi     .L1
        subs    r3, r3, #87
        b       .L4

vs

scn_u32_hexo(char const*, unsigned int):
        push    {r4, r5, lr}
        mov     r2, r0
        add     r1, r1, r0
        movs    r0, #0
.L2:
        cmp     r2, r1
        bne     .L7
.L1:
        pop     {r4, r5, pc}
.L7:
        ldrb    r3, [r2], #1    @ zero_extendqisi2
        sub     r4, r3, #48
        uxtb    r5, r4
        cmp     r5, #9
        bls     .L3
        sub     r4, r3, #65
        cmp     r4, #25
        itt     ls
        addls   r3, r3, #32
        uxtbls  r3, r3
        subs    r4, r3, #1
        cmp     r4, #101
        bhi     .L1
        subs    r3, r3, #87
        orr     r0, r3, r0, lsl #4
        b       .L2
.L3:
        orr     r0, r4, r0, lsl #4
        b       .L2

there is obvious stack usage (push pop) by hexo and non by hexk

maribu

See inline. Please squash right away.

To me, the code is roughly equally readable. So let's go with what the compiler likes more.

sys/fmt/fmt.c

pyropeter · 2022-12-08T17:12:33Z

sys/fmt/fmt.c

-        char c = *str++;
-        if (!fmt_is_digit(c)) {
+        unsigned char c = *str++;
+        unsigned d = c - '0';


At least in C99, char literals are of type int, so this does not in fact make use of unsigned integer overflows, but instead unleashes undefined behavior.

many thanks for pointing that out.

I tried to trigger a UB sanatizer that and i could not.
Found out it is not UB but a promotion issue (-> it is well defined to go wrong since if c is unsigned char it may be promoted to int and the literal being int is just the icing to trigger that).

I also ran the test before I uploaded this PR but the char was a addition to the code after that.

pyropeter · 2022-12-08T17:13:57Z

sys/fmt/fmt.c

-            res *= 10;
-            res += (c - '0');
-        }
+        res *= 10;


This integer literal is signed as well; undefined behavior again.

riot-ci · 2022-12-08T17:41:00Z

Murdock results

✔️ PASSED

18a783d sys/fmt: optimize scn_u32_dec scn_u32_hex

Success	Failures	Total	Runtime
6865	0	6865	11m:33s

Artifacts

Documentation preview

Teufelchen1 · 2022-12-08T18:07:29Z

Please note, we got lucky that murdock catched this one. The fmt unittest is lacking in quality. Maybe this PR can also work on that for the affected two functions?

Maribu wrote:

To me, the code is roughly equally readable.

I would disagree; I perceived this as a downgrade in readability regards.

kfessel · 2022-12-08T21:11:50Z

I would disagree; I perceived this as a downgrade in readability regards.

I changed for the n version which i think more readable than the original and yields better code

@pyropeter please have another look for UB

maribu

see inline

sys/fmt/fmt.c

maribu · 2022-12-08T22:40:17Z

sys/fmt/fmt.c

+            d = c - 'A' + 0xa;
+        }
+        else if ( 'a' <= c && c <= 'f' ) {
+            d = c - 'a' + 0xa;


Suggested change

d = c - 'a' + 0xa;

d = c - (unsigned)'a' + 0xa;

there shall never be an overflow (passed the condition) -> this is defined even if a is promoted to int.

I could add it for peace of mind

kfessel · 2022-12-09T13:53:21Z

added the peace of mind interger Suffixes and unsigned casts

benpicco · 2023-01-03T16:20:24Z

Please squash

benpicco · 2023-02-17T16:34:31Z

bors merge

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19256: pkg/tinyusb: add GD32VF103 support r=benpicco a=gschorcht ### Contribution description This PR provides the tinyUSB support for GD32VF103 and enables the `tinyusb_device` feature as well as `stdio_tinyusb_cdc_acm` for GD32VF103 boards. ### Testing procedure ``` BOARD=sipeeed-longan-nano make -C tests/shell flash term ``` should work ### Issues/PRs references 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references 19286: cpu/esp_common: use generic WIFI_SSID/WIFI_PASS defines r=benpicco a=benpicco Co-authored-by: Karl Fessel <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]>

bors · 2023-02-17T16:39:30Z

Build failed (retrying...):

Murdock

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19256: pkg/tinyusb: add GD32VF103 support r=benpicco a=gschorcht ### Contribution description This PR provides the tinyUSB support for GD32VF103 and enables the `tinyusb_device` feature as well as `stdio_tinyusb_cdc_acm` for GD32VF103 boards. ### Testing procedure ``` BOARD=sipeeed-longan-nano make -C tests/shell flash term ``` should work ### Issues/PRs references Co-authored-by: Karl Fessel <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]>

bors · 2023-02-17T16:41:08Z

This PR was included in a batch that was canceled, it will be automatically retried

benpicco · 2023-02-17T16:43:19Z

bors merge

bors · 2023-02-17T16:43:22Z

Already running a review

benpicco · 2023-02-17T16:43:34Z

bors cancel
bors merge

bors · 2023-02-17T16:43:36Z

Canceled.

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) Co-authored-by: Karl Fessel <[email protected]>

benpicco · 2023-02-17T16:46:41Z

bors cancel
bors merge

bors · 2023-02-17T16:46:43Z

Canceled.

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references 19284: boards: support for the LILYGO TTGO T8 ESP32-S2 board r=benpicco a=gschorcht ### Contribution description This PR provides the support for the LILYGO TTGO T8 ESP32-S2 board which has a OLED display (not yet supported) and a SD-Card slot on board. The board is equipped with a USB-C connector that connects either to a USB-to-UART bridge or to the USB-OTG/JTAG interface of the ESP32-S2 via some DIP switches. The PR includes a very small fix of printf format string in `tests/malloc`. I can split it off. ### Testing procedure t.b.d. ### Issues/PRs references 19286: cpu/esp_common: use generic WIFI_SSID/WIFI_PASS defines r=benpicco a=benpicco Co-authored-by: Karl Fessel <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]>

bors · 2023-02-17T16:58:50Z

Build failed (retrying...):

Murdock

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=benpicco a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references Co-authored-by: Karl Fessel <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]>

bors · 2023-02-17T17:00:35Z

This PR was included in a batch that was canceled, it will be automatically retried

19027: sys/fmt: optimize scn_u32_dec scn_u32_hex r=benpicco a=kfessel ### Contribution description Improves the compilation result for `scn_u32_dec` `scn_u32_hex` especially on `cortex-m` reducing either stack usage and or code size. This makes use of unsigned int overflow (slightly less better without doing that `hexn`). See godbolt (original versions got an `o` attached, modified versions got `k`s) all functions are marked `_S_` defined to `static` by assigning the global at end the compiled function can be changed (`deco deck hexo hexk hexkk hexn`) this PR is `hexkk` and `deck` ### Testing procedure run unit-test/test-fmt ``` <RIOT>/tests/unittests$ make tests-fmt <RIOT>/tests/unittests$ make term ``` ### Issues/PRs references [godbolt](https://godbolt.org/z/MzT1zh4q1) 19269: cpu/gd32v/periph_i2c: interrupt based driver r=gschorcht a=gschorcht ### Contribution description This PR provides an interrupt-driven version of the I2C low-level driver. The existing I2C low-level driver for GDVF103 uses a busy-waiting approach where the status register is continuously polled while waiting for a certain status when sending or receiving. The MCU is thus occupied the whole time during a send or receive operation. The driver provided with this PR uses an interrupt-driven approach. This is, while waiting for a certain status when sending or receiving, the calling thread is suspended and woken up by interrupts. Since the I2C controller allows to receive up to two bytes before the application has to react, receiving a single byte, two bytes or more than two bytes needs a different handling for correct receiption. This requires a tricky implementation which distinguish a number of different case. There the driver requires 860 byte more ROM and 8 byte more RAM. ### Testing procedure The driver should work with any I2C sensor/actuator. It was tested with - `tests/driver_bmp180` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) BMP180 test application +------------Initializing------------+ Initialization successful +------------Calibration------------+ AC1: 8448 AC2: -1208 AC3: -14907 AC4: 33310 AC5: 24774 AC6: 19213 B1: 6515 B2: 49 MB: -32768 MC: -11786 MD: 2958 +--------Starting Measurements--------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.49 Pressure at see level [hPa]: 1025.55 Altitude [m]: 157 +-------------------------------------+ Temperature [°C]: 22.0 Pressure [hPa]: 1006.56 Pressure at see level [hPa]: 1025.58 Altitude [m]: 157 +-------------------------------------+ ``` </details> - `tests/driver_ccs811` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) CCS811 test application +------------Initializing------------+ +--------Starting Measurements--------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 0 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 0 eCO2 [ppm]: 400 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ TVOC [ppb]: 7 eCO2 [ppm]: 446 +-------------------------------------+ ``` </details> - `tests/driver_sht3x` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-355-g940c7-cpu/gd32v/periph_i2c_interrupt_driven) SHT3X test application +------------Initializing------------+ Initialization successful +--------Starting Measurements--------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.50 +-------------------------------------+ Temperature [°C]: 21.47 Relative Humidity [%]: 54.53 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.48 +-------------------------------------+ Temperature [°C]: 21.46 Relative Humidity [%]: 54.47 +-------------------------------------+ ``` </details> - `tests/driver_l3gxxxx` <details> ``` main(): This is RIOT! (Version: 2023.04-devel-375-g75547-cpu/gd32v/periph_i2c_interrupt_driven) L3GXXXX gyroscope driver test application Initializing L3GXXXX sensor [OK] gyro [dps] x: +0, y: -1, z: -2 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: +0, y: +0, z: +0 gyro [dps] x: -1, y: +0, z: +4 gyro [dps] x: +0, y: +0, z: -21 gyro [dps] x: +0, y: +0, z: +6 gyro [dps] x: -43, y: +0, z: -13 gyro [dps] x: -21, y: -2, z: +0 gyro [dps] x: +0, y: +1, z: +3 gyro [dps] x: +25, y: +0, z: +0 ``` </details> - `tests/driver_hd44780` with `pcf8574a` I2C interface ### Issues/PRs references Co-authored-by: Karl Fessel <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]>

bors · 2023-02-17T20:09:41Z

Build failed (retrying...):

Murdock

bors · 2023-02-17T23:09:26Z

Build succeeded:

kfessel requested a review from kaspar030 as a code owner December 8, 2022 12:44

github-actions bot added the Area: sys Area: System label Dec 8, 2022

kfessel force-pushed the p-fmt-scn-opt branch from 9395feb to d3f8c9a Compare December 8, 2022 12:58

kfessel added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: low priority If set, builds of this PR will be queued behind others labels Dec 8, 2022

kfessel requested a review from maribu December 8, 2022 13:20

kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 8, 2022

maribu reviewed Dec 8, 2022

View reviewed changes

sys/fmt/fmt.c Outdated Show resolved Hide resolved

sys/fmt/fmt.c Outdated Show resolved Hide resolved

sys/fmt/fmt.c Outdated Show resolved Hide resolved

sys/fmt/fmt.c Outdated Show resolved Hide resolved

kfessel force-pushed the p-fmt-scn-opt branch from d3f8c9a to 3d7ae26 Compare December 8, 2022 15:38

pyropeter reviewed Dec 8, 2022

View reviewed changes

kfessel force-pushed the p-fmt-scn-opt branch from 4e3dde5 to f6a759d Compare December 8, 2022 21:23

maribu reviewed Dec 8, 2022

View reviewed changes

kfessel force-pushed the p-fmt-scn-opt branch 2 times, most recently from 439425a to 2b27f00 Compare December 9, 2022 13:46

benpicco approved these changes Feb 17, 2023

View reviewed changes

sys/fmt: optimize scn_u32_dec scn_u32_hex

18a783d

kfessel force-pushed the p-fmt-scn-opt branch from 2b27f00 to 18a783d Compare February 17, 2023 15:20

bors bot merged commit 8c2a4b4 into RIOT-OS:master Feb 17, 2023

MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023

kfessel mentioned this pull request Aug 22, 2023

sys/fmt: optimize scn_u32_hex() #19894

Closed

Teufelchen1 mentioned this pull request Mar 12, 2024

tests: Cover API of scn_u32_hex in the unittest #20458

Merged

Conversation

kfessel commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

kaspar030 commented Dec 8, 2022

Uh oh!

kfessel commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pyropeter Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

kfessel Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

kfessel Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

pyropeter Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

riot-ci commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

Teufelchen1 commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kfessel commented Dec 8, 2022

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maribu Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

kfessel Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

kfessel Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

kfessel commented Dec 9, 2022

Uh oh!

benpicco commented Jan 3, 2023

Uh oh!

benpicco commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

benpicco commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

benpicco commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

benpicco commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

bors bot commented Feb 17, 2023

Uh oh!

kfessel commented Dec 8, 2022 •

edited

Loading

kfessel commented Dec 8, 2022 •

edited

Loading

riot-ci commented Dec 8, 2022 •

edited

Loading

Teufelchen1 commented Dec 8, 2022 •

edited

Loading